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

[Refactor] Added BattlerTagLapseType.AFTER_HIT #3655

Merged
merged 9 commits into from
Oct 22, 2024

Conversation

ElizaAlex
Copy link
Contributor

@ElizaAlex ElizaAlex commented Aug 20, 2024

What are the changes the user will see?

No changes user-side

Why am I making these changes?

Beak Blast and Shell Trap both implement BattlerTags that have some form of effect when the tag holder is hit but currently, each such effect needs to be handled and checked for individually. Adding a new LapseType to handle these similar tags all at once should help reduce duplicate code, standardize their implementations, and streamline future development.

What are the changes from a developer perspective?

  • BattlerTagLapseType.AFTER_HIT has been added
  • The implementations of Beak Blast and Shell Trap have been slightly adjusted to use this lapse type
  • Instead of handling each tag individually, MoveEffectPhase now lapses all AFTER_HIT tags using lapseTags()
  • Minor utility functions have been added in pokemon.ts and battler-tags.ts

How to test the changes?

There should be no change in gameplay, so just proceed as you normally would and watch for any unexpected bugs.
There are also existing tests for Beak Blast and Shell Trap:
npm run test beak_blast
npm run test shell_trap

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)

Adjusted BeakBlastChargingTag and ShellTrapTag to use new lapse type

Adjusted MoveEffectPhase to now lapse all tags with the ON_GET_HIT lapse type
Adjusted BeakBlastChargingTag and ShellTrapTag to use new lapse type

Adjusted MoveEffectPhase to now lapse all tags with the ON_GET_HIT lapse type
@ElizaAlex ElizaAlex marked this pull request as ready for review August 20, 2024 00:33
@torranx
Copy link
Collaborator

torranx commented Aug 20, 2024

Overlaps with #2559

@torranx torranx added the Refactor Rewriting existing code related label Aug 20, 2024
Copy link
Collaborator

@DayKev DayKev left a comment

Choose a reason for hiding this comment

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

Some minor style consistency nits.

src/data/battler-tags.ts Outdated Show resolved Hide resolved
src/data/battler-tags.ts Outdated Show resolved Hide resolved
src/field/pokemon.ts Outdated Show resolved Hide resolved
@DayKev
Copy link
Collaborator

DayKev commented Sep 21, 2024

Are you still around to update this PR? #2559 added a HIT lapse type, but didn't update the existing moves to use it.

Change `isOpponentTo` to `isOpponent`
@DayKev DayKev added Move Affects a move Miscellaneous Changes that don't fit under any other label labels Oct 14, 2024
@DayKev DayKev requested a review from a team as a code owner October 14, 2024 05:51
@DayKev DayKev changed the title [Refactor] Added ON_GET_HIT BattlerTagLapseType [Refactor] Added AFTER_HIT BattlerTagLapseType Oct 14, 2024
@DayKev DayKev self-requested a review October 14, 2024 05:52
@DayKev
Copy link
Collaborator

DayKev commented Oct 14, 2024

After comparing to the HIT tag type added in 2559, this seems like an AFTER_HIT tag in regards to timing unlike the former which is more ON_HIT, so I renamed the tag from ON_GET_HIT to AFTER_HIT.

DayKev
DayKev previously approved these changes Oct 14, 2024
@DayKev DayKev changed the title [Refactor] Added AFTER_HIT BattlerTagLapseType [Refactor] Added BattlerTagLapseType.AFTER_HIT Oct 14, 2024
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.

this could potentially open up a fix for #4552

src/data/battler-tags.ts Outdated Show resolved Hide resolved
src/data/battler-tags.ts Outdated Show resolved Hide resolved
src/field/pokemon.ts Outdated Show resolved Hide resolved
DayKev
DayKev previously approved these changes Oct 21, 2024
@DayKev DayKev requested a review from torranx October 21, 2024 09:46
@innerthunder innerthunder merged commit 7066a15 into pagefaultgames:beta Oct 22, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Miscellaneous Changes that don't fit under any other label Move Affects a move Refactor Rewriting existing code related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants