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 for "Moves Can Miss Against Protect, Baneful Bunker, King's Shield" #4262

Merged
merged 9 commits into from
Sep 15, 2024

Conversation

PrabbyDD
Copy link
Contributor

What are the changes the user will see?

Fixing #4060, so if pokemon misses against a move that protects, it will properly protect against the move despite missing
After effects of a protected, missed attack do t trigger if the move is contact (tested with slash)
After effects of a protected, missed attack do not trigger if the move is non contact (tested with flash cannon)

Why am I making these changes?

To fix #4060

What are the changes from a developer perspective?

Moved protect check to be before hit check, and avoid hitcheck if the pokemon is protected

Screenshots/Videos

2024-09-14.13-19-17.mp4
2024-09-14.13-19-32.mp4
2024-09-14.13-20-17.mp4
2024-09-14.13-20-32.mp4
2024-09-14.13-45-09.mp4

How to test the changes?

Use npm run test, as well as changing override files to test moves, and manually go into move-effect-phase.ts and change return value of hitCheck() to be false.

npm test obstruct (modified)
npm test baneful_bunker (added)

Run these two tests as specifically. I also tested manually via removing the forceMiss() function, and changing hitCheck() to return false to be sure.

I would appreciate any feedback on the changes to the move-effect-phase.ts file. Not really a good way to test protect specifically, since it doesnt have after effects.

I tested manually with other moves as well, like King's Shield and Burning Bulwark. Seems to work fine for all now.

Checklist

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

…bc no easy way to test specifically with protect, and changes in move-effect to fix the issue
@PrabbyDD PrabbyDD requested a review from a team as a code owner September 14, 2024 20:49
@innerthunder innerthunder added P2 Bug Minor. Non crashing Incorrect move/ability/interaction Move Affects a move labels Sep 14, 2024
Copy link
Collaborator

@innerthunder innerthunder left a comment

Choose a reason for hiding this comment

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

The source code seems good; just some nits on the test code

src/test/moves/baneful_bunker.test.ts Outdated Show resolved Hide resolved
src/test/moves/baneful_bunker.test.ts Outdated Show resolved Hide resolved
src/test/moves/baneful_bunker.test.ts Outdated Show resolved Hide resolved
src/test/moves/baneful_bunker.test.ts Outdated Show resolved Hide resolved
src/test/moves/obstruct.test.ts Outdated Show resolved Hide resolved
@Tempo-anon Tempo-anon merged commit 3a683c0 into pagefaultgames:beta Sep 15, 2024
14 checks passed
@PrabbyDD PrabbyDD deleted the prabbyd4096ONEMORETIME branch September 15, 2024 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Move Affects a move P2 Bug Minor. Non crashing Incorrect move/ability/interaction
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants