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

Taunt Implementation #2159

Closed
wants to merge 1 commit into from

Conversation

mKroogz
Copy link
Contributor

@mKroogz mKroogz commented Jun 12, 2024

What are the changes?

This PR implements Taunt and Taunt handling logic around currently implemented use/edge cases.

Why am I doing these changes?

I wanted a fun first PR to contribute to this awesome game and a friend asked me to implement Taunt.

What did change?

I have added implemented the Taunt move with logic alongside the conditions that that would cause it fail. I've also added the (english) dialogue for all of the taunt messages and interactions.

Screenshots/Videos

Will try to add these shortly.

How to test the changes?

  • Run locally and start a new game with all Sinnoh starters
  • Exclusively take lures to enhance chance of double battles
  • Level Chimchar to 9 and learn Taunt
  • Taunt will work on enemies but in double battle you can taunt your ally and test a lot better
  • Try to use status moves and they should fail
  • You can also test that Taunt leaves on swap

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?

@mKroogz mKroogz changed the title initial commit of taunt implementation Taunt Implementation Jun 12, 2024
Comment on lines +123 to +124
this.summonData.isTaunted = source.summonData.isTaunted;
this.summonData.tauntedTurns = source.summonData.tauntedTurns;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you're going to be keeping track of turns like this then Taunt might be better implemented as a BattleTag or StatusEffect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooooooh ok! I'll Iook into it right away :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had used Disable as a guideline since it was similar :)

@Madmadness65 Madmadness65 added the Move Affects a move label Jun 13, 2024
@zacharied
Copy link
Contributor

zacharied commented Jun 13, 2024

Please uncheck "There is no overlap with another PR". There is one PR that already does this (#1153) and another that is directly related to it #2051.

@mKroogz
Copy link
Contributor Author

mKroogz commented Jun 13, 2024

See #2051, #2159.

Should I wait for #2051 and adapt my branch around it?

@zacharied
Copy link
Contributor

zacharied commented Jun 13, 2024

@mKroogz, sorry, I updated that comment, check it again. I'm not entirely sure what should be done about this, as neither PRs have any comments from devs.

@mKroogz
Copy link
Contributor Author

mKroogz commented Jun 13, 2024

@mKroogz, sorry, I updated that comment, check it again. I'm not entirely sure what should be done about this.

@zacharied I read updated comment, unchecked, and now I'm also unsure 🤔

A lot of my logic was based on how disable was handled especially in the isUsable method and just made a change in approach for what moves can't be used.

@zacharied
Copy link
Contributor

I'd say feel free to leave it as an example of an alternate approach, and a dev can choose which to merge. I don't know if my solution will get merged in the end either.

}

getTriggerMessage(pokemon: Pokemon, abilityName: string, ...args: any[]): string {
return getPokemonMessage(pokemon, `'s ${abilityName} prevented it from being Taunted!`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

use getPokemonWithAffix here please and make this localizable obviously

target.summonData.isTaunted = true;
target.summonData.tauntedTurns = 3;

user.scene.queueMessage(getPokemonMessage(target, ` fell for the taunt!`));
Copy link
Collaborator

Choose a reason for hiding this comment

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

needs to be localizable

@@ -2549,6 +2555,9 @@ export class MovePhase extends BattlePhase {
if (this.move.moveId && this.pokemon.summonData?.disabledMove === this.move.moveId) {
this.scene.queueMessage(`${this.move.getName()} is disabled!`);
}
else if (this.move.moveId && this.pokemon.summonData?.isTaunted) {
this.scene.queueMessage(`${this.pokemon.name} can't use\n${this.move.getName()} after the taunt!`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

localizable please

@torranx
Copy link
Collaborator

torranx commented Aug 10, 2024

Closing this in favor of #2051

@torranx torranx closed this Aug 10, 2024
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.

6 participants