[Bug] Fix enemy faint causing Frenzy moves to mishandle paralysis

[geeilhan]

[P2 BUG] Fixed Frenzy Counter Reset Method Should Move Fail (#4277)

What are the changes the user will see?

Example situation (How it used to be):

1. Player Pokemon start turn paralyzed
2. Player selects Thrash on the 1st turn (It should hit 3 times in this example)
3. The attack is successful and enemy Pokemon faints
4. During the next battle the player is not able to switch Pokemon due to Thrash (frenzy) (=> correct behaviour)
5. Player Pokemon fails the automatic attack (Thrash) because of paralyze
6. Enemy Pokemon used a sacrificing move and faints
7. During the start of the next battle the Player is not able to switch Pokemon nor able to select a move (=> incorrect behaviour)
8. Player Pokemon automatically uses Thrash and gets confused should it hit (=> semi correct behaviour)

My fix corrects the incorrect behaviour of Step 7 in the example situation. Step 8 does also differ since step 7 cancels the move (Thrash).

After the fix:
[...]
7. During the start of the next battle the Player is able to switch Pokemon and able to select a move
8. Player Pokemon does not get confused after the attack

Why am I making these changes?

I have a uni course where my goal is to contribute to and interact with an open source project of our choosing.
In addition, this bug affects gameplay and should therefore be fixed.
I found this bug from an existing issue.

What are the changes from a developer perspective?

handlePreMoveFailures() Link did not consider a move being cancelled during a frenzy move and therefore does not remove the BattlerTagType.FRENZY on the pokemon (pokemon.summonData.tags) Link as well as the frenzy moves queued on the pokemon (pokemon.getMoveQueue()) Link.

My implementation removes the BattlerTagType.FRENZY as well as empties the moveQueue of the frenzy moves should the attack be cancelled due to a status effect such as Paralysis, Sleep and Freeze Link .

The last commit Merge branch 'pagefaultgames:beta' into beta happend because I synced my fork before the PR and has no influence on my code.

Screenshots/Videos

At the 3rd turn the Pokemon doesn't attack due to paralysis but during my manual tests the Pokemon automatically attacks and gets confused after. (I just wasn't recording)

Before_Changes_Paralyzed_Thrash.webm

After_Changes_Paralyzed_Thrash.webm

How to test the changes?

I used src/overrides.ts in order to test my code. I was necessary since the test rely on RNG with Paralysis and the amount of hits for Thrash. Manually testing this might take some time but in order to reduce the testing time I changed some RNG values of the game.

This is the override I used is:
const overrides = { STARTER_SPECIES_OVERRIDE: Species.PONYTA, STARTING_LEVEL_OVERRIDE: 100, STATUS_OVERRIDE: StatusEffect.PARALYSIS, MOVESET_OVERRIDE: [ Moves.THRASH ], OPP_MOVESET_OVERRIDE: [ Moves.MEMENTO ], OPP_LEVEL_OVERRIDE: 1, OPP_IVS_OVERRIDE: [ 1, 1, 1, 1, 1, 1 ], BATTLE_TYPE_OVERRIDE: "single", } satisfies Partial<InstanceType<typeof DefaultOverrides>>;

I also changed the turnCount Link to 2 instead of a random number between 1 and 2.
As well as the paralysis chance to 2 Link.

Currently there are no automated test cases but I am working on it. Manual tests and npm run test have passed the tests.

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?
• If I have text, did I make it translatable and add a key in the English locale file(s)?
• Have I tested the changes (manually)?
  • Are all unit tests still passing? (npm run test)
• Are the changes visual?
  • Have I provided screenshots/videos of the changes?

[geeilhan]

This is my first contribution ever so if something is missing or is wrong in my PR please give me some feedback.

I was wondering if anyone has experience creating test cases for RNG outcomes such as "Does Pokemon attack during paralysis". I was thinking of if() {} else {} but was wondering if there is a better solution.

[DayKev]

This is my first contribution ever so if something is missing or is wrong in my PR please give me some feedback.

I was wondering if anyone has experience creating test cases for RNG outcomes such as "Does Pokemon attack during paralysis". I was thinking of if() {} else {} but was wondering if there is a better solution.

A decent amount of the RNG is controlled for automatically in tests, and for the stuff that isn't usually it's possible to force certain RNG outcomes by using vi.spyOn(). In this case though... it's a bit tricky. I might look into writing some status effect helper functions or something.

[geeilhan]

This is my first contribution ever so if something is missing or is wrong in my PR please give me some feedback.
I was wondering if anyone has experience creating test cases for RNG outcomes such as "Does Pokemon attack during paralysis". I was thinking of if() {} else {} but was wondering if there is a better solution.

A decent amount of the RNG is controlled for automatically in tests, and for the stuff that isn't usually it's possible to force certain RNG outcomes by using vi.spyOn(). In this case though... it's a bit tricky. I might look into writing some status effect helper functions or something.

Thanks for the feedback.
Yes that might be helpful for other future test cases too. Should writing more helper functions for this be a new issue?

[DayKev]

I'll put up a PR for the status activation override stuff in a few minutes (nearly finished).

[DayKev]

Alright, to use the new function in your test once it's added you can put await game.move.forceStatusActivation(true); during the turn you want paralysis to activate.
Example:

// first part of test

game.move.select(Moves.THRASH); // <-- this line is probably optional on the forced turn of a Frenzy move
await game.move.forceStatusActivation(true);
await game.toNextTurn();

// rest of test

[geeilhan]

Alright, to use the new function in your test once it's added you can put await game.move.forceStatusActivation(true); during the turn you want paralysis to activate. Example:

// first part of test

game.move.select(Moves.THRASH); // <-- this line is probably optional on the forced turn of a Frenzy move
await game.move.forceStatusActivation(true);
await game.toNextTurn();

// rest of test

Thank you that is very helpful :)

[PigeonBar]

Welcome to the PokeRogue project!

Could you please give some more info on how the frenzyMissFunc approach didn't work? It could potentially be a lot cleaner if it is possible to get it to work.

[geeilhan]

Welcome to the PokeRogue project!

Could you please give some more info on how the frenzyMissFunc approach didn't work? It could potentially be a lot cleaner if it is possible to get it to work.

I did not look deeply into it but from my test result I could see that the BattlerTag got reset but the MoveQueue did not.

Thinking back I put the frenzyMissFunc in the apply() function of FrenzyAttr (apply()). I will test again using the frenzyMissFunc in src/phases/move-phase.ts.

export const frenzyMissFunc: UserMoveConditionFunc = (user: Pokemon, move: Move) => {
  while (user.getMoveQueue().length && user.getMoveQueue()[0].move === move.id) {
    user.getMoveQueue().shift(); 
  }
  user.removeTag(BattlerTagType.FRENZY); // FRENZY tag should be disrupted on miss/no effect

  return true;
};

Update: @PigeonBar thank you for the insight you are completely right. I thought it wouldn't work because I didn't think about exporting the function frenzyMissFunc() to move-phases.ts .
I have cleaned my code using the frenzyMissFunc() and it is already implemented in my latest commit.
Manual tests as well as npm run test still passes.

[geeilhan]

I have added an automated test case (Commit) for this bug fix.
It checks the specific scenario described in (#4277).

Should I also make the automated test cases for every other frenzy move (e.g.: Outrage, Petal Dance, etc.)?