jopeek / fvtt-loot-sheet-npc-5e

FVTT - Loot Sheet NPC 5E
MIT License
44 stars 85 forks source link

currency exchange issue #364

Open rinjuu123 opened 2 years ago

rinjuu123 commented 2 years ago

Describe the bug If a player want to sell a item that has a buyout for 1 GP and the sell ration is 25% so he should get 0,25gp from the merchant, but he gets 5 SP. I think there is a problem with the math in it. My exchange rates are "1 GP = 10 SP".

To Reproduce Steps to reproduce the behavior:

  1. Go to Merchant
  2. Sell Item "Net" for 0,25 GP
  3. get only 5 SP (value of 0.5 GP)

Expected behavior After selling the "Net" my player would get 2 SP an 5 CP

AdamJDyer commented 2 years ago

I have also experienced this issue. In my case, the player sold an item with a 15gp price to a merchant that was buying at 15% price. It should have been sold for 2.25 gold, or 2 gold 2 silver and 5 copper. The chat log confirmed this. Instead, it was sold for 2 gold and 5 silver, with no copper pieces exchanged. Essentially it treated the exchange price as if it were 2.5 gold instead of 2.25 gold. The player received 2.5gp and the merchant had 2.5gp in coins taken away, but the chat log still showed the purchasing price as 2.25gp.

AdamJDyer commented 2 years ago

I have attempted modifying the .js file directly and I think I have located what might be the issue, in the "_smoothenFunds" function within TradeHelper.

I noticed that when I removed electrum from the list of conversions (because I don't like electrum), like so... const compensation = { "pp": "gp", "gp": "sp", "ep": "sp", "sp": "cp" }; ...the initial issue was fixed, except that now prices from transactions exchanged the correct amount of gold and silver (and no electrum, like I wanted), but surprisingly exchanged 10x as much copper as it was supposed to.

It turns out that this line let change = (currency === 'pp') ? currentPart / rates[currency] : currentPart * rates[compensation[currency]]; seems to be the culprit. With electrum removed from the conversion chain, the issue became clear. The currency calculation for a 2.25 gold exchange went like this as it iterated through the loop in the method:

With electrum removed from the conversion chain, I was able to fix the behavior by altering the code as such: let change = (currency === 'pp') ? currentPart / rates[currency] : currentPart * 10; However this will only work with a 1gp=10sp=100cp conversion rate, and only if electrum is ignored. Thus I think the proper fix should be to base this calculation off of the conversion rate between the last coin and the current coin, instead of between gold and the current coin. I could be wrong, of course, so be sure to investigate yourself. In addition, I'm not sure if the pp calculations in this section are working correctly, but I have not investigated that yet.

(Incidentally, I'd love an optional toggle in the settings to ignore and bypass electrum entirely)

rinjuu123 commented 2 years ago

I have attempted modifying the .js file directly and I think I have located what might be the issue, in the "_smoothenFunds" function within TradeHelper.

I noticed that when I removed electrum from the list of conversions (because I don't like electrum), like so... const compensation = { "pp": "gp", "gp": "sp", "ep": "sp", "sp": "cp" }; ...the initial issue was fixed, except that now prices from transactions exchanged the correct amount of gold and silver (and no electrum, like I wanted), but surprisingly exchanged 10x as much copper as it was supposed to.

It turns out that this line let change = (currency === 'pp') ? currentPart / rates[currency] : currentPart * rates[compensation[currency]]; seems to be the culprit. With electrum removed from the conversion chain, the issue became clear. The currency calculation for a 2.25 gold exchange went like this as it iterated through the loop in the method:

  • 2.25 gold ends up as 2 gold with a remainder of .25, which is multiplied by silver's "rates" variable, which is 10.
  • Then 2.5 silver ends up as 2 silver with a remainder of .5, which is multiplied by copper's "rates" variable.
  • However, because the rates variables are based off the conversion from gp and not from the currency above them, it multiplies .5 times 100, ending up with 50 remaining copper instead of the correct value of 5!

With electrum removed from the conversion chain, I was able to fix the behavior by altering the code as such: let change = (currency === 'pp') ? currentPart / rates[currency] : currentPart * 10; However this will only work with a 1gp=10sp=100cp conversion rate, and only if electrum is ignored. Thus I think the proper fix should be to base this calculation off of the conversion rate between the last coin and the current coin, instead of between gold and the current coin. I could be wrong, of course, so be sure to investigate yourself. In addition, I'm not sure if the pp calculations in this section are working correctly, but I have not investigated that yet.

(Incidentally, I'd love an optional toggle in the settings to ignore and bypass electrum entirely)

Thank you for this, but i'm not intelligent enough for that kind of fix :)

I have to hope, that the dev will help us here :)

DanielBoettner commented 2 years ago

Hey I just merged some updates. Will take a look at this thread over the weekend how your research effects the changes or if I refactored that.

If you are a dev and would like to contribute I would be glad to help you get started with any questions on the lootsheet.