[fixed] Crovax, Ascendant Hero multiple activations
Moderators: BAgate, drool66, Aswan jaguar, gmzombie, stassy, CCGHQ Admins
[fixed] Crovax, Ascendant Hero multiple activations
by etphonehome » 13 Sep 2021, 20:29
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
and
It's creating this creature in hand each time it activates.
THX
- Attachments
-
- Crovax, Ascendant Hero savegame.rar
- (2.15 KiB) Downloaded 70 times
Last edited by drool66 on 13 Sep 2021, 22:24, edited 1 time in total.
Reason: fixed
Reason: fixed
- etphonehome
- Posts: 299
- Joined: 21 May 2020, 12:50
- Has thanked: 270 times
- Been thanked: 39 times
Re: Crovax, Ascendant Hero multiple activations and bugged c
by drool66 » 13 Sep 2021, 22:23
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.
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.
Last edited by drool66 on 14 Sep 2021, 07:31, edited 1 time in total.
Reason: reset commit
Reason: reset commit
The latest images for Manalink will be here.
The latest Manalink installation directory will be here. Well, not quite, anymore. Check the latest patches.
The latest Manalink installation directory will be here. Well, not quite, anymore. Check the latest patches.
-
drool66 - Programmer
- Posts: 1163
- Joined: 25 Nov 2010, 22:38
- Has thanked: 186 times
- Been thanked: 267 times
Re: [fixed] Crovax, Ascendant Hero multiple activations
by Korath » 14 Sep 2021, 01:12
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.
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.
-
Korath - DEVELOPER
- Posts: 3707
- Joined: 02 Jun 2013, 05:57
- Has thanked: 496 times
- Been thanked: 1106 times
Re: [fixed] Crovax, Ascendant Hero multiple activations
by drool66 » 14 Sep 2021, 07:03
Got it - Remand is the case I was trying to think of, thank you.
Reset & recommitted in 2dd848e, pushed
Reset & recommitted in 2dd848e, pushed
The latest images for Manalink will be here.
The latest Manalink installation directory will be here. Well, not quite, anymore. Check the latest patches.
The latest Manalink installation directory will be here. Well, not quite, anymore. Check the latest patches.
-
drool66 - Programmer
- Posts: 1163
- Joined: 25 Nov 2010, 22:38
- Has thanked: 186 times
- Been thanked: 267 times
4 posts
• Page 1 of 1
Who is online
Users browsing this forum: No registered users and 67 guests