[confirmed]Moonlit Strider bad parameter & crash to desktop.

[etphonehome]

Another bad parameter alert and crash to desktop.

THX

[Aswan jaguar]

AI doesn't like something in Moonlit Strider code, and it is not that it doesn't validate it's target. If there is another creature in AI's battlefield and AI has in hand Moonlit Strider and tries to cast it, crash.

[Korath]

The dump file in the attached rar isn't anything like the one in your screenshot.

[etphonehome]

After this alert i think it gave another alert and crashed.

[drool66]

It's my fault -
markup of is_this_dying(): "this should be checked only during EVENT_GRAVEYARD_FROM_PLAY to be realiable"
me: checks it during TRIGGER_LEAVE_PLAY

EDIT: Oh, ok. Moonlit Strider's EVENT_RESOLVE_ACTIVATION tries to pump ability using the parent as the source... on an ability that sacrifices the parent. That will give you a bad param. Fixed

[Aswan jaguar]

For me with below code if you didn't change anything else it dumps and crashes.

error | Open

bad parameters
get_card_instance(24254622, 24254622)
0: 0x025B2218
1: 0x025B8CCE
2: 0x025C91EB
3: 0x0043467C
4: 0x0043C147
5: 0x004399BD
6: 0x0047902C
7: 0x004946E9
8: 0x7C80B729

Code: Select all

int card_moonlit_strider(int player, int card, event_t event){
   /* CARD_ID_MOONLIT_STRIDER   7521
   Moonlit Strider   |3|W
   Creature - Spirit 1/4
   Sacrifice ~: Target creature you control gains protection from the color of your choice until end of turn.
   Soulshift 3 (When this creature dies, you may return target Spirit card with converted mana cost 3 or less from your graveyard to your hand.)*/

   if( ! IS_GAA_EVENT(event) ){
      return soulshift(player, card, event, 3, 1);
   }

   target_definition_t td;
   default_target_definition(player, card, &td, TYPE_CREATURE );
   td.preferred_controller = td.allowed_controller = player;

   card_instance_t *instance = get_card_instance( player, card );

   if( event == EVENT_CAN_ACTIVATE || event == EVENT_ACTIVATE ){
      return generic_activated_ability(player, card, event, GAA_CAN_TARGET | GAA_SACRIFICE_ME | GAA_LITERAL_PROMPT, MANACOST0, 0,
                              &td, "Select target creature you control.");
   }

   if( event == EVENT_RESOLVE_ACTIVATION ){
      if( valid_target(&td) ){
         pump_ability_until_eot(player, card, instance->targets[0].player, instance->targets[0].card,
                           0, 0, select_a_protection(player) | KEYWORD_RECALC_SET_COLOR, 0);
      }
   }

   return 0;
}

[drool66]

You also have to make a change in leaves_play() - that's the part that's my fault. Try moving the lines:

Code: Select all

         if( is_this_dying(player, card) && check_for_death_trigger_removal(player, card) && can_be_suppressed )
            return 0;


under the "if" just below it ( if affect_me()... ) , right above "if( event == EVENT_TRIGGER )" Doing this seems to shield it from bad params by checking affect_me() and trigger_cause/_controller before getting an instance from them. I can play a game with two Moonlit Strider s on the bf and one in AI's hand with no crashes or dumps.

[Korath]

Pretty sure the root problem here is that you can reactivate the strider an arbitrary number of times in response to its own soulshift trigger.

Why are you always triggering on TRIGGER_LEAVE_PLAY in legacy_effect_pump_ability_until_eot(), anyway, instead of just when the effect's configured to do something when the source or affected object leaves the bf?

[drool66]

Why are you always triggering on TRIGGER_LEAVE_PLAY in legacy_effect_pump_ability_until_eot(), anyway, instead of just when the effect's configured to do something when the source or affected object leaves the bf?

Do you mean instead of this:

Code: Select all

   leaves_play(instance->targets[6].player, instance->targets[6].card, event, 0, 0);
   if( event == EVENT_LEAVES_PLAY_ABILITY && ((instance->targets[3].player & PAUE_END_IF_SOURCE_UNTAP ) || (instance->targets[3].player & PAUE_END_IF_SOURCE_LEAVES_PLAY ))){
      end_effect = 1;
   }

Something like this?:

Code: Select all

   if( ((instance->targets[3].player & PAUE_END_IF_SOURCE_UNTAP ) || (instance->targets[3].player & PAUE_END_IF_SOURCE_LEAVES_PLAY )) ){
      leaves_play(instance->targets[6].player, instance->targets[6].card, event, 0, 0);
      if( event == EVENT_LEAVES_PLAY_ABILITY ){
         end_effect = 1;
      }
   }

That makes sense

[Aswan jaguar]

Any news on this?

[drool66]

I'll commit the changes to legacy_effect_pump_ability_until_eot() soon. The others snuck in under my nose in d693305. I believe it's fixed.