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

[QoL] Show message when quick claw is triggered #1684

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

torranx
Copy link
Collaborator

@torranx torranx commented Jun 1, 2024

What are the changes?

  • Added message when quick claw is triggered: "{{pokemonName}} used its quick claw to move faster!"

Why am I doing these changes?

  • To avoid confusion when slower pokemon "suddenly" gets the first action, the same way it shows a message when grip claw is triggered.

What did change?

  • Message when quick claw is triggered

Screenshots/Videos

Screen.Recording.2024-06-01.at.6.32.24.PM.mov

How to test the changes?

  • set STARTER_SPECIES_OVERRIDE to a slow pokemon like Shuckle
  • set STARTING_HELD_ITEMS_OVERRIDE: Array<ModifierOverride> = [{name: "QUICK_CLAW", count: 1}];

Checklist

  • 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)
  • Are the changes visual?
  • Have I provided screenshots/videos of the changes?

@@ -775,6 +775,9 @@ export class BypassSpeedChanceModifier extends PokemonHeldItemModifier {

if (!bypassSpeed.value && pokemon.randSeedInt(10) < this.getStackCount()) {
bypassSpeed.value = true;
if (this.type instanceof ModifierTypes.PokemonHeldItemModifierType && this.type.id === "QUICK_CLAW") {
pokemon.scene.queueMessage(getPokemonMessage(pokemon, " used its quick claw to move faster!"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should probably be localized

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

unless we remove the pokemon prefix coming from the getPokemonMessage() and place the string to a regular i18n locale translation, I dont think this is possible to be localized. please see src/messages.ts

so we have 2 options.

  • keep the pokemon prefix but wont be localized
  • or remove the pokemon prefix so localization would be possible

Copy link
Collaborator

Choose a reason for hiding this comment

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

How about you abstract it to make it work?

Suggested change
pokemon.scene.queueMessage(getPokemonMessage(pokemon, " used its quick claw to move faster!"));
const pokemonPrefix = getPokemonPrefix(pokemon);
pokemon.scene.queueMessage(i18n.t(:itemUse:quickclaw', {pokemon, pokemonPrefix}));

and then in your translations something like

const de: Record<string, any> = {
  "itemUse:quickclaw": "{{pokemonPrefix}} {{pokemon.name}} used its quick claw to move first!"
}

(just an inspiration, not actual code)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the getPokemonPrefix() returns an untranslated text too tho. the whole src/messages.ts does not have translation functionality. i think it needs a separate PR for it

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree.
but you can still translate the part you are in control of

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i cant translate only part of it tho, and i wouldnt wanna touch anything from messages.ts on this PR because its being used all over the codebase.
also for reference someone is already working on the translation https://github.com/pagefaultgames/pokerogue/pull/1276/files

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Tempo-anon it's now up to you

@Tempo-anon Tempo-anon added the Localization Provides or updates translation efforts label Jun 1, 2024
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.

Got user complaints about this. I'm fine with not having it localized just yet and having the localization in a different PR.

@flx-sta
Copy link
Collaborator

flx-sta commented Jun 4, 2024

@Tempo-anon the foundation for localization support can be found in this PR
#1276

@Tempo-anon Tempo-anon merged commit 48f60a5 into pagefaultgames:main Jun 4, 2024
3 checks passed
Jason954 pushed a commit to Jason954/pokerogue that referenced this pull request Jun 6, 2024
Korwai pushed a commit to Korwai/pokerogue that referenced this pull request Jun 14, 2024
SavGRY pushed a commit to SavGRY/pokerogue that referenced this pull request Jun 16, 2024
@torranx torranx deleted the qol-quick-claw branch October 4, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Localization Provides or updates translation efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants