fix]Garruk Relentless fight ability vs protection from black

[gnomefry]

Describe the Bug:

Garruk Relentless // Garruk, the Veil-Cursed 3 damage 'fight' ability can target but not damage Whirling Dervish. I'm guessing this bug is related to the fact that Garruk is a double-sided Innistrad card and his second side, Garruk, the Veil Cursed, is black.

Which update are you using? (date, name)Which type? (duel, gauntlet, sealed deck)

Holidays 2021 ad1a6f8 - gauntlet

What exactly should be the correct behavior/interaction?

My understanding is that Garruk Relentless targets, delivers and receives the fight damage while still being green. It is the post-damage-prevention condition of having received damage and going below 3 loyalty counters which flips the card to the black Garruk version.

Are any other cards possibly affected by this bug?

Other protection from black creatures.

[Korath]

I can't conveniently load your savegame, and it seems to work for me. I do tend towards believing you, though, for at least two reasons:

• It deals damage normally to the creature, but just immediately removes loyalty counters from itself. If either player's able to activate or cast anything during the damage prevention step, Garruk will flip before the damage resolves. ... though the damage card itself should still be green-only, and protection should only be looking at a damage source's color as damage is dealt.
• The card's artificially stupid. If player 1 (not necessarily the AI, so it breaks multiplayer too) controls it, any creature with power at least equal to Garruk's loyalty is forced to be an invalid target - it's incapable of trading even with something like a 20/1 Coiling Woodworm, or if it's indestructible, or if the AI has plenty of mana and another copy in its hand, etc. If the fight-like ability had a loyalty cost, or something removed counters in response to activation, that could easily produce your symptoms: it'd have enough loyalty counters to target at announcement, but not enough by resolution, so targeting would fail and the effect would fizzle.

[Aswan jaguar]

I confirm with your savegame and in a clean game but only on release. In dev I can't reproduce the bug, so this seems to be fixed.

[Korath]

It was commit 0bbe1c935 that mostly fixed it, very likely the change in effect_damage() from "is_protected_from(instance->damage_target_player, instance->damage_target_card, instance->damage_source_player, instance->damage_source_card, IPF_DAMAGE)". I think what happened is that the unconditional get_abilities() call in is_protected_from() eventually resulted in an EVENT_STATIC_EFFECTS, which flips Garruk before the damage card finishes processing. That'd be enough for my first scenario above to break things - and I see effect_damage() still looks at the color and types of the damage source directly for protections implemented through keyword_t bits, just like in the original MicroProse version. So it's likely this is still broken, just not happening every time.

So what needs to be done is

1. Make Garruk Relentless be dealt damage by the creature instead of removing loyalty counters. This will also make things work right if Garruk gains protection, or you want to prevent the damage at instant speed, or whatever.
2. Make the keyword_t protection bits all look at the damage card, not its source.
3. Remove the heavy-handed target munging. If the AI starts making Garruk suicide against too-weak creatures, then hint it with ai_modifier. There's times when that won't work (usually after a random event like a coin flip or die roll, or after a human player choice without an intervening speculation point), but this is unlikely to be one.

[drool66]

Make the keyword_t protection bits all look at the damage card, not its source.

I believe this was fixed in 2ce322b (2/4/22), correct?

[Aswan jaguar]

1 and 3 bugs are fixed in commit 6ef719f. So if 2nd was fixed in commit mentioned by drool66, everything is fixed.

[Korath]

I believe this was fixed in 2ce322b (2/4/22), correct?

Yes - I was trying to find the exact way 0bbe1c935 kept this bug from manifesting (so we'd know for sure whether it actually fixed it or just masked the symptoms better) and must have still had an old version checked out when I looked at these bits.