Page 1 of 1

[fixed] Crovax, Ascendant Hero multiple activations

PostPosted: 13 Sep 2021, 20:29
by etphonehome
Crovax, Ascendant Hero activates multiple times, even having to pay 2 life points.

and

It's creating this creature in hand each time it activates.


THX

Re: Crovax, Ascendant Hero multiple activations and bugged c

PostPosted: 13 Sep 2021, 22:23
by drool66
Fixed in 1aefc91 [EDIT: reset this commit]

I'm going to need some feedback on this fix though, since it's a change to bounce_permanent() - really just an in_play() check before dispatching the triggers. I don't see any reason that shouldn't be there.

Re: [fixed] Crovax, Ascendant Hero multiple activations

PostPosted: 14 Sep 2021, 01:12
by Korath
We can't give you real feedback until you push.

But best guess based solely on what you wrote here, in_play() is too restrictive for what you're trying to check for - it'll either break cards that return permanents from the stack to their owners' hands, like Remand and Unsubstantiate, if you do it early, or not actually fix this, if you do it late.

You don't want to see if what you're bouncing is on the battlefield to see if an object can be returned to its owner's hand; you want to see if the object still exists - that is, card_instance_t::internal_card_id >= 0 - and return immediately if it doesn't. in_play() checks that and also that it's specifically on the battlefield. You do want to check in_play() at other points in that function, too, before dispatching TRIGGER_LEAVE_PLAY and TRIGGER_BOUNCE_PERMANENT and the awful hacks at the end for specific cards.

You should also be aware that, like discard() and discard_card(), the executable version of bounce_permanent() was never overridden to use the C one, and no effort was made to ensure that all calls to it were replaced. Whimsy, at a minimum, still uses all three.

Re: [fixed] Crovax, Ascendant Hero multiple activations

PostPosted: 14 Sep 2021, 07:03
by drool66
Got it - Remand is the case I was trying to think of, thank you.
Reset & recommitted in 2dd848e, pushed