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

[Move] Full Stuff cheeks implementation #4941

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

Conversation

geeilhan
Copy link
Contributor

@geeilhan geeilhan commented Nov 26, 2024

What are the changes the user will see?

Stuff Cheeks is now fully implemented. (#3503)

What are the changes from a developer perspective?

Added a new class MoveSelectCondition inside move.ts which checks if a move is selectable in the move menu. It contains the condition (UserMoveConditionFunc) that allows selecting a move.

StuffCheeksCondition extends MoveSelectCondition and contains the condition that fails if the user does not hold at least 1 berry. If the condition fails a StuffCheeksTag, which extends MoveRestrictionTag, is added. This tag gets removed at the end of the turn.

Also added
private selectableConditions: MoveSelectCondition[] = [];
which saves the condition for the move.

Screenshots/Videos

Before implementation:

stuff-cheeks-implementation-before.webm

After Implementation:

stuff-cheeks-implementation-after.webm

How to test the changes?

Added 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)?

@geeilhan geeilhan requested a review from a team as a code owner November 26, 2024 07:09
@Tempo-anon Tempo-anon added the Move Affects a move label Nov 26, 2024
@geeilhan geeilhan marked this pull request as draft November 27, 2024 18:34
@geeilhan
Copy link
Contributor Author

Currently working on refactoring this so I use BattlerTags (MoveRestricitonBattlerTag) instead of making the move directly unselectable

@geeilhan geeilhan marked this pull request as ready for review December 1, 2024 06:20
@geeilhan
Copy link
Contributor Author

geeilhan commented Dec 1, 2024

I think something is wrong with the tests. npm run test doesn't work after syncing with beta branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Move Affects a move
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants