It is currently 07 Jul 2021, 11:57
   
Text Size

Shandalar - Program Bugs

Mishra's Factory can be activated for mana while tapped (fix completed)

 

To repro:
  1. Debug 2 Black Lotus, Mishra's Factory, Obsianus Golem into hand
  2. Play the lotuses and factory
  3. Click the golem to begin casting; {6} to pay
  4. Click the factory; it bypasses its normal menu (because mana is being paid) and taps and leaves {5} to pay
  5. Click the factory again; it again bypasses its normal menu and pays another {C}.

Comments

Posted by Korath » 17 Mar 2016, 00:22

Doesn't occur in RT1, trying to track down where it changed.

Posted by Korath » 17 Mar 2016, 03:48

The commit that broke this | Open
commit c79b2c0d1632bc7a4493c7aada85472a1b215ec7
Author: Korath <dgk@Dirge.none>
Date: Tue Feb 9 01:05:50 2016 -0500

[AS] +mana-producing cards; +infrastructure for those and nonmana+mana cards

Cards added:
Cadaverous Bloom
Millikin
Nimbus MAze
Pristine Talisman
Springleaf Drum
Treasonous Ogre
Wall of Roots

Rewritten:
Gem Bazaar
Mana Crypt
Priest of Yawgmoth

Support:
ActivateAddCounter
ActivateExileFromHand
ActivateProduceMana (internal to mana_producers.cpp)
ActivateProduceManaTapped (internal to mana_producers.cpp)

Publicize select_card_in_hand_to_exile().

Import LCBP_CHARGING_MANA from Manalink, so cards with both mana-producing and
non-mana-producing activated abilities can be activated while paying mana.

autotap_mana_source() doesn't abort for tapped cards or summonsick creatures.

TENTATIVE_reassess_all_cards_helper() correctly highlights mana-producing
creatures that are tapped or summonsick as activateable (so long as they return
true from EVENT_CAN_ACTIVATE, as usual).

autotap_mana_source() and charge_mana() call recopy_card_onto_stack(), so cards
with both mana-producing and non-mana-producing abilities and that store data
during EVENT_ACTIVATE have access to it during EVENT_RESOLVE_ACTIVATION.

All cards are sent EVENT_CAN_ACTIVATE while manually tapping while charging
mana, instead of just non-lands.

Cards are removed from the hand just before charging mana rather than just
after (so you can't, for example, try to pay for casting a spell by exiling it
to Cadaverous Bloom).

Incidental:
Rainbow Vale uses legacy_at_next_eot() interface instead of hand-rolling, so
e.g. it works right if activated during the end step, or (eventually) if the
delayed trigger is Stifle'd.


I expect, without having yet looked into it to be sure, that Mishra's Factory was relying on autotap_mana_source() (or perhaps the matching one to TENTATIVE_reassess_all_cards_helper(), but I think that unlikely) to prevent it from activating while a mana cost is being paid, without itself checking whether it's tapped. The change was put in to fix the Shandalar version of this Manalink bug.

Posted by Korath » 17 Mar 2016, 04:19

Won't affect any other cards; all of the remaining unrewritten cards that produce mana respond properly to EVENT_CAN_ACTIVATE. Mishra's Factory (and its separate but almost identical implementation as Assembly-Worker) always uses the same EVENT_CAN_ACTIVATE handler, but autopicks activation for mana during EVENT_ACTIVATE even if it would have been disabled in the activation dialog.
Last edited by Korath on 17 Mar 2016, 04:19, edited 1 time in total.
Reason: typo

Posted by Korath » 17 Mar 2016, 05:17

commit a93b99833394c9c115f52416c41b7b53bdeff9d2
Author: Korath <dgk@Dirge.none>
Date: Thu Mar 17 01:17:18 2016 -0400

[AS] FIX #1054: can't activate Mishra's Factory if tapped & LCBP_CHARGING_MANA

The MicroProse implementation of Mishra's Factory (and the separate one for
Assembly-Worker) always autopicks choice 0 on the dialog menu, "Get mana.",
if needed_mana_colors is set. (That'll happen during EVENT_ACTIVATE while
paying a mana cost, whether manually or by double-clicking (which is the same
method the AI uses).) However, it always uses the same logic for its
EVENT_CAN_ACTIVATE handler, so if it can pay {C} (for the animate ability), the
factory thinks it can activate its mana ability even if it's tapped.

Until commit c79b2c0d1632bc7a4493c7aada85472a1b215ec7, there was logic in
autotap_mana_source() and TENTATIVE_reassess_all_cards_helper() that forced
mana sources that were summonsick creatures or tapped not to be activateable,
regardless of what their EVENT_CAN_ACTIVATE handler said. This prevented this
bug from manifesting, but prevented cards that produce mana without needing to
tap to do it from working properly (see e.g.
<http://www.slightlymagic.net/forum/viewtopic.php?t=18055>).

Worse, at the time EVENT_CAN_ACTIVATE is dispatched here, needed_mana_colors
isn't set yet, so that isn't usable to check whether the menu will be bypassed.

Fortunately, the same commit imported LCBP_CHARGING_MANA from Manalink
(recently added there in commit ee91a887a1778602f69ba3602f4e25513c31853d), and
that *is* set in time.

So wrapping card_mishras_factory() and card_assembly_worker() to return 0 for
EVENT_CAN_ACTIVATE if LCBP_CHARGING_MANA is set and otherwise forwarding to the
MicroProse functions seems to work properly.

Other cards aren't affected; the remaining MicroProse mana-producing cards all
always check whether they can tap in their EVENT_CAN_ACTIVATE handlers, and the
ones in Shandalar.dll all either do so as well or check LCBP_CHARGING_MANA.
For safety's sake, though, check it in mana_battery()'s EVENT_ACTIVATE handler
(which just inspected needed_mana_colors), and set it around the dispatch of
EVENT_CAN_ACTIVATE in can_activate_for_mana().

Ticket details

  • Ticket ID: 1054
  • Project: Shandalar
  • Status: Fix completed
  • Component: Individual Card
  • Project version: Abandoned Shrine 1
  • Priority: Normal
  • Severity: Major
  • Assigned to: Korath
  • Reported by: Korath
  • Reporter's tickets: List all tickets
  • Reported on: 17 Mar 2016, 00:16
  • Last visited by Korath » 07 Dec 2016, 16:46.
 

Login Form