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] Fully implement Rage #3667

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

Conversation

ElizaAlex
Copy link
Contributor

@ElizaAlex ElizaAlex commented Aug 20, 2024

What are the changes the user will see?

Rage now correctly applies an attack boost when a Pokémon is hit and Rage is the last move they used.

Some important notes about Rage's intended behavior:

  • A pokemon starts raging and gains the RageTag tag if it successfully hits a non-immune target with Rage.
  • A pokemon stops raging and loses the RageTag tag if it successfully uses a move that is not Rage, even if that move misses and/or fails.
  • A pokemon does NOT stop raging if
    • It uses rage and misses or the target is immune.
    • It was commanded to use a different move but has not acted yet
    • The Run or Ball options are selected in battle
    • It attempts to use a different move but is unable to act (via being flinched, asleep, confused, etc)

Why am I making these changes?

Rage is not fully implemented #3503

What are the changes from a developer perspective?

  • data/battler-tags: Added new RageTag to mark that the holder is raging. Uses MOVE_EFFECT and CUSTOM lapse types, to check if the tag should be removed because the holder used a different move, and to apply the attack boost when hit respectively.
  • phases/move-effect-phase: Added code to lapse a target's RageTagwhen they are hit, in line with the implementations of Shell Trap and Beak Blast.
  • data/move: Added AddBattlerTagAttr to Rage to apply a RageTag to the user on a successful hit.
  • test/moves/rage.test: New test file to test that rage gives attack boosts as intended
  • locales: Rage uses a custom message for its attack boosts, and since there's no other indicator I also added a separate message that doesn't exist in game for when a Pokémon successfully enters a rage. All non-English text is placeholder, and is just a copy of the English text.

(Added to the main post from a comment so people don't miss it):
The "rage is starting to build" text is not present in the mainline games, I added it tentatively in part because otherwise there is no in-game indicator that a pokemon has successfully started raging, and mostly because I needed to test things and record example videos and it saved me from having to play through an extra turn every time to check if the tag was/wasn't applied correctly.

It's a slight deviation from cartridge, but since Rage is a bit unintuitive, I figured I'd leave it in until people wanted it gone.

Screenshots/Videos

A raging pokemon gains an attack boost when hit:
A pokemon stays raging between turns:
A pokemon does not stop raging until it uses a non-rage move:

FastRage-SlowOther.mp4

A pokemon does not start raging until it hits a target with rage:

SlowRage.mp4

A pokemon does not start raging if the target is immune:

RageImmune.mp4

A Pokémon does not start raging if the initial Rage misses:

RageMiss.mp4

A Pokémon does not stop raging if the Player selects Run or Ball:

RageRunBall.mp4

Rage provides an attack boost per instance of damage:

DoublesRage.mp4

How to test the changes?

There are unit tests written in tests/moves/rage.test.ts that should hopefully cover most functionality! Currently I am aware that there are not tests for:

  • When the player selects Run or Ball
  • When Rage just misses normally
  • Comprehensive checks for any discrepancy in behavior between Player and Enemy Pokemon. (Most tests cover one or the other, but rarely both)
    In addition, the testing for when Rage fails or has no effect includes specific tests against Ghost types and against Pokemon who are semi-invulnerable through the use of Fly, but may not be exhaustive.

For the tests, just make sure that they look right and are running without issue, and potentially double check them using overrides, otherwise, you can check for any edge cases I may have missed also using the options in overrides.ts. I described Rage's expected behavior further up, and it can be cross referenced here

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?
  • If I have text, did I add placeholders for them in locales?
  • 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?

@flx-sta flx-sta added Move Affects a move Enhancement New feature or request labels Aug 20, 2024
src/data/move.ts Outdated Show resolved Hide resolved
src/data/battler-tags.ts Outdated Show resolved Hide resolved
@Adri1
Copy link
Contributor

Adri1 commented Aug 20, 2024

Does the text tied to "rageOnAdd" in the locales really exists? Can't find it anywhere

@ElizaAlex
Copy link
Contributor Author

Does the text tied to "rageOnAdd" in the locales really exists? Can't find it anywere

I don't think it does! (and the main post now says that) I tentatively added it partially because otherwise there is no in-game indicator that a pokemon has successfully started raging, and mostly because I needed to record example videos and it saved me from having to play through an extra turn every time to check if the tag was/wasn't applied correctly.

It's a slight deviation from cartridge, but since Rage is a bit unintuitive, I figured I'd leave it in until people wanted it gone.

@Adri1
Copy link
Contributor

Adri1 commented Aug 20, 2024

Does the text tied to "rageOnAdd" in the locales really exists? Can't find it anywere

I don't think it does! (and the main post now says that) I tentatively added it partially because otherwise there is no in-game indicator that a pokemon has successfully started raging, and mostly because I needed to record example videos and it saved me from having to play through an extra turn every time to check if the tag was/wasn't applied correctly.

It's a slight deviation from cartridge, but since Rage is a bit unintuitive, I figured I'd leave it in until people wanted it gone.

Thanks for your answer! As from the main crew of the translation team it was to know if it's just a mistake (if so I would I waited for its removal to translate) or on purpose 🫡

NicusPulcis
NicusPulcis previously approved these changes Aug 20, 2024
Copy link
Contributor

@NicusPulcis NicusPulcis left a comment

Choose a reason for hiding this comment

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

italian localization done

src/locales/it/battler-tags.ts Outdated Show resolved Hide resolved
ElizaAlex and others added 7 commits August 20, 2024 18:56
Added spacing inside of function calls

Co-authored-by: schmidtc1 <[email protected]>
Added pt_BR,  it, and fr translations.

Co-authored-by: Lugiad' <[email protected]>
Co-authored-by: Niccolò <[email protected]>
Co-authored-by: José Ricardo Fleury Oliveira <[email protected]>
@ElizaAlex ElizaAlex marked this pull request as ready for review August 20, 2024 23:39
Copy link
Contributor

@40chyan 40chyan left a comment

Choose a reason for hiding this comment

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

zh

src/locales/zh_CN/battler-tags.ts Outdated Show resolved Hide resolved
src/locales/zh_TW/battler-tags.ts Outdated Show resolved Hide resolved
@CodeTappert
Copy link
Collaborator

So there is a problem.
In German the "rageOnHit" Message is " verfällt in Raserei!" in the official games.
Which means something like " goes into a frenzy!" or " begins to rage".

So i dont really know what to put for "rageOnAdd"

src/data/battler-tags.ts Show resolved Hide resolved
src/test/moves/rage.test.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@AsdarDevelops AsdarDevelops left a comment

Choose a reason for hiding this comment

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

Couldn't find the "starting to build" as an official text, at least not in the newer games.

Added ES, and I also checked every other translation and suggestion commits before mine for typos regarding the use of \n and {{}}.

src/locales/es/battler-tags.ts Outdated Show resolved Hide resolved
@ElizaAlex ElizaAlex requested review from a team as code owners September 6, 2024 16:38
sodaMelon
sodaMelon previously approved these changes Sep 6, 2024
src/data/battler-tags.ts Outdated Show resolved Hide resolved
src/data/battler-tags.ts Outdated Show resolved Hide resolved
src/data/battler-tags.ts Outdated Show resolved Hide resolved

/**
* Displays a message to show that the user has started Raging.
* This is message isn't displayed on cartridge, and was included for clarity during gameplay and while testing.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* This is message isn't displayed on cartridge, and was included for clarity during gameplay and while testing.
* This message isn't displayed on cartridge, and was included for clarity during gameplay and while testing.

/**
* Ally Attack-Boost Test.
*
* Checks that Rage provides an attack boost if the user it hit after use.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Checks that Rage provides an attack boost if the user it hit after use.
* Checks that Rage provides an attack boost if the user is hit after use.


const leadPokemon = game.scene.getPlayerPokemon()!;

// Regieliki uses Fly. Boltund uses Rage, but Regieleki is invulnerable
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Regieliki uses Fly. Boltund uses Rage, but Regieleki is invulnerable
// Regieliki uses Phantom Force. Boltund uses Rage, but Regieleki is invulnerable

expect(leadPokemon.getStatStage(Stat.ATK)).toBe(0);
expect(leadPokemon.getTag(RageTag)).toBeNull;

// Regieleki finishes Fly, Boltund uses Rage
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Regieleki finishes Fly, Boltund uses Rage
// Regieleki finishes Phantom Force. Boltund uses Rage

@Adri1 Adri1 removed the Localization Provides or updates translation efforts label Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Move Affects a move
Projects
Development

Successfully merging this pull request may close these issues.