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

[Ability] Implement Unburden #4534

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

Conversation

muscode13
Copy link

What are the changes the user will see?

Unburden implemented

Why am I making these changes?

Ability is not implemented

What are the changes from a developer perspective?

UnburdenBerryAbAttr - Triggers the speed boost once the user eats berries. It's based off how many they eat, so if the pokemon eats one berry they get +2, if they eat two they get +4 and so on.
UnburdenDefStolenAbAttr - This triggers if the user's item is stolen or lost when they are defending, in cases of Magician or moves like Thief, Knock Off and Pluck.
UnburdenAtkStolenAbAttr - This triggers when an item is lost during attacking, in cases like Pickpocket
pokemon.turnData.itemsLost - This variable tracks how many items the pokemon has lost during a turn. It is an integer that is used in UnburdenDefStolenAbAttr and UnburdenAtkStolenAbAttr. The integer gets decremented when an Unburden boost occurs. The integer gets incremented in various spots where items get stolen (Grip Claw, Moves, Abilities)

How to test the changes?

  • Use Pokemon with Unburden in the game and give them berries. Also have a pokemon with Magician steal items from them in double battles
  • run npm run test unburden

Checklist

  • I'm using beta as my base branch
  • There is no overlap with another PR? - I believe the other Unburden PR was closed
  • 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?
  • If I have text, did I make it translatable and add a key in the English locale file(s)?
  • Have I tested the changes (manually)?
    • Are all unit tests still passing? (npm run test)
  • Are the changes visual?
    • Have I provided screenshots/videos of the changes?

@muscode13 muscode13 requested a review from a team as a code owner October 1, 2024 17:02
@torranx torranx added the Ability Affects an ability label Oct 1, 2024
@Xavion3
Copy link
Contributor

Xavion3 commented Oct 2, 2024

This should give a battler tag that doubles speed, not directly interact with combat stages.

@muscode13
Copy link
Author

Will do!

@muscode13
Copy link
Author

Alright I switched it to tags! I wasn't sure if it should keep doubling speed for each item lost, but I can implement that if you guys want.

@torranx torranx requested a review from Xavion3 October 2, 2024 15:26
Copy link
Collaborator

@Tempo-anon Tempo-anon left a comment

Choose a reason for hiding this comment

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

I think for simplicity's sake of implementation a battler tag is a fine way of going about implementation. Any item lost/used up will grant the full doubled speed boost.

Right now I believe it's missing some documentation and edge case interactions though.

src/data/ability.ts Outdated Show resolved Hide resolved
src/data/ability.ts Outdated Show resolved Hide resolved
src/data/battler-tags.ts Outdated Show resolved Hide resolved
src/data/battler-tags.ts Show resolved Hide resolved
@muscode13
Copy link
Author

Alright I added documentation to the attributes, renamed them, added a test for neutralizing abilities, and accounted for getEffectiveStat

@Xavion3
Copy link
Contributor

Xavion3 commented Oct 3, 2024

Don't double the speed twice, current it doubles in both getEffectiveStat and onAdd

@muscode13
Copy link
Author

Don't double the speed twice, current it doubles in both getEffectiveStat and onAdd

Oh right forgot about that. Got a fix, just trying to get the Grip Claw to actually steal in my tests now

@muscode13
Copy link
Author

OK should only be doubling in one place now

@muscode13
Copy link
Author

Any updates? Been waiting for a review for almost a week now

Copy link
Collaborator

@Tempo-anon Tempo-anon left a comment

Choose a reason for hiding this comment

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

I think functionally for the most part it is fine. The only real blocker I see is to have the neutralizing gas interaction.

Should we also slap a (P) on unburden for the weird edge cases of gaining/losing this ability though?

src/data/ability.ts Outdated Show resolved Hide resolved
src/data/ability.ts Show resolved Hide resolved
src/data/ability.ts Show resolved Hide resolved
src/data/ability.ts Show resolved Hide resolved
src/field/pokemon.ts Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ability Affects an ability
Projects
Development

Successfully merging this pull request may close these issues.

4 participants