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

[battle, damage] refactor damage formula to match gen5+ #3196

Merged
merged 3 commits into from
Aug 11, 2023

Conversation

SBird1337
Copy link
Collaborator

@SBird1337 SBird1337 commented Aug 2, 2023

Refactors the damage calculation and fixes any (to my knowledge) inconsistancies to the calculation of later games.

Description

Key notes are:

  • Gamefreak often times rounds down on n.5 in their fixed point mathematics.
  • fixed point arithmetic (multiplication) is not associative, this changes the order of operations to match the original games.
  • A lot of the damage calculation function was quite messy, some aspects were factored in at the wrong place.
  • The main damage calculation should now be clearer to read.

Also fixes the contrary test against intimidate, which was using a wrong multiplier.

Adds tests for the basic damage calculation using an example from https://bulbapedia.bulbagarden.net/wiki/Damage#Example
Changes the test in weather_snow.c against solar blade from KNOWN_FAILING to PASS.

At the current state 2 old tests are failing with these changes:

  • Dry Skin increases damage taken from Fire-type moves by 25%
  • Swarm boosts Bug-type moves in a pinch

Refactored the 2 problematic tests to use exact values.

this is due to numerical issues in the damage formula that also exist in the original games. I verfied both on showdown and accepted that as ground truth.

Discord contact info

karathan

@SBird1337 SBird1337 marked this pull request as draft August 2, 2023 20:01
@SBird1337
Copy link
Collaborator Author

Technically I think this is ready for review, but I'll leave it as draft until we figure out what to do with the failing tests.

@SBird1337 SBird1337 force-pushed the refactor/damage branch 2 times, most recently from 8d2c097 to 23dd7b6 Compare August 3, 2023 08:49
@SBird1337 SBird1337 marked this pull request as ready for review August 3, 2023 08:49
@SBird1337
Copy link
Collaborator Author

Technically I think this is ready for review, but I'll leave it as draft until we figure out what to do with the failing tests.

I used exact values I obtained from a Smogon Calc for the 2 problematic tests. Ready for review :)

@SBird1337 SBird1337 force-pushed the refactor/damage branch 3 times, most recently from a59c0d6 to 14fafe9 Compare August 3, 2023 09:11
src/battle_util.c Outdated Show resolved Hide resolved
test/ability_swarm.c Outdated Show resolved Hide resolved
test/ability_dry_skin.c Outdated Show resolved Hide resolved
src/battle_util.c Outdated Show resolved Hide resolved
@SBird1337
Copy link
Collaborator Author

Please let me rebase before merging

@SBird1337
Copy link
Collaborator Author

addressed Edus review and rebased.

@mrgriffin
Copy link
Collaborator

@SBird1337 would you like to rebase, should I squash merge, or should I do a regular merge commit? :)

@AsparagusEduardo AsparagusEduardo merged commit 59da940 into rh-hideout:upcoming Aug 11, 2023
1 check passed
HunarPG pushed a commit to HunarPG/pokeemerald-expansion that referenced this pull request Aug 12, 2023
* [battle, damage] refactor damage formula to match gen5+

* [test] use exact values for dry skin, swarm tests

* fixup: assume stats for dry-skin, swarm tests

---------

Co-authored-by: sbird <[email protected]>
@AsparagusEduardo AsparagusEduardo mentioned this pull request Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants