Several abilities are not properly handled

[mingu600]

I believe that the engine is not properly handling several abilities, such as Defiant, Competitive, and Weak Armor. In modify_attack_against.py, we seem to trigger if the attacking move has any kinds of boosts, and then additionally boosts the attacker. For example, if my Slowbro uses Calm Mind against Defiant Bisharp, the function sees that Calm Mind gives boosts to the attacker, and then gives Slowbro +2 in attack. This is incorrect in numerous ways; Defiant should only work on negative boosts to the defending Pokemon, and then the defending Pokemon with Defiant should be the one to get an attack boost. Competitive is also similarly handled wrong. Just from checking, it seems that Weak Armor is also programmed in a similar way.

Also, should Clear Body/White Smoke/other negative boost resistant abilities be handled in this file as well? I haven't confirmed, but it seems like these abilities aren't being handled anywhere, except for the one case of Intimidate on switch-in.

[mingu600]

Here's my proposal for a change for Defiant, for example:

def defiant(attacking_move, attacking_pokemon, defending_pokemon):
    attacking_move = attacking_move.copy()
    if attacking_move['target'] == 'normal':
        if constants.BOOSTS in attacking_move:
            attacking_move[constants.BOOSTS] = attacking_move[constants.BOOSTS].copy()
            for boost in attacking_move[constants.BOOSTS].copy():
                if attacking_move[constants.BOOSTS][boost] < 0:
                    if constants.ATTACK not in attacking_move[constants.BOOSTS]:
                            attacking_move[constants.BOOSTS][constants.ATTACK] = 2
                    else:
                        attacking_move[constants.BOOSTS][constants.ATTACK] += 2
        elif attacking_move[constants.SECONDARY] and constants.BOOSTS in attacking_move[constants.SECONDARY]:
            attacking_move[constants.SECONDARY] = attacking_move[constants.SECONDARY].copy()
            attacking_move[constants.SECONDARY][constants.BOOSTS] = attacking_move[constants.SECONDARY] [constants.BOOSTS].copy()
            for boost in attacking_move[constants.SECONDARY][constants.BOOSTS].copy():
                if attacking_move[constants.SECONDARY][constants.BOOSTS][boost] < 0:
                    if constants.ATTACK not in attacking_move[constants.SECONDARY][constants.BOOSTS]:
                        attacking_move[constants.SECONDARY][constants.BOOSTS][constants.ATTACK] = 2
                    else:
                        attacking_move[constants.SECONDARY][constants.BOOSTS][constants.ATTACK] += 2
    return attacking_move

[pmariglia]

Wow thanks for catching this @mingu600!

Yeah as is evidence by that atrocious code, some parts of the engine are completely spaghetti and can lead to ugly situations like this. The real problem is the handling of things like the side-effects of boosts should be done in some sort of callback handler, but that would require a larger re-design of the engine.

I try to keep tests for the battle mechanics as much as possible, but clearly I was missing these edge cases.

I've added a fix for defiant and competitive here based on your suggestion: 70635fd55aa46ae42749dfbf65713600abd1c969.

I do not see anything wrong with weakarmor - could you elaborate on that one?

clearbody, whitesmoke, and fullmetalbody are in a constant variable which is used elsewhere in the engine: https://github.com/pmariglia/showdown/blob/70635fd55aa46ae42749dfbf65713600abd1c969/constants.py#L360-L364

^ There are problems with that logic too, it isn't perfect. I try not to prioritize the edgiest of edge-cases, but it's something I'll be sure to make a note of.

[mingu600]

No worries, I was just super confused why my Slowbro kept getting +4 attack boosts :P

Forgive me if I'm wrong, but the way weak armor is currently written, isn't the attacker getting the weak armor stat changes, and not the defender?

[pmariglia]

No worries, I was just super confused why my Slowbro kept getting +4 attack boosts :P

Forgive me if I'm wrong, but the way weak armor is currently written, isn't the attacker getting the weak armor stat changes, and not the defender?

I dont think so? Check out this test: https://github.com/pmariglia/showdown/blob/70635fd55aa46ae42749dfbf65713600abd1c969/tests/test_battle_mechanics.py#L6846-L6865

The attacker (the bot) is using tackle, and the defender (the opponent) takes damage, loses defense, and gains speed.

The module where weakarmor is defined is modify_attack_against.py. Anything changed in there is changing a move that is being used on a pokemon. In the case of weakarmor, it is changing the tackle to include the defense drop and the speed boost.

[mingu600]

Sounds like it's working properly! I'll close the issue