[fixed]Hapatra, Vizier of Poisons trigger if opponent puts

[Aswan jaguar]

Describe the Bug:
Hapatra, Vizier of Poisons triggers also if opponent puts a -1/-1 counter on a creature.

Which card did behave improperly?
Hapatra, Vizier of Poisons

Which update are you using? (date, name)Which type? (duel, gauntlet, sealed deck)
June 2020 - Amonkhet: The Return of the Mummies version 3 - duel

What exactly should be the correct behavior/interaction?
Hapatra, Vizier of Poisons: Whenever you put one or more -1/-1 counters on a creature.

Are any other cards possibly affected by this bug?
Nest of Scarabs that I have just committed.
Defiant Greatmaw that is a bit different as it triggers " Whenever you put one or more -1/-1 counters on Defiant Greatmaw " seems to work correctly even if it is for not the right way to do it.

[drool66]

The problem is for some reason the caller isn't stored in add_counters(), only the target and the number and type of counters; and by the time the counters hit the card the activation, trigger, or spell is off the stack so we can't find it there. The way to fix these two cards is to add the calling player to the arguments of the function and its derivatives, and trigger an event with attacker. This is what Shandalar does. This would require among other things a search and replace across the card & function files - looks like somewhere in the order of of 100-200 replacements.

[Aswan jaguar]

The problem is for some reason the caller isn't stored in add_counters(), only the target and the number and type of counters; and by the time the counters hit the card the activation, trigger, or spell is off the stack so we can't find it there. The way to fix these two cards is to add the calling player to the arguments of the function and its derivatives, and trigger an event with attacker.

Why not just make a new function? Is that too expensive by means of new xtrigger address?

[drool66]

That's what I think we need to do in order to fix it - what kind of function were you thinking, and how would it get the parameter of who placed the counters?
My reasoning is that we need an event with attacker to carry the card that gets the counters as affected_card/_controller, the number of counters added stored in a special global value counters_added, and the player/card that put the counters there as attacking_card/_controller (see EVENT_COUNTERS, which is unique to Shandalar). I'm not 100% sure, but I don't think triggers can carry an attacking_card/_controller. We then have Hapatra & Nest of Scarabs look for that event & check if attacking_card_controller == player. Easy, except that in order to get & set the parameters for the event, it has to be an argument of the function and for some reason, unlike almost every other function in Manalink, add_counters does not have player, card as arguments (well, it does, but they're the target player & card).
We have about 170 open slots for events safely, but theoretically there are another, I think, 3,840 (ie. if we can use the HIWORD - we'll see if we ever get there). I don't know how high XTRIGGERS can go but if it's 01FFFF00 like it looks, we have about 65,000 open slots. The limitation is probably in the enable_xtrigger_flags.

Total replacements (more than I thought):
add_counters(): 80
remove_counters(): 52
add_counter(): 220
remove_counter(): 77
add_1_1_counter(): 247
add_1_1_counters(): 181
remove_1_1_counter(): 23
remove_1_1_counters(): 18
add_minus1_minus1_counters(): 57

I think that's all we need to change, plus the declarations in manalink.h and the functions themselves of course. A search & replace would proably work, although we need to be careful because of "ACT_ADD_COUNTERS" which is a mode of manipulate_all/_type. I don't think it would mess with the cards that use them because player, card is always the calling player & card.

[drool66]

Fixed in 7f7b368. I had missed the following that needed to be updated as well:

add_counters_predoubled()
copy_counters()
remove_all_counters()
move_counters()

I used EVENT_COUNTERS with attackers, but I should probably have used an XTRIGGER so there can be some interaction