-
Notifications
You must be signed in to change notification settings - Fork 1
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
Adding the Choice of Balance event and aura system #102
base: mystery-battle-events
Are you sure you want to change the base?
Conversation
@@ -2151,12 +2152,23 @@ export default class BattleScene extends SceneBase { | |||
} | |||
|
|||
addMoney(amount: integer): void { | |||
this.money = Math.min(this.money + amount, Number.MAX_SAFE_INTEGER); | |||
const mysteryIncomeAura = this.mysteryEncounterAuras.FindAura(AuraType.INCOME); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just leaving a comment here so I don’t forget, this functionality should probably be moved to an applyModifiers() function
this.updateMoneyText(); | ||
this.animateMoneyChanged(true); | ||
this.validateAchvs(MoneyAchv); | ||
} | ||
|
||
getFormattedMoneyString(moneyAmount: number): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we having to multiply the current money value in the string by modifiers? Shouldn’t this just be a function to obtain a string.
//const userLocale = navigator.language || "en-US"; | ||
//const formattedMoneyAmount = moneyAmount.value.toLocaleString(userLocale); | ||
//const message = i18next.t("battle:moneyPickedUp", { moneyAmount: formattedMoneyAmount }); | ||
const message = this.scene.getFormattedMoneyString(moneyAmount.value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to above, you should probably move the logic of that function back to here where it was before, or at least have the util function in this class
encounter.misc.push(rewardsArray[i]); | ||
} | ||
|
||
encounter.setDialogueToken("dynamic1", rewardsArray[0].generateMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor nit: can we have the names of these tokens be more descriptive to their function? "option1Tooltip" or something like that
this.positiveOption = positiveOption; | ||
} | ||
|
||
generateMessage(): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This type of function is going to probably cause lots of issues with translations. Could we instead just have a few properly formatted string templates that are stored in the locales file? And you can inject the programmatic logic in via token instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Examples of a string template:
"augmentTextTemplate": "{{descriptor}} for {{duration}}"
"augmentTextJoiner": ", then "
etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and to get the proper string, you would just do i18next.t("mysteryEncounter:augmentTextTemplate", { descriptor: this.getDescriptorText(negativeText), duration: this.formattedWaves(negativeDuration) });
break; | ||
// These are for money rewards | ||
case PositiveRewards.INSTANT_MONEY: | ||
newStrength = "$" + String(strength); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a proper "poke dollar" character you can use.
//} | ||
} | ||
|
||
private formattedStrengths(type: number, strength: number): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd recommend moving this function to i18n formatters. Check out this example:
PokeRogue-Events/src/plugins/i18n.ts
Line 142 in ec49cd0
i18next.services.formatter.add("money", (value, lng, options) => { |
@@ -14,3 +16,185 @@ export class MysteryEncounterData { | |||
} | |||
} | |||
} | |||
|
|||
export class MysteryEncounterAuras { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a note/reminder since we've already discussed. This logic can probably sit in Modifier class extensions :)
✅ Deploy Preview for pokerogue-mystery-encounter-beta ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
What are the changes?
Starting to add the Choice of Balance event and aura system to go with it
Why am I doing these changes?
What did change?
Screenshots/Videos
How to test the changes?
Checklist
npm run test
)