Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed binary incompatibility introduced by #2632 #2691

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

KABoissonneault
Copy link
Collaborator

While the changes introduced by #2632 do not require users to change their source to work (API compatible), it does prevent mods compiled on another DFU version from working (ABI incompatible).

For example, Jagget tested current master on v1.1.1, and got the following error:

System.MissingMethodException: single DaggerfallWorkshop.Game.Banking.DaggerfallBankManager.get_goldUnitWeightInKg()
  at Game.Mods.UncannyUI.Scripts.UncannyInventoryWindow.UpdateLocalTargetIcon () [0x0001e] in <73871b0f6c3841c4ab7a9171904397a8>:0 
  at Game.Mods.UncannyUI.Scripts.UncannyInventoryWindow.Setup () [0x00614] in <73871b0f6c3841c4ab7a9171904397a8>:0 
  at DaggerfallWorkshop.Game.UserInterfaceWindows.DaggerfallBaseWindow.Update () [0x00069] in <10d69f57d88e45598ab36ac2d243732c>:0 
  at DaggerfallWorkshop.Game.UserInterfaceWindows.DaggerfallPopupWindow.Update () [0x00000] in <10d69f57d88e45598ab36ac2d243732c>:0 
  at DaggerfallWorkshop.Game.UserInterfaceWindows.DaggerfallInventoryWindow.Update () [0x00000] in <10d69f57d88e45598ab36ac2d243732c>:0 
  at Game.Mods.UncannyUI.Scripts.UncannyInventoryWindow.Update () [0x00000] in <73871b0f6c3841c4ab7a9171904397a8>:0 
  at DaggerfallWorkshop.Game.DaggerfallUI.Update () [0x000d8] in <10d69f57d88e45598ab36ac2d243732c>:0 

Without changing his code, the code now relied on the get from goldUnitWeightInKg, which did not exist in v1.1.1. This is not an issue, since mods don't have to support older versions. But it does show that the opposite problem would occur if a mod expected a const and found only a get.

See 44b829b for reference.

I reintroduced goldUnitWeightInKg as an obsolete constant (warns if users recompile their mods without changing to the new variable), and changed the new variable to 'goldItemWeightInKg'

…UnitWeightInKg as an obsolete constant, and changed the new variable to 'goldItemWeightInKg'
Jagget
Jagget previously approved these changes Aug 20, 2024
@Mentill
Copy link
Contributor

Mentill commented Aug 20, 2024

Sorry to hear my PR led to this issue, even if it was using the code you suggested. However I see two issues:

First, the name 'goldItemWeightInKg' is ambiguous as it's unrelated to the chunk of gold used in potion-making. I suggest 'goldPieceWeightInKg'

Second, if I'm understanding this correctly, this would create a new variable that pulls from the item template data, but this new variable isn't referenced anywhere else in the game's code so it wouldn't actually affect the weight of gold pieces in the player's inventory.

@KABoissonneault
Copy link
Collaborator Author

I need to reduce the warnings in DFU, I missed that my code has introduced more by staying on the obsolete variable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants