Page 1 of 1

[confirmed]Lumithread Field not removed from combat

PostPosted: 03 Feb 2017, 08:19
by Korath
Describe the Bug:
If you unmorph an attacking (or presumably blocking; I didn't test) Lumithread Field, it stays in combat. Or at least in the combat window.

Which card behaved improperly?
Lumithread Field.

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

What exactly should be the correct behavior/interaction?
506.4. A permanent is removed from combat if it leaves the battlefield, if its controller changes, if it phases out, if an effect specifically removes it from combat, if it's a planeswalker that's being attacked and stops being a planeswalker, or if it's an attacking or blocking creature that regenerates (see rule 701.13) or stops being a creature. A creature that's removed from combat stops being an attacking, blocking, blocked, and/or unblocked creature. A planeswalker that's removed from combat stops being attacked.

Are any other cards possibly affected by this bug?
Zoetic Cavern, no doubt; and Whetwheel would if it were in Manalink.

....do we know how to get a creature out of the combat window, short of obliterating and re-creating it? Removing STATE_ATTACKING|STATE_BLOCKING and setting blocking to -1, like in regenerate_target_exe(), isn't working.

Re: [confirmed]Lumithread Field not removed from combat

PostPosted: 20 Feb 2017, 20:33
by Korath
There's a couple of weirdnesses conspiring here to keep this from working right.

The main problem is that turning a card face-up doesn't check to see if the card's a creature afterward and remove it from combat if it isn't. That part isn't weird, just a plain omission of the rule.

The weird part is that cards with STATE_ATTACKED (which we've always meant to mean "this card attacked this turn, but isn't currently attacking") are always drawn in the combat window as if they're still attacking. The reason why things appear to work right with regeneration, but not anything else that removes a creature from combat, is that STATE_ATTACKED and STATE_BLOCKED don't get added to a regenerating card unless it regenerates during first strike or normal combat damage resolution. (remove_from_combat() always adds the bits; it's only the version of remove_from_combat that's inlined in the body of regenerate_target() that sometimes doesn't.) (And also note that STATE_BLOCKED means "this card blocked another this turn", not "this card is currently blocked", which is STATE_ISBLOCKED.)

So if you Lightning Bolt an Uthden Troll before declaring blockers, it no longer shows up as attacking; but it'll cause problems with things that check to see whether they attacked or not. (For example, if you destroy a transformed Civilized Scholar // Homicidal Brute after it attacks but before any combat damage is dealt, and then regenerate it with say Death Ward, it'll think it didn't attack at end of turn.) This isn't just a case of us misinterpreting what the STATE_ATTACKED bit is supposed to do, though, since the original (and current?) version of Siren's Call is broken too.

I think the original idea was so that when a defending creature regenerates during damage resolution, it stays in the combat window until the end of combat so the creature it was blocking is distinguishable from an unblocked creature. But that hardly seems worth keeping and working around the ancillary bugs with STATE_ATTACKED, since combat is about to end anyway.

So - the way to fix everything here is to
  1. In flip_card(), after updating to the new type, check to see if the card's still a creature and call remove_from_combat() on it if not.
  2. Fix regenerate_target so it always adds STATE_ATTACKED and STATE_BLOCKED. While you're at it, it should only check the card it's regenerating for STATUS_CANNOT_REGENERATE, instead of searching for effect cards attached to it with STATUS_CANNOT_REGENERATE set, since we don't use the latter method anymore. The version from shandalar/cards/regenerate.cpp should work in Manalink without modification.
  3. Change the combat window's function so it no longer considers STATE_ATTACKED enough (by itself) to draw a card in it, instead of in its controller's territory window. That's done in wndproc_AttackClass(), in its handler of WM_USER+0x12; there's two places that look like "|| state & STATE_ATTACKED" that can just be changed to "|| state & 0". - Never mind, someone's already done it in Manalink, just not in Shandalar.
#3 is trivial to do if you have a working decompilation database for magic.exe, which I no longer do. But #1 and #2 should be done regardless, since they cause actual wrong behavior instead of just visual anomalies.

Re: [confirmed]Lumithread Field not removed from combat

PostPosted: 02 Nov 2019, 20:30
by Aswan jaguar
Fixed flip_card(), in commit 7068d3fa. The other point seems more difficult since manalink's regenerate_target points to exe version and I don't understand how shandalar's version checks for STATE_ATTACKED and STATE_BLOCKED.