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][Ability] Fix mold breaker effect lingering if the user's move runs out of PP #5265

Open
wants to merge 4 commits into
base: beta
Choose a base branch
from

Conversation

SirzBenjie
Copy link
Member

@SirzBenjie SirzBenjie commented Feb 7, 2025

What are the changes the user will see?

Mold breaker will now no longer suppress abilities when its user uses a move with its last pp.

Why am I making these changes?

I was asked to look into this odd interaction with mold breaker
image

What are the changes from a developer perspective?

In move-phase's end method, MoveEndPhase will always be unshifted, instead of only if the move was not called via a follow up and an erroneous check to the canMove call (which would previously return false after the move's pp had been deducted, as the canMove check also looks at the remaining pp).

Instead, this.followUp is now passed as an argument to the MoveEndPhase constructor which is called unconditionally by end.
 
MoveEndPhase takes a new optional parameter - wasFollowUp that is checked before lapsing BattlerTagLapseType.AFTER_MOVE.
I don't know the interactions that would cause this lapse to need to not happen when a move is called via a follow up (something with Encore it looks like). This behavior, however, is consistent with what would happen prior to this PR.

Screenshots/Videos

Note me highlighting the lack (or presence of, in the fixed version) of the MoveEndPhase in the console.

Before the fix
mold-breaker-lure-bug-trimmed.mp4
After the fix
mold-breaker-pp-fix-condensed.mp4

How to test the changes?

Boy oh boy is testing this complicated. We went through a lot of discussion in the #endless-grinding channel on discord to figure out exactly how to trigger this bug. (first message link here: https://discord.com/channels/1125469663833370665/1225619031156064317/1337131126992539725)

In short, I was able to reproduce this after a lot of effort with the following set up:

Alternatively, you can have an ally with the double-lure abilities/passive where one of them is ignorable (e.g. watchog with illuminate / no guard).

For the lure test (which the video depicts), you'll need to get yourself into a situation where you have run out of PP at the same time as you KO the last enemy pokemon in a double battle with your mold breaker user, AND make sure your watchog ally has used its move that turn already. (If a move is used after the PP drops, the ignoreAbilities arena flag will be cleared by that move's MoveEndPhase).
The wave that you do this on has to be before there will be a trainer or boss, as these will nullify the lures. I did my test on wave 109.
When you KO the last pokemon, without this bugfix, the ignoreAbilities modifier will be true. Since No Guard is an ignorable ability, when checking for abilities that have the lure effect, it will be ignored. When moving on to the next battle, if it isn't a double battle, that means that no guard's lure ability didn't trigger.

Alternatively, you may be able to whip up a scenario with a slightly easier way to test it, though I didn't end up testing it this way:
Get a pokemon to have mold breaker and an easily-confirmed ignorable passive, like earth eater / levitate. Make sure your ally pokemon does not have mold breaker or an intervening ability. Have your ally have a single-target ground type move, and have it go after your mold breaker ally. Have your mold breaker ally use a damaging move on its last PP, and then have its ally use a ground move on your mold breaker pokemon. This must happen before any enemy moves intervene, so it's best if you use a spread move and KO both opponents.

Before the change, mold breaker would have erroneously still been active, and so the ground move would go through levitate / earth eater.
With the change, this should not happen.

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 tested the changes manually?
  • Are all unit tests still passing? (npm run test)
    • Have I created new automated tests (npm run create-test) or updated existing tests related to the PR's changes?
  • Have I provided screenshots/videos of the changes (if applicable)?
    • [ ] Have I made sure that any UI change works for both UI themes (default and legacy)?

Are there any localization additions or changes? If so:

  • [ ] Has a locales PR been created on the locales repo?
    • [ ] If so, please leave a link to it here:
  • [ ] Has the translation team been contacted for proofreading/translation?

@SirzBenjie SirzBenjie requested a review from a team as a code owner February 7, 2025 05:30
@Snailman11
Copy link
Collaborator

Link to Discord's Bug Report (no guard and illuminate on the same mon dont stack their lure effects)
https://discord.com/channels/1125469663833370665/1296386119604375583

and
(Wonder Guard does not work)
https://discord.com/channels/1125469663833370665/1251966075906556025

@SirzBenjie
Copy link
Member Author

Note: The current failing test had some assumptions that are invalidated by this PR. The functionality of the test still works as expected, though the automated methodology to advance the phases along causes issues. Specifically, the test expects dancer-invoked moves to not have MoveEndPhase (which they did not before, which is actually another way to trigger this bug).

@Madmadness65 Madmadness65 added Ability Affects an ability P2 Bug Minor. Non crashing Incorrect move/ability/interaction labels Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ability Affects an ability P2 Bug Minor. Non crashing Incorrect move/ability/interaction
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

4 participants