search

Interkarma / daggerfall-unity

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

Replace PC Gold Weight Literal With Loaded Value #2632

Open Mentill opened 4 months ago

Mentill commented 4 months ago

The weight of Gold Pieces in the player's inventory is stored in DaggerfallBankManager.cs as a literal value. This change instead indirectly loads it in from the itemTemplates array set in ItemHelper.cs

The intended result is that it thus exposes to mods the weight of gold pieces in the player's inventory, which it does according to my tests. It also appropriately affects certain transfers of gold pieces.

numidium commented 4 months ago

I have a few issues with the change as it is implemented:

1. `goldUnitWeightInKg` is now a publicly mutable variable, which means its value can change at any time

2. Being a `static` variable, I can't be certain of when its initialization from the item template is made. How do we know the modded value is being taken into account?

Good point. Couldn't we also just use a lambda expression to directly reference the item template?

public static float goldUnitWeightInKg => DaggerfallUnity.Instance.ItemHelper.GetItemTemplate(ItemGroups.Currency, 0).baseWeight;

KABoissonneault commented 4 months ago

Good point. Couldn't we also just use a lambda expression to directly reference the item template?

I was considering it, but I wasn't sure if we wanted to support mods that modified the template in the middle of gameplay. Plus there's the performance consideration of doing that access each time, though caching is not free either.

Mentill commented 4 months ago

The property cannot be modified, and the value would not be invoked until anything in DFU actually relies on the weight of gold, which should be long past the point at which mods modify item templates and such.

I tested your code and it seems to work fine, so I'll update it to that.

KABoissonneault commented 3 months ago

No need to request my review. I'm leaving this on hold because I'm leaving DFU stable while we wait for potential v1.1 fixes. PRs will reopen once I have a plan for v1.2