PathOfBuilding icon indicating copy to clipboard operation
PathOfBuilding copied to clipboard

Mana Calculations Rounding Error

Open agentyoda opened this issue 3 years ago • 13 comments

Check version

  • [X] I'm running the latest version of Path of Building and I've verified this by checking the changelog

Check for duplicates

  • [X] I've checked for duplicate issues by using the search function of the issue tracker

Check for support

  • [X] I've checked that the calculation is supposed to be supported. If it isn't please open a feature request instead (Red text is a feature request).

What is the value from the calculation ingame?

Mana cost is 29 for Consecrated Path. Specifically this setup of Consecrated Path:

  • Consecrated Path (Lv. 18)
  • Fist of War
  • Elemental Damage with Attacks
  • Pulverise
  • Increased Critical Damage
  • Ruthless

What is the value from the calculation in Path of Building?

Mana cost is 30 for the same linked Consecrated Path.

Normally, the calculation for this is something like: 8 * 1.4 = 11.2 11.2 * 1.3 = 14.56 14.56 * 1.3 = 18.92(8) (cuts off decimals past the second) 18.92 * 1.3 = 24.59(6) 24.59 * 1.2 = 29.508 -> rounds down to 29.

This was consistent with how PoB (and in-game) calculated mana costs previously - I recall checking this behavior before, consistent with what I've seen in discussions like this reddit thread - but somewhere the calculation changed in the most recent patch (v2.22.0), so it no longer calculates this value correctly.

How to reproduce the issue

  1. Select the following gems (other combinations will show the issue as well, but to see this one specifically):
  • Consecrated Path (Lv. 18)
  • Fist of War
  • Elemental Damage with Attacks
  • Pulverise
  • Increased Critical Damage
  • Ruthless
  1. Check mana cost calculations in Path of Building
  2. Verify that it's 30 instead of 29

Character build code

https://pobb.in/ac-oOO71g1QZ

Screenshots

consecrated_path_full

agentyoda avatar Dec 09 '22 16:12 agentyoda

The rounding is so annoying level 18 blight -> petrified blood -> duration + brutality = 9 mana and 4 life ingame (caculates to 9.1 mana and 3.64 life) Whereas your 29.528 mana is displayed as 29 ingame

Rounding down (not current release wording) image

QuickStick123 avatar Dec 09 '22 21:12 QuickStick123

Do you have any increased/reduced modifiers affecting that Life cost in-game? Because, for some reason, base mana cost and multipliers on base mana cost always round down to the hundredth (and then down to the nearest lower integer at the end), but the increased/reduced modifiers applying after base mana cost calculation round up. I'll probably do some testing on this later in the league, since these mana mechanics aren't well documented...

agentyoda avatar Dec 09 '22 22:12 agentyoda

I think I have it figured out it seems petrified blood is using the floored number after mana multiplier is applied and rounding at after proportion is applied?

QuickStick123 avatar Dec 09 '22 22:12 QuickStick123

I had a go at figuring out a formula that fixes the rounding but haven't had any luck creating something robust. Here is a desmos table with a bunch of number that I was trying to get to all zero. https://www.desmos.com/calculator/fx0avlhnqc

QuickStick123 avatar Dec 10 '22 05:12 QuickStick123

Just to clarify, this issue is specifically about an issue that was introduced in v2.22.0. To illustrate this, I downloaded PoB v2.21.1 and compared the following build. The first set of screenshots was of the build in v2.21.1. The second set is immediately after I pressed 'Update' and updated to v2.23.0.

v2.21.1: 2 21 pob 2 21 pob mana

v2.23.0: 2 23 pob 2 23 pob mana

The build PoB for this one is: https://pobb.in/7dVrhdQ0qR8t (I'm assuming the build PoB can be imported into v2.21.1 since I made no changes to it, but if you need me to get the link from it in v.2.21.1, let me know and I can get that for you).

So there must have been some change to the calculation for mana cost for PoB. You can see in the second screenshot of each version how the 3.99 mana multiplier became a 4.0 multiplier even though nothing else changed between them.

I know this seems like a super small issue (and it is for most players), but I've been relying on PoB to calculate mana costs for sustaining Indigon, and that requires very exact mana numbers; even just one extra mana cost can make the Indigon mana series diverge, which means it can't be sustained. If this isn't a priority to fix, I certainly understand; I can calculate the mana costs by hand if need be. But since it was working in a previous version, my assumption is that this change is unintended and, thus, a calculation bug.

agentyoda avatar Dec 10 '22 22:12 agentyoda

After further investigation, the new numbers look more correct to me than the old ones.

Before we were flooring to two decimal points. Actual value 3.99854 for the multi became 3.99. We are now rounding, taking 3.99854 to 4.0.

Old calculation was 0.00854 below the actual value. New calculation is 0.00146 above the actual value, which is closer. If it's preferable to underestimate by six times as much as we're overestimating isn't something I can confidently answer.

This rounding now also takes place when adding the mod to the modlist. To get the old value we would need to remove the rounding added to ModList (line 110) and the rounding in the mana cost itself (which doesn't change the result of this specific example because it's the mod in the modlist that was being rounded).

Lilylicious avatar Dec 10 '22 23:12 Lilylicious

If it's supposed to round down instead of to nearest int, then we just need to change ModList and CalcOffence to floor instead of round.

Lilylicious avatar Dec 10 '22 23:12 Lilylicious

I changed the rounding in my skill costs pr https://github.com/PathOfBuildingCommunity/PathOfBuilding/pull/5009 recently but the bigger problem is I have no idea how they are rounding in game to get there numbers it is just not clear see my desmos link. I think the floor was working in more situations so I am fine to change it back.

QuickStick123 avatar Dec 11 '22 00:12 QuickStick123

I'll test this exact setup in-game and see what the mana cost is; that should tell us if we want to floor or simply round. Thanks for looking into this more.

agentyoda avatar Dec 11 '22 00:12 agentyoda

I tested the same setup (with respect to mana costs: a 6 mana skill + 4 130%'s + 140% mana multiplier, in that order, as seen in below screenshots). In-game had 23 mana, while v2.23.0 had 24 mana:

In-game: cleave 23 mana

PoB: cleave pob

EDIT: there seem to be other tests that suggest it does floor to the hundredths after each multiplier, and then down to the nearest integer after multiplying the base cost to the combined multiplier. I can do more, if we want more confidence before changing anything; just let me know.

agentyoda avatar Dec 11 '22 00:12 agentyoda

image image x1 is increased mana y1 is flat added mana from divine blessing z1 is mana tooltip l1 is life tooltip m1 is base cost, might need rounding / truncating? r1 is mana multiplier e1 is error it is off by formula by. image As you can see this formula works very well on regular situations but breaks down on others. I haven't been able to find something that works robustly especially with the cost conversion with pet blood.

QuickStick123 avatar Dec 11 '22 01:12 QuickStick123

The PR above floors the multipliers individually rather than at the end like we did before. The result is that the multiplier is 3.97 in the test PoB you gave us, compared to 3.99 in 2.21 and 4.0 in 2.22. The end result mana cost is still 23.

Lilylicious avatar Dec 11 '22 08:12 Lilylicious

That looks perfect. Thank you for taking the time to look into this!

agentyoda avatar Dec 11 '22 15:12 agentyoda