[BUG] Inaccurate Hit Chance Display for Custom Throwing Skills

[Darxo]

Versions

• MSU: 1.5.1

Describe the bug

Custom throwing skills using this.m.AdditionalAccuracy and this.m.AdditionalHitChance show incorrect hit chance values in the tooltip when using MSU's getRangedTooltip function.

To Reproduce

Steps to reproduce the behavior:

1. Look at the vanilla throwing weapon code for throwing axes
2. Notice how they add +20 flat hitchance and -10 hit chance at the bottom
3. Notice how they display +30 hitchance and -10 hitchance in the tooltip
4. Now implement a custom throwing skill, using this.m.AdditionalAccuracy of 20 and this.m.AdditionalHitChance of -10
5. Use MSU's getRangedTooltip function to display a tooltip for this
6. Notice how this skill instead displays +20 hitchance and -10 hitchance in the tooltip

Expected behavior

This tooltip should instead show +30 Hitchance in this case.

Screenshots

This is MSU's current implementation

This is the internal calculation for vanilla. You can see that the the hitchance penalty does not start until this.m.MinRange is reached. No, it is even inverted, if you hit enemies that are closer than your this.m.MinRange

In practice this means that when vanilla decides that throwing axes have these hitchance values

with a this.m.MinRange of 2

Then they need to display these tooltip values

[LordMidas]

The MSU display is accurate. Unfortunately, this feature was implemented in a very early version of MSU and we later realized that the vanilla AdditionalAccuracy and AdditionalHitChance are extremely misleading fields and are only relevant for applying the changes from named items. Whereas the actual modifications to accuracy and tile-based hitchance are hard-coded into the onAnySkillUsed function.

Nevertheless, this is changed in MSU whereby the AdditionalAccuracy and AdditionalHitChance now mean exactly what they say in their variable name. So these values are applied as they are defined. So if you define them to be 20 and 10 they will appear as 20 and 10 and that is intended. This also means that when using these fields in the MSU style and using getRangedTooltip, the modder should take that into account and not do any hardcoded modifications to hitchance in the onAnySkillUsed (unless for some other reason).

[Darxo]

So if you define them to be 20 and 10 they will appear as 20 and 10 and that is intended.

In vanilla when they "define" them as 20 and -10, as in apply those values to _properties.

They display 30 and -10 in the tooltip

Is your intention in MSU that the modder uses 3 values? AdditionalAccuracy only for MSU display, AdditionalHitChance for MSU display and for the _property change per tile. And then a RangedSkillModifier for the actual flat _property change?

[LordMidas]

Is your intention in MSU that the modder uses 3 values? AdditionalAccuracy only for MSU display, AdditionalHitChance for MSU display and for the _property change per tile. And then a RangedSkillModifier for the actual flat _property change?

No, the intention is to use AdditionalAccuracy and AdditionalHitChance for all the ranged skills you implement while using MSU. Then inside the getTooltip function you can use the MSU getRangedTooltip function to get the properly generate tooltip using those values. And inside the onAnySkillUsed function you use those two fields to modify the _properties. See the implementation of this in Reforged's hooks on skills such as Quick Shot.

[Darxo]

Here I post two snapshots of the same vanilla file, showing the two steps you describe but how Vanilla does it.

They add +20 Ranged Skill, -10 Hitchance per Tile during onUpdate And in the Tooltip they write +30% chance to hit and -10% chance to hit per tile.

Would you consider that as correct, what Vanilla does here?