Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replicate known rollover points in damage formula #4962

Closed
wants to merge 5 commits into from

Conversation

@SadisticMystic
Copy link
Contributor

commented Nov 10, 2018

After a variety of tests documented in #4820 and elsewhere, I'm confident that these mechanics are accurate in generations that use the current vocabulary of modifiers.

This commit captures four spots where the in-game formula can produce a rollover:

  • after multiplying L * P * A, if the result exceeds the boundaries of a 32-bit register, only the lower 32 bits are kept, then that leftover value is put through division by D and 50 as usual
  • during the application of any modifier, if the current damage value times the numerator exceeds 32 bits, prior to dividing by the denominator (4096)
  • during the random roll step, if the current damage times the (much smaller) numerator exceeds the usual 32 bits, prior to dividing by the denominator (which this time is only 100)
  • at the very end (even after the check for minimum 1 damage in gen 6-7), the damage figure is truncated from 32 bits to 16 to match the data width of the variable used to store HP, and the upper 16 bits are simply discarded

These changes would have a disruptive effect on all-out competitive Custom Game battles if Showdown continues to use the same engine limitations there as in normal battles, causing almost all the moves in use there to deal less damage than before, and making it even harder for such battles to reach a satisfactory conclusion. As if the notion of "competitive Custom Game" wasn't already unappealing enough.

@Zarel

This comment has been minimized.

Copy link
Owner

commented Nov 10, 2018

Man, it'd be nice to note overflows in parens, but it'd be an information leak. :(

@Zarel

This comment has been minimized.

Copy link
Owner

commented Nov 10, 2018

It'd be nice if we could make this optional. Maybe a rule named No 32-bit Overflow

@Zarel

This comment was marked as resolved.

Copy link
Owner

commented Nov 10, 2018

This pull request introduces 4 alerts when merging cd35142 into c1dc805 - view on LGTM.com

new alerts:

  • 4 for Missing 'this' qualifier

Comment posted by LGTM.com

sim/battle.js Outdated Show resolved Hide resolved

@Zarel Zarel closed this in f709490 Nov 28, 2018

@SadisticMystic SadisticMystic deleted the SadisticMystic:patch-4 branch Dec 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.