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

[Bug] Fixed OHKO moves being affected by accuracy and evasion battle stats #3117

Merged
merged 5 commits into from
Jul 25, 2024

Conversation

Corrade
Copy link
Contributor

@Corrade Corrade commented Jul 24, 2024

What are the changes?

Fixed OHKO moves being affected by accuracy and evasion battle stats.

Why am I doing these changes?

Fixing a bug that was recently raised: https://discord.com/channels/1125469663833370665/1176874654015684739/1265121370954797056

Bulbapedia: "This chance of hitting is not an accuracy: it is not affected by accuracy and evasion stats" (https://bulbapedia.bulbagarden.net/wiki/Fissure_(move))

What did change?

One line as noted in the diff of this PR. Note that the changed function is only called by hitCheck() in src\phases.ts:3058, which looks like:

    // other logic omitted

    // properly accounts for OHKO moves
    const moveAccuracy = this.move.getMove().calculateBattleAccuracy(user, target);

    if (moveAccuracy === -1) {
      return true;
    }

    // doesn't account for OHKO moves until this PR
    const accuracyMultiplier = user.getAccuracyMultiplier(target, this.move.getMove());
    const rand = user.randSeedInt(100, 1);

    return rand <= moveAccuracy * accuracyMultiplier;

Screenshots/Videos

N/A

How to test the changes?

npm run test fissure
npm run test hustle

TODO (in my opinion) - refactor hitCheck() to separate its RNG components (as outlined above) so we can test accuracy stuff without needing to call deeper functions.

Checklist

  • I'm using beta as my base branch
  • There is no overlap with another PR?
  • The PR is self-contained and cannot be split into smaller PRs?
  • Have I provided a clear explanation of the changes?
  • Have I considered writing automated tests for the issue?
  • Have I tested the changes (manually)?
    • Are all unit tests still passing? (npm run test)
  • [No] Are the changes visual?
    • [No] Have I provided screenshots/videos of the changes?

@torranx
Copy link
Collaborator

torranx commented Jul 24, 2024

Can you please update the skipped test on src/test/abilities/hustle.test.ts? And if you can add addtl test/s on fissure, that'd be great!


vi.spyOn(pikachu, "getAccuracyMultiplier");
vi.spyOn(allMoves[Moves.FISSURE], "calculateBattleAccuracy");

game.doAttack(getMovePosition(game.scene, 0, Moves.FISSURE));
await game.phaseInterceptor.to(DamagePhase);

expect(game.scene.getEnemyPokemon().turnData.damageTaken).toBe(game.scene.getEnemyPokemon().hp);
expect(enemyPokemon.turnData.damageTaken).toBe(enemyPokemon.getMaxHp());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

game.scene.getEnemyPokemon() returned undefined when it was originally called here, so I moved it up a bit.

src/test/moves/fissure.test.ts Outdated Show resolved Hide resolved
src/test/moves/fissure.test.ts Outdated Show resolved Hide resolved
@Tempo-anon Tempo-anon added the (Legacy) Bug Legacy Label, don't apply to new issues/PRs label Jul 24, 2024
@Corrade Corrade requested a review from torranx July 24, 2024 08:23
Copy link
Collaborator

@torranx torranx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me

@Tempo-anon Tempo-anon merged commit 928a3ae into pagefaultgames:beta Jul 25, 2024
4 checks passed
Vassiat pushed a commit to Vassiat/pokerogue that referenced this pull request Jul 26, 2024
…stats (pagefaultgames#3117)

* Fixed OHKO moves being affected by accuracy and evasion battle stats

* Added related tests for Fissure, unskipped related test for Hustle

* Tweaked fissure accuracy and evasion tests to use spyOn() for getAccuracyMultiplier() as per feedback

* Fixed accuracy test for Fissure
@Corrade Corrade deleted the ohko_battle_stat_fix branch September 28, 2024 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(Legacy) Bug Legacy Label, don't apply to new issues/PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants