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

Adds Commander and Order Up #5246

Merged
merged 27 commits into from
Sep 29, 2024
Merged

Conversation

AlexOn1ine
Copy link
Collaborator

Closes #2764
Closes #2490

I'll do clean up once the review is done.
Also this will need proper in-game testing

@AlexOn1ine AlexOn1ine added the category: ability Pertains to abilities label Aug 23, 2024
@hedara90
Copy link
Collaborator

hedara90 commented Aug 24, 2024

Problems with the implementation.

  • AI will attack the Tatsugiri despite not being "on the field"
  • If Tatsugiri would act after a Pokemon that knocks out Dondozo, it will attack the opponent in front of them (or diagonal if none is in front) with the move in the first slot.
  • If Tatsugiri is in the right slot, it will take damage from AoE moves after exiting Dondozo if Dondozo faints to the move.
slowTatsu.mp4

@hedara90
Copy link
Collaborator

Things that needs testing for how they should work:

  • Does Ditto Transformed into Tatsugiri trigger Commander?
    Currently it does.
  • If Dondozo is in the semi-invulnerable turn of Dive when Tatsugiri switches in, what happens?
    Currently Commander triggers normally.
  • If Tatsugiri is in the semi-invulnerable turn of Dive when Dondozo switches in, what happens? (Not actually possible in SV)
    Currently Commander triggers normally.
  • What happens if Tatsugiri has a Substitute up before being swallowed?
    Currently Substitute reappears when exiting Dondozo.

@AlexOn1ine
Copy link
Collaborator Author

* Does Ditto Transformed into Tatsugiri trigger Commander?
  Currently it does.

it has to be a non transformed user

* If Dondozo is in the semi-invulnerable turn of Dive when Tatsugiri switches in, what happens?
  Currently Commander triggers normally.

yes, commander is still triggered

* If Tatsugiri is in the semi-invulnerable turn of Dive when Dondozo switches in, what happens? (Not actually possible in SV)
  Currently Commander triggers normally.

didn't test but I assume it is the same as above

* What happens if Tatsugiri has a Substitute up before being swallowed?
  Currently Substitute reappears when exiting Dondozo.

tatsugiri keeps the sub

All according to showdown but I assume that this is correct since it is a very viable VGC mechanic.

@hedara90
Copy link
Collaborator

* What happens if Tatsugiri has a Substitute up before being swallowed?
  Currently Substitute reappears when exiting Dondozo.

tatsugiri keeps the sub

All according to showdown but I assume that this is correct since it is a very viable VGC mechanic.

I don't know if this is how it's supposed to be. I haven't found any example of this, and Substitute is not represented in the move% for Tatsugiri usage at any point in time.
The other things are correct.

@AlexOn1ine AlexOn1ine closed this Aug 25, 2024
@AlexOn1ine AlexOn1ine reopened this Aug 25, 2024
@AlexOn1ine
Copy link
Collaborator Author

this has a bug I can't solve. putting on draft for now

@AlexOn1ine AlexOn1ine marked this pull request as draft August 30, 2024 13:55
@hedara90
Copy link
Collaborator

this has a bug I can't solve. putting on draft for now

What's the bug?

@AlexOn1ine
Copy link
Collaborator Author

this has a bug I can't solve. putting on draft for now

What's the bug?

when tatsugiri switches in while an opposing mon attacks with a spread move the attacker and target are switched so it becomes and attack from dozo.

@hedara90
Copy link
Collaborator

Ah, yea, that bug...

@hedara90
Copy link
Collaborator

hedara90 commented Sep 1, 2024

Could the actions for the turn be redone/reassigned (whatever the term if) after Dozo switches in?

Copy link

@PhallenTree PhallenTree left a comment

Choose a reason for hiding this comment

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

when tatsugiri switches in while an opposing mon attacks with a spread move the attacker and target are switched so it becomes and attack from dozo.

Here are tests for this bug:

DOUBLE_BATTLE_TEST("(Commander) Attacker is kept (Dondozo Left Slot)")
{
    GIVEN {
        ASSUME(gMovesInfo[MOVE_SURF].target == MOVE_TARGET_FOES_AND_ALLY);
        PLAYER(SPECIES_WOBBUFFET);
        PLAYER(SPECIES_TATSUGIRI) { Ability(ABILITY_COMMANDER); }
        PLAYER(SPECIES_DONDOZO);
        OPPONENT(SPECIES_WOBBUFFET);
        OPPONENT(SPECIES_WOBBUFFET);
    } WHEN {
        TURN { MOVE(opponentRight, MOVE_TACKLE, target: opponentLeft); }
        TURN { SWITCH(playerLeft, 2); MOVE(opponentLeft, MOVE_SURF); }
    } SCENE {
        ANIMATION(ANIM_TYPE_MOVE, MOVE_TACKLE, opponentRight);
        ABILITY_POPUP(playerRight, ABILITY_COMMANDER);
        MESSAGE("Tatsugiri was swallowed by Dondozo and became Dondozo's commander!");
        ANIMATION(ANIM_TYPE_MOVE, MOVE_SURF, opponentLeft);
        HP_BAR(playerLeft);
        MESSAGE("Foe Wobbuffet's attack missed!");
        HP_BAR(opponentRight);
    }
}

DOUBLE_BATTLE_TEST("(Commander) Attacker is kept (Dondozo Right Slot)")
{
    GIVEN {
        ASSUME(gMovesInfo[MOVE_SURF].target == MOVE_TARGET_FOES_AND_ALLY);
        PLAYER(SPECIES_TATSUGIRI) { Ability(ABILITY_COMMANDER); }
        PLAYER(SPECIES_WOBBUFFET);
        PLAYER(SPECIES_DONDOZO);
        OPPONENT(SPECIES_WOBBUFFET);
        OPPONENT(SPECIES_WOBBUFFET);
    } WHEN {
        TURN { MOVE(opponentRight, MOVE_TACKLE, target: opponentLeft); }
        TURN { SWITCH(playerRight, 2); MOVE(opponentLeft, MOVE_SURF); }
    } SCENE {
        ANIMATION(ANIM_TYPE_MOVE, MOVE_TACKLE, opponentRight);
        ABILITY_POPUP(playerLeft, ABILITY_COMMANDER);
        MESSAGE("Tatsugiri was swallowed by Dondozo and became Dondozo's commander!");
        MESSAGE("Foe Wobbuffet's attack missed!");
        ANIMATION(ANIM_TYPE_MOVE, MOVE_SURF, opponentLeft);
        HP_BAR(playerRight);
        HP_BAR(opponentRight);
    }
}

Test 1 fails because "Dondozo's attack missed!" instead of "Foe Wobbuffet's attack missed!".
Test 2 fails because the animation for Surf is played on the player's side of the field.

Confirmed after the following changes both pass and none of the other tests fail (that didn't fail before).

Note that the Tatsugiri (the mon with Commander) is also gBattleScripting.battler therefore there is no need for both gBattlerAttacker and gBattlerTarget to be used. Since the swap of target and attacker is gone, the bug should be fixed.

src/battle_util.c Outdated Show resolved Hide resolved
data/battle_scripts_1.s Outdated Show resolved Hide resolved
data/battle_scripts_1.s Outdated Show resolved Hide resolved
@AlexOn1ine
Copy link
Collaborator Author

Note that the Tatsugiri (the mon with Commander) is also gBattleScripting.battler therefore there is no need for both gBattlerAttacker and gBattlerTarget to be used. Since the swap of target and attacker is gone, the bug should be fixed.

Right, I'm stupid. Thanks for the help! I'll update the branch later.

@AlexOn1ine AlexOn1ine marked this pull request as ready for review September 8, 2024 12:51
@AlexOn1ine
Copy link
Collaborator Author

Ready for review again.

Copy link
Collaborator

@hedara90 hedara90 left a comment

Choose a reason for hiding this comment

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

Adding this here as to not forget it

src/data/trainers.party Outdated Show resolved Hide resolved
src/data/trainers.h Outdated Show resolved Hide resolved
asm/macros/battle_script.inc Outdated Show resolved Hide resolved
data/battle_scripts_1.s Outdated Show resolved Hide resolved
src/battle_script_commands.c Outdated Show resolved Hide resolved
test/battle/ability/commander.c Outdated Show resolved Hide resolved
test/battle/ability/commander.c Outdated Show resolved Hide resolved
test/battle/ability/commander.c Outdated Show resolved Hide resolved
test/battle/ability/commander.c Outdated Show resolved Hide resolved
src/battle_main.c Outdated Show resolved Hide resolved
@AlexOn1ine
Copy link
Collaborator Author

I can't check how it's supposed to be in S/V, ...
..., Tatsugiri does not use the chosen move. Reading Bulbapedia it's supposed to cancel the move.

Test for this

DOUBLE_BATTLE_TEST("(Commander) Tatsugiri does not attack if Dondozo faints the same turn it's switched in")
{
    GIVEN {
        PLAYER(SPECIES_WOBBUFFET);
        PLAYER(SPECIES_TATSUGIRI) { Ability(ABILITY_COMMANDER); }
        PLAYER(SPECIES_DONDOZO) { HP(1); }
        OPPONENT(SPECIES_WOBBUFFET);
        OPPONENT(SPECIES_WOBBUFFET);
    } WHEN {
        TURN { SWITCH(playerLeft, 2); MOVE(opponentLeft, MOVE_TACKLE, target: playerLeft); MOVE(opponentRight, MOVE_CELEBRATE, target: playerRight); MOVE(playerRight, MOVE_CELEBRATE); SEND_OUT(playerLeft, 0); }
    } SCENE {
        MESSAGE("Dondozo fainted!");
        NOT MESSAGE("Tatsugiri used Celebrate!");
    }
}

fixed

Ready for re-review!

@hedara90
Copy link
Collaborator

Maybe also add some Dragon Darts + Commander tests.

Dragon Darts is broken currently.

dartsCommands.mp4

@hedara90
Copy link
Collaborator

Maybe also add some Dragon Darts + Commander tests.

DOUBLE_BATTLE_TEST("(Commander) Tatsugiri does not get hit by Dragon Darts when commanding Dondozo")
{
    GIVEN {
        ASSUME(gMovesInfo[MOVE_DRAGON_DARTS].effect == EFFECT_DRAGON_DARTS);
        PLAYER(SPECIES_TATSUGIRI) { Ability(ABILITY_COMMANDER); }
        PLAYER(SPECIES_DONDOZO);
        OPPONENT(SPECIES_WOBBUFFET);
        OPPONENT(SPECIES_WYNAUT);
    } WHEN {
        TURN { MOVE(opponentRight, MOVE_DRAGON_DARTS, target: playerRight); }
    } SCENE {
        MESSAGE("Tatsugiri was swallowed by Dondozo and became Dondozo's commander!");
        ANIMATION(ANIM_TYPE_MOVE, MOVE_DRAGON_DARTS, opponentRight);
        NOT HP_BAR(playerLeft);
    }
}

@AlexOn1ine
Copy link
Collaborator Author

Added Dragon Darts test made by hedara. One of them doesn't pass because because of incomplete turn but works fine in game. I'm not sure what is up there.

@AlexOn1ine
Copy link
Collaborator Author

hedara fixed the test!

Ready for review

Copy link
Collaborator

@AsparagusEduardo AsparagusEduardo left a comment

Choose a reason for hiding this comment

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

.

include/battle_script_commands.h Outdated Show resolved Hide resolved
src/battle_main.c Outdated Show resolved Hide resolved
src/battle_script_commands.c Outdated Show resolved Hide resolved
src/battle_script_commands.c Outdated Show resolved Hide resolved
src/battle_script_commands.c Outdated Show resolved Hide resolved
test/battle/ability/commander.c Outdated Show resolved Hide resolved
test/battle/ability/commander.c Outdated Show resolved Hide resolved
test/battle/ability/commander.c Outdated Show resolved Hide resolved
test/battle/ability/commander.c Outdated Show resolved Hide resolved
test/battle/ability/commander.c Outdated Show resolved Hide resolved
test/battle/ability/commander.c Show resolved Hide resolved
test/battle/ability/commander.c Show resolved Hide resolved
src/battle_script_commands.c Show resolved Hide resolved
test/battle/hold_effect/eject_pack.c Outdated Show resolved Hide resolved
@AlexOn1ine
Copy link
Collaborator Author

Ready for re-review

Copy link
Collaborator

@AsparagusEduardo AsparagusEduardo left a comment

Choose a reason for hiding this comment

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

lgtm. just needs hedara's ok.

@AlexOn1ine
Copy link
Collaborator Author

lgtm. just needs hedara's ok.

please wait for Phallen to give the okay as well

@PhallenTree
Copy link

Looks good to me!

@hedara90
Copy link
Collaborator

Hopefully the last edge case.
Order Up should always be boosted by Sheer Force, but the Stat increase when Commander is active should also always apply when Sheer Force is boosting Order up.
I'm writing tests for this and will PR them to you.

Copy link
Collaborator

@hedara90 hedara90 left a comment

Choose a reason for hiding this comment

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

Looks good now, all tests passed locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: ability Pertains to abilities category: move effect Pertains to move effects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants