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] Fix #5358 Abilities that Redirect Moves Consider Move-Typings before Ability Modifiers #5464

Merged
merged 5 commits into from
Mar 11, 2025

Conversation

emdeann
Copy link
Contributor

@emdeann emdeann commented Mar 2, 2025

What are the changes the user will see?

Redirection abilities based on type (Lightningrod, Storm Drain) will consider the current type of a move, not its base type. i.e. Lightningrod will not affect normalize thunderbolt, and Storm Drain will affect liquid voice psychic noise.

Why am I making these changes?

Fixes #5358

What are the changes from a developer perspective?

There is already a method to get a move's current type based on the pokemon using it, these abilities just weren't using it. I added the user of the move as an expected parameter (in args) to apply for RedirectMoveAbAttr and replaced allMoves[moveId].type with user.getMoveType(allMoves[moveId]).

I don't forsee any edge cases here as these abilities should always consider the active type of a move, and if getMoveType doesn't always return the correct type then it would be an issue with that method.

Screenshots/Videos

2025-03-02.00-24-05.mp4

How to test the changes?

Automated 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 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?

@emdeann emdeann requested a review from a team as a code owner March 2, 2025 08:41
@Madmadness65 Madmadness65 added Ability Affects an ability P2 Bug Minor. Non crashing Incorrect move/ability/interaction labels Mar 2, 2025
@emdeann emdeann changed the title [Bug] Fix Abilities that Redirect Moves Consider Move-Typings before Ability Modifiers [Bug] Fix #5358 Abilities that Redirect Moves Consider Move-Typings before Ability Modifiers Mar 3, 2025
Copy link
Collaborator

@damocleas damocleas left a comment

Choose a reason for hiding this comment

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

tested and works for weather ball and other instances I could find

Copy link
Member

@SirzBenjie SirzBenjie left a comment

Choose a reason for hiding this comment

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

Good set of tests ✅
Clean, concise code ✅
Added documentation ✅
10/10 would review again

@SirzBenjie SirzBenjie self-requested a review March 11, 2025 02:58
@SirzBenjie SirzBenjie enabled auto-merge (squash) March 11, 2025 02:59
@SirzBenjie SirzBenjie merged commit 929392f into pagefaultgames:beta Mar 11, 2025
13 checks passed
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
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Abilities that Redirect Moves Consider Move-Typings before Ability Modifiers
4 participants