[fixed] Lignify doesn't immediately update at resolution

[Korath]

Describe the Bug:
Lignify almost works, but cards on the battlefield - whether the enchanted one or ones affected by the enchanted one losing abilities - aren't updated until something else happens. (Plus, extra copies are created in the graveyard at resolution, with SACRFICE.WAV playing. A variant of this bug, it seems.) Screenshot 1 is immediately after letting the third Lignify of the game, enchanting Lord of Atlantis, resolve; drawing a card or playing a Plains or most anything else updates them to screenshot 2.

Also, if I Terror the enchanted creature, Lignify stays on the bf. Yick. Still gonna says it "almost works", though, since I expect that's a consequence of the bug first reported for Oko.

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

What exactly should be the correct behavior/interaction?
Screenshot 2 immediately after resolving, not waiting for another call to recalculate_all_cards_in_play(). Also, no extra copies or irritating sounds, and the aura dies when the object it's attached to leaves the bf.

Are any other cards possibly affected by this bug?
I can't find any fully-working auras with "Enchanted creature.*loses all abilities", so yes. But not this precise variant.

[drool66]

fixed in d693305

[Aswan jaguar]

Dev 78826f.
I cast Lignify in AI's Abattoir Ghoul then I cast Confiscate on the creature I got the creature but Lignify went to graveyard. It shouldn't leave the battlefield of course.

[Korath]

It doesn't do anyone any good to mention unpushed commit hashes. Only you can see them, and they'll change if you have to rebase before pushing because someone else pushed first.

But yes, this happens in d693305 too. It doesn't seem to be a problem in aura(), since transferring control of a creature enchanted by Armor of Thorns or Daybreak Coronet doesn't kill the enchantment. (It doesn't for Emblem of the Warmind, either, though it should.) Probably not attached_is_forced_subtype() either, since Call to Serve, using the similar attached_is_additional_subtype(), works.

Edit: Nope, it's aura(). Adding a forcible breakpoint when a Lignify is put into a graveyard gives me:

backtrace | Open

Code: Select all

Thread 13 received signal SIGTRAP, Trace/breakpoint trap.
[Switching to Thread 704.0x828]
0x025eb139 in raw_put_iid_on_top_of_graveyard_impl (player=player@entry=0, iid=iid@entry=5613,
    position_hint=position_hint@entry=0, source_val=16777736) at functions/deck.c:1397
1397    {if (ai_is_speculating != 1 && iid != -1 && cards_data[iid].id == CARD_ID_LIGNIFY) { __asm__("int $3"); }
(gdb) bt
#0  0x025eb139 in raw_put_iid_on_top_of_graveyard_impl (player=player@entry=0, iid=iid@entry=5613, position_hint=position_hint@entry=0, source_val=16777736) at functions/deck.c:1397
#1  0x025eb77c in raw_put_iid_on_top_of_graveyard_with_triggers (player=0, iid=5613, player_while_instantiated=<optimized out>, card_while_instantiated=8) at functions/deck.c:1462
#2  0x00477c62 in kill_card_guts () at Magic2.asm:76070
#3  0x025ed34a in kill_card_impl (player=0, card=card@entry=8, kill_mode=<optimized out>, kill_mode@entry=KILL_STATE_BASED_ACTION, flags=0) at functions/deck.c:1199
#4  0x025ed5c1 in kill_card_impl (flags=0, kill_mode=KILL_STATE_BASED_ACTION, card=8, player=<optimized out>) at functions/deck.c:1205
#5  0x025b03c0 in aura_impl (player=0, card=8, event=EVENT_STATIC_EFFECTS, td=0x1ab5f22c, prompt=0x26792a0 <arr_from_hardcoded_subtype_to_subtype+960> "TARGET_CREATURE", literal_prompt=0) at functions/auras.c:337
#6  0x025b18f7 in aura (player=<optimized out>, player@entry=0, card=<optimized out>, card@entry=8, event=<optimized out>, event@entry=EVENT_STATIC_EFFECTS, td=td@entry=0x0, prompt=prompt@entry=0x0) at functions/auras.c:427
#7  0x024ddb72 in card_lignify (player=0, card=8, event=EVENT_STATIC_EFFECTS) at cards/lorwyn.c:4103
#8  0x02623855 in call_card_fn_impl () from G:\home\dgk\pristine\master\ManalinkEh.dll
#9  0x025b553c in call_card_fn (address=address@entry=0x2007a0a <CodeSectionEx+27140>, instance=0x4efea0 <sub_4E9564+26940>, player=player@entry=0, card=card@entry=8, event=event@entry=EVENT_STATIC_EFFECTS) at functions/events.c:51
#10 0x025b55ba in call_card_function (player=player@entry=0, card=card@entry=8, event=event@entry=EVENT_STATIC_EFFECTS) at functions/events.c:19
#11 0x025b5ba2 in dispatch_event_arbitrary_to_one_card (player=player@entry=0, card=card@entry=8, event=event@entry=EVENT_STATIC_EFFECTS, new_affected_card_controller=new_affected_card_controller@entry=0, new_affected_card=new_affected_card@entry=8, new_attacking_card_controller=new_attacking_card_controller@entry=1, new_attacking_card=new_attacking_card@entry=-1) at functions/events.c:237
#12 0x025b5c05 in dispatch_event_with_attacker_to_one_card (new_affected_card_controller=new_affected_card_controller@entry=0, new_affected_card=new_affected_card@entry=8, event=event@entry=EVENT_STATIC_EFFECTS, new_attacking_card_controller=new_attacking_card_controller@entry=1, new_attacking_card=new_attacking_card@entry=-1) at functions/events.c:214
#13 0x025e7cd1 in recalculate_all_cards_in_play () at functions/engine.c:215
#14 0x00477d81 in kill_card_guts () at Magic2.asm:76070
#15 0x025ed34a in kill_card_impl (player=1, card=card@entry=8, kill_mode=<optimized out>, kill_mode@entry=KILL_EXILE, flags=1) at functions/deck.c:1199
#16 0x025edc11 in kill_card_impl (flags=1, kill_mode=KILL_EXILE, card=8, player=<optimized out>) at functions/deck.c:1210
#17 0x0259f564 in real_switch_control (player=player@entry=1, card=card@entry=8) at functions/gain_control_and_legend_rule.c:167
#18 0x025a02b7 in gain_control_permanently_impl (player=0, t_player=1, t_card=8, sound=1) at functions/gain_control_and_legend_rule.c:414
#19 0x025a0447 in gain_control_permanently (player=player@entry=0, t_player=t_player@entry=1, t_card=t_card@entry=8) at functions/gain_control_and_legend_rule.c:428
#20 0x025b2265 in gain_control_and_attach_as_aura (player=0, card=10, event=EVENT_RESOLVE_SPELL, t_player=1, t_card=8) at functions/auras.c:678
#21 0x025b2640 in generic_stealing_aura (player=player@entry=0, card=card@entry=10, event=event@entry=EVENT_RESOLVE_SPELL, td=td@entry=0x1ab5f518, prompt=prompt@entry=0x2645778 "TARGET_PERMANENT") at functions/auras.c:744
#22 0x0203c169 in card_confiscate (player=0, card=10, event=EVENT_RESOLVE_SPELL) at cards/urza_saga.c:1040
#23 0x02623855 in call_card_fn_impl () from G:\home\dgk\pristine\master\ManalinkEh.dll
#24 0x025b553c in call_card_fn (address=0x200cbd6 <CodeSectionEx+48080>, instance=0x4f00f8 <sub_4E9564+27540>, player=player@entry=0, card=card@entry=10, event=event@entry=EVENT_RESOLVE_SPELL) at functions/events.c:51
#25 0x025b5cc9 in call_card_function_i (event=EVENT_RESOLVE_SPELL, card=10, player=0, instance=<optimized out>) at functions/events.c:30
#26 dispatch_event_to_single_card (player=0, card=10, event=EVENT_RESOLVE_SPELL, new_attacking_card_controller=1, new_attacking_card=-1) at functions/events.c:271
#27 0x004368c6 in resolve_top_card_on_stack () at Magic2.asm:41793
#28 0x025ea0a9 in put_card_on_stack3 (player=0, card=10) at functions/engine.c:2901
#29 0x0043bfaa in main_phase () at Magic2.asm:45732
#30 0x004399bd in run_turn () at Magic2.asm:44078
#31 0x0047902c in start_duel () at Magic2.asm:76779
#32 0x004946e9 in start_duel_thread () at Magic2.asm:89827
#33 0x75ccfa29 in KERNEL32!BaseThreadInitThunk () from C:\WINDOWS\SysWOW64\kernel32.dll
#34 0x77b47a9e in ntdll!RtlGetAppContainerNamedObjectPath () from C:\WINDOWS\SysWOW64\ntdll.dll
#35 0x77b47a6e in ntdll!RtlGetAppContainerNamedObjectPath () from C:\WINDOWS\SysWOW64\ntdll.dll
#36 0x00000000 in ?? ()
(gdb) frame 5
#5  0x025b03c0 in aura_impl (player=0, card=8, event=EVENT_STATIC_EFFECTS, td=0x1ab5f22c, prompt=0x26792a0 <arr_from_hardcoded_subtype_to_subtype+960> "TARGET_CREATURE", literal_prompt=0) at functions/auras.c:337
337                                     kill_card(player, card, KILL_STATE_BASED_ACTION);
(gdb) l
332
333             if( in_play(player, card) && instance->damage_target_player > -1 ){
334
335                     if (event == EVENT_STATIC_EFFECTS && affect_me(player, card)){
336                             if( !valid_target(td) )
337                                     kill_card(player, card, KILL_STATE_BASED_ACTION);
338                     }
339
340
341                     if( event == EVENT_ABILITIES && affect_me(player, card)){
(gdb) mlgci 0 8
$1 = {special_counters = 0 '\000', counters2 = 0 '\000', counters3 = 0 '\000', counters4 = 0 '\000',
  damage_target_card = 10, state = 196738, damage_source_player = -1 '\377', unused0 = 0 '\000', toughness = 0,
  damage_on_card = 0, counter_power = 0, unknown0x14 = 0, token_status = 128, counter_toughness = 0, color = 8 '\b',
  destroys_if_blocked = 0 '\000', dummy3 = 0, blocking = 255 '\377', initial_color = 0 '\000', unused0x26 = 0,
  regen_status = 100663296, mana_to_untap = "\000\000\000\000\000\000\000", power = 0, number_of_targets = 0 '\000',
  unknown0x37 = 0 '\000', info_slot = 0, original_internal_card_id = 5613, color_id = "\000\000\000\000\000",
  backup_internal_card_id = 5613, damage_source_card = -1, eot_toughness = 0, damage_target_player = 0 '\000',
  special_counter_type = 255 '\377', unk52 = 255 '\377', unk53 = 255 '\377', timestamp = 18, mana_color = 8 '\b',
  card_color = 8 '\b', unk5a = 0 '\000', unk5b = 0 '\000', upkeep_flags = 0, attack_rating = 9,
  display_pic_csv_id = 0, display_pic_num = 0, kill_code = 3 '\003', unk69 = 0 '\000', unk6A = 0 '\000',
  unk6B = 0 '\000', internal_card_id = 5613, unknown0x70 = 0, targets = {{player = 1, card = 8}, {player = -1,
      card = -1} <repeats 13 times>, {player = -1, card = 33556480}, {player = 0, card = -1}, {player = -1, card = 0},
    {player = -1, card = -127}, {player = -1, card = -1}}, parent_controller = -1, parent_card = -1,
  hack_mode = "\000\000\000\000\000", unknown0x11a = 0, untap_status = 0, counters = 0 '\000', counters5 = 0 '\000',
  unknown0x122 = 0, upkeep_colorless = 0 '\000', upkeep_black = 0 '\000', upkeep_blue = 0 '\000',
  upkeep_green = 0 '\000', upkeep_red = 0 '\000', upkeep_white = 0 '\000', upkeep_artmana = 0 '\000',
  counters_m1m1 = 0 '\000'}


For reference, mlgci in my .gdbinit is

Code: Select all

define mlgci
  p (((card_instance_t (*)[151])(0x4ef540)) [$arg0][$arg1])
end
document mlgci
Manalink get_card_instance().  Usage: "mlgci 0 14"
end

valid_target() tries to validate whatever's in targets[0]. That's not necessarily the same as what the aura is attached to, which is {aura->damage_target_player,aura->damage_target_card}. Here targets[0]={1,8} (the Abattoir Ghoul when it was under the AI's control), and damage_target_*={0,10} (the Abattoir Ghoul under the human's control).

(Also, you'll need to make it ignore shroud and hexproof, and - if they deal with it during targeting - Bartel Runeaxe's and Tetsuo Umezawa's "can't be the target of Aura spells". And I suppose Raiding Party and Spectral Shield and Suq'Ata Firewalker and so on - anything saying "can't be the target of" without saying "can't be enchanted by" or "has protection from" - if they're in Manalink. Setting td->illegal_abilities = 0 is not the right way to do it, not here and not earlier in aura() for SF3_IN_PLAY_DIRECTLY - that breaks Roots and Trapped in the Tower and so on.)

What I don't get is why it's breaking for Lignify and not for anything else.

[Aswan jaguar]

Anything that uses both disable_attached() & disabling_aura_ai()
has the same issue.

[drool66]

valid_target() tries to validate whatever's in targets[0]. That's not necessarily the same as what the aura is attached to, which is {aura->damage_target_player,aura->damage_target_card}.

Yep, I failed to clean up this piece before I pushed. I've already changed it to:

Code: Select all

         if( ! would_validate_arbitrary_target(td, instance->damage_target_player, instance->damage_target_card) )
            kill_card(player, card, KILL_STATE_BASED_ACTION);


What I don't get is why it's breaking for Lignify and not for anything else.

I would think it's because I check it in EVENT_STATIC_EFFECTS, which isn't an IS_AURA_EVENT - another thing I should have cleaned up. I think EVENT_ABILITIES is probably better.

By the way, many auras are also crashing for AI due to the AI snippet in pump_attached() by checking targets[0] when this function is called before calling aura_impl() on EVENT_CAST_SPELL. Switched to checking during EVENT_RESOLVE_SPELL, when targets[0] should still be valid.

[Korath]

EVENT_STATIC_EFFECTS is as close to the right place as we've got - still too often for the game-level concept of state-based actions, but not by a whole lot. EVENT_ABILITIES can be called from anywhere get_abilities() is, instead of just recalculate_all_cards_in_play(); it's not supposed to have any side effects; and the enchanted object won't have any keyword abilities that aren't intrinsic to the card (not just flying for oddballs like Roots - plenty of effects add protection from colors).

IS_AURA_EVENT() is supposed to include everything aura() will respond to, rather than aura() responding only to what IS_AURA_EVENT() lets through. The point is to A) avoid constructing target_definition_t's during irrelevant high-volume events, and B) make it safe to call get_card_instance() and in_play() and so on before checking event, and still be confident that player and card will always be valid.

(For that matter, aura() and the other front ends to aura_impl() should be calling it themselves before they construct target_definition_t's and call get_card_instance() and so on, instead of assuming their callers already have.)

[drool66]

Cleaned up in 696e966
[EDIT] Marking fixed until someone says differently