search

Interkarma / daggerfall-unity

Open source recreation of Daggerfall in the Unity engine
http://www.dfworkshop.net
MIT License
2.74k stars 332 forks source link

Fixed binary incompatibility introduced by #2632 #2691

Open KABoissonneault opened 3 months ago

KABoissonneault commented 3 months ago

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 44b829bb659f66374b7c301503 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'

Mentill commented 3 months ago

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 commented 3 months ago

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