j-cob44 / max-hit-calc

Runelite plugin which automatically calculates stats about max hit and displays to the player.
https://runelite.net/plugin-hub/show/max-hit-calculator
BSD 2-Clause "Simplified" License
2 stars 1 forks source link

Issue with soulreaper max hit #27

Closed hugocowan closed 6 months ago

hugocowan commented 7 months ago

Hey there, looks like another rounding issue type of problem! image

The max hit shows as 67 when it should be 68.

The next max hit tooltip is interesting, it says you need 0 strength bonus to achieve a max hit of 68, and 2% extra prayer bonus.

j-cob44 commented 7 months ago

I have a few questions about your setup so I can do calculations on my side:

  1. Are you at boosted max strength level (118)?
  2. Is your equipment strength with that setup 166?
  3. ~What attack style are you using?~ NVM, it says defensive right there.
  4. Is the special attack calculation correct with this setup?

With that I should be able to help more.


Usually the prediction tooltip only shows zeros when the max hit shown is different from the max hit value used in the prediction calculation. That is interesting. For the panel, the value is rounded down and turned into an integer and while in the Predictions it is not rounded at all and used as the original double value returned. This should be fixed as they should be equivalent. But this also could be a red herring for fixing the axe calculation as the prediction could be using any value under 68 but more than 67.00.

https://github.com/j-cob44/max-hit-calc/blob/8f37cb409706fe0fabb059c68dcebe435329763d/src/main/java/com/maxhitcalc/MaxHitCalcPlugin.java#L248-L248

https://github.com/j-cob44/max-hit-calc/blob/8f37cb409706fe0fabb059c68dcebe435329763d/src/main/java/com/maxhitcalc/PredictNextMax.java#L44-L44

If you'd like to try to debug this, I think the first step would be seeing the actual value the MaxHit.calculateMeleeMaxHit function is returning by outputting the maxHit variable before it returns on line 290 in MaxHit.java.

hugocowan commented 7 months ago
  1. Yes, 118 strength
  2. Yes, Melee strength is 166
  3. Yep, defensive
  4. Yes, the special attack is correct

The baseDamage in calculateMeleeMaxHit is 67.703125, which ends up as 67 when rounded down and returned as maxHit

I'll also add that this rounding error happens with multiple different strength bonus variations. The amount baseDamage is off by varies. Sometimes it's 0.05 off from the proper number, sometimes it's 0.3 off.

I tried changing the magic value of 0.009 to other values, and that did fix this specific max hit, but also broke others. Seems that something else is wrong :/

j-cob44 commented 7 months ago

Right now, I'm trying to narrow down the causes to find where changes need to be made. Thanks for the additional info, it should help out.

With high melee strength bonus setups like this one, is the calculator correct if you are not using any prayers?

Edit: With the initial setup in this thread (118 str, 166 bonus, defensive style, piety on), is the correct max hits in order: 55, 58, 60, 63, 65, 68?

j-cob44 commented 7 months ago

Hey @hugocowan, I've made a significant breakthrough.

I misunderstood how the soul stack bonus was calculated in game. Originally, I thought the prayer bonus and soul stack bonus were added together: strengthlevel * (prayerbonus + soulstackbonus). The 0.009 value I found was just from looking at formulas, using Desmos, and spreadsheeting results. The current implementation uses this method:

https://github.com/j-cob44/max-hit-calc/blob/8f37cb409706fe0fabb059c68dcebe435329763d/src/main/java/com/maxhitcalc/MaxHit.java#L239-L251

This is flawed because it is affecting the calculation of the prayer bonus. They should be separate and additive like this: (strength * prayerbonus) + (strength * soulstackbonus) Therefore, I believe the correct implementation is this:

https://github.com/j-cob44/max-hit-calc/blob/601ec965433049a76d0b532658e161aacd8ff78e/src/main/java/com/maxhitcalc/MaxHit.java#L239-L248


I've compared it's results to all the original tests in my spreadsheet and the new problem in this thread and it is correct for all of them so far. And no more magic numbers either! I've created the branch sr-axe-test and pushed the changes there. When you have the chance, please test this change on that branch in game and let me know how it's doing!

hugocowan commented 6 months ago

I just tried out the new branch with multiple different strength configurations, going through each and every max hit per soul stack level and everything is right now! I think you've done it!

j-cob44 commented 6 months ago

Great! I'm going to rename the branch to v1.12.3 and begin using it for updating other problems I've found for next release. I'll update the progress here and keep the issue open until it is confirmed working when released in Runelite.

j-cob44 commented 6 months ago

Hey @hugocowan ! Update 12.3 was pushed into runelite last night! Please let me know if everything is still working correctly.

hugocowan commented 6 months ago

Hi there! Nicely done, I currently no longer have the soulreaper axe, but I used the branch extensively and found 0 issues with it. Thanks so much for working on this plugin and for sorting out the axe!

j-cob44 commented 6 months ago

Thanks for all the help testing it! Glad we got it figured out.