[fixed]Rep: Oko and Aura

[Aswan jaguar]

Please write full name of the card as it appears on the card. There are 4 cards beginning with Oko two with 2nd ability. I guess you mean Oko, Thief of Crowns right?

[Aswan jaguar]

The bug is confirmed also in dev. Also the aura that enchants a creature animated by Oko, Thief of Crowns if the creature leaves play, the aura stays on battlefield.

[drool66]

I have it figured out, but before I commit: Aswan jaguar, do you know the semantic difference between generic_aura_impl() and targeted_aura_impl()? When would we use one vs the other, and is there any reason they can't be merged? It could mean we could get rid of that targeting by csvid section near the top of generic_aura_impl() ie. "if( csvid == CARD_ID_EMBLEM_OF_THE_WARMIND || csvid == CARD_ID_DYING_WISH..."

[Korath]

generic_aura() was the precise opposite of generic: it added or replaced subtypes; it changed color and power and toughness and keyword abilities; prevented activated abilities, and removed all abilities; it removed abilities, or just activated ones, or just nonmana activated ones; it contained all the special cases for everything, none of which should have been special-cased at all; and it could only enchant creatures, because it defined its own targeting, as modified by a zillion and one card-specific cases in the function itself.

targeted_aura() was introduced specifically to only do the absolute minimum that every aura needs to work - castability, targeting when cast, attaching when put on the bf, object selection when moved - essentially implementing an other-wise blank aura card with "Enchant &td". It moved the targeting def out into the caller (where it belongs) and had no effects or any special cases whatsoever (where they don't belong).

Now, ha ha ha, it has effects of its own and has started collecting card-specific special cases, too, because keeping an actually-generic version around untouched was just too tempting. Burn them both down to the ground and start over, says I.

[drool66]

My bright idea here is to have the new aura_impl() take its args for p/t, abilities, color & types in the form of a test_t. I went ahead and added sp_keyword and sp_keyword_flag to the test_definition_t struct and their interpretation in make_test / _in_play(). I've also created enums for p/t, color and type flags germane only to this application that translate the current generic_aura_flags_t.
One issue is how to handle p/t since default_test_definition() defaults them to -1. One idea is to create default_ability_mod() which defaults p/t to 1<<24, and have anything interpreting it to ignore values equal to that. The other is to ignore negative values for p/t, and have aura_impl() rely on power/toughness_flag == F2_SUBTRACT_POWER / _TOUGHNESS.
I've tested both methods working by having generic_aura_impl() and targeted_aura_impl() translate their args to this format - full implementation would have anything currently calling generic_aura_impl() and targeted_aura_impl() to call the new function instead, rather than simply replacing these two functions with ones that call the new function. It's just a matter of what makes the most sense to do - I'm not sure if this makes life easier or harder. Any thoughts on this?

[Korath]

pump_ability_until_eot_arguments_t seems a better match, both structurally and semantically. Auras and effect cards are basically the same thing in Manalink anyway, while test_definition_t instead asks "Does this object match this set of constraints?".

I do think you're better off keeping your base aura function as bare-bones as possible - implement "Enchant [target_definition_t]" and nothing else. Do everything else with mixins - design them right, and you can use the same ones for equipment and effect cards. It probably makes sense to keep front ends implemented in terms of the base function for the very common cases of changing power/toughness, adding keywords, and both; and perhaps even for removing abilities and preventing activation, attacking, and/or blocking. But they - and especially rarer things like changing subtypes and colors and so on - are orthogonal to what an aura is, so don't really belong in the same function. If you have a single monolithic front-end function that does everything under the sun, you have to remember what all of its dozen hardly-ever-used parameters do, even for the simple cases.

Anyway, you should first figure out what you want your individual card functions' source to look like, and design the interface to match that as much as possible; not implement the back end first, and then have to suffer with it every time you program a new card.

Example apis | Open

Code: Select all

int
card_dehydration(int player, int card, event_t event)
{
  /* Dehydration        |3|U
   * Enchantment - Aura
   * Enchant creature
   * Enchanted creature doesn't untap during its controller's untap step. */

  attached_doesnt_untap(player, card, event);

  // functions named target_* construct a target_definition_t in their first parameter, and return it
  target_definition_t td;
  return enchant(player, card, event, target_creature(&td, 1-player));
}

int
card_controlled_instincts(int player, int card, event_t event)
{
  /* Controlled Instincts       |U
   * Enchantment - Aura
   * Enchant |Sred or |Sgreen creature
   * Enchanted creature doesn't untap during its controller's untap step. */

  attached_doesnt_untap(player, card, event);

  // functions named td_* modify the already-initialized target_definition_t in their first parameter, and return it
  // enchant() does the slighting for you - you should hardly ever need to call get_sleight() from individual cards
  target_definition_t td;
  return enchant(player, card, event,
                 td_color(target_creature(&td, 1-player), COLOR_TEST_RED | COLOR_TEST_GREEN));
}

int
card_inertia_bubble(int player, int card, event_t event)
{
  /* Inertia Bubble   |1|U
   * Enchantment - Aura
   * Enchant artifact
   * Enchanted artifact doesn't untap during its controller's untap step. */

  attached_doesnt_untap(player, card, event);

  target_definition_t td;
  return enchant(player, card, event, target_artifact(&td, 1-player));
}

int
card_ice_over(int player, int card, event_t event)
{
  /* Ice Over   |1|U
   * Enchantment - Aura
   * Enchant artifact or creature
   * Enchanted permanent doesn't untap during its controller's untap step. */

  attached_doesnt_untap(player, card, event);

  target_definition_t td;
  return enchant(player, card, event, target_type(&td, TYPE_ARTIFACT | TYPE_CREATURE, 1-player));
}

int
card_mark_of_the_vampire_1(int player, int card, event_t event)
{
  /* Mark of the Vampire        |3|B
   * Enchantment - Aura
   * Enchant creature
   * Enchanted creature gets +2/+2 and has lifelink. */

  attached_gets_pt(player, card, event, 2, 2);
  attached_has_spkw(player, card, event, SP_KEYWORD_LIFELINK);

  target_definition_t td;
  return enchant(player, card, event, target_creature(&td, player));
}

int
card_mark_of_the_vampire_2(int player, int card, event_t event)
{
  /* Mark of the Vampire        |3|B
   * Enchantment - Aura
   * Enchant creature
   * Enchanted creature gets +2/+2 and has lifelink. */

  // target creature implicit in aura_pump() - prefers player if p/t all >= 0, 1-player otherwise
  return aura_pump(player, card, event, 2, 2, 0, SP_KEYWORD_LIFELINK);
}

int
card_nimbus_wings_1(int player, int card, event_t event)
{
  /* Nimbus Wings       |1|W
   * Enchantment - Aura
   * Enchant creature
   * Enchanted creature gets +1/+2 and has flying. */

  attached_gets_pt(player, card, event, 1, 2);
  attached_has_kw(player, card, event, KEYWORD_FLYING);

  target_definition_t td;
  return enchant(player, card, event, target_creature(&td, player));
}

int
card_nimbus_wings_2(int player, int card, event_t event)
{
  /* Nimbus Wings       |1|W
   * Enchantment - Aura
   * Enchant creature
   * Enchanted creature gets +1/+2 and has flying. */

  /* See how easy it is to mix up which parameter does what, even in this short version?  Try remembering which
   * parameter is subtype in generic_aura() without the function def open in another window, let alone what its
   * "flags" parameter does. */
  return aura_pump(player, card, event, 1, 2, KEYWORD_FLYING, 0);
}

int
card_call_to_serve(int player, int card, event_t event)
{
  /* Call to Serve      |1|W
   * Enchantment - Aura
   * Enchant non|Sblack creature
   * Enchanted creature gets +1/+2, has flying, and is an Angel in addition to its other types. */

  attached_gets_pt(player, card, event, 1, 2);
  attached_has_kw(player, card, event, KEYWORD_FLYING);
  attached_is_additional_subtype(player, card, event, SUBTYPE_ANGEL);

  target_definition_t td;
  return enchant(player, card, event, td_noncolor(target_creature(&td, player), COLOR_TEST_BLACK));
}

int
card_raven_wings(int player, int card, event_t event)
{
  /* Raven Wings        |2
   * Artifact - Equipment
   * Equipped creature gets +1/+0, has flying, and is a Bird in addition to its other types.
   * Equip |2 */

  attached_gets_pt(player, card, event, 1, 0);
  attached_has_kw(player, card, event, KEYWORD_FLYING);
  attached_is_additional_subtype(player, card, event, SUBTYPE_BIRD);

  return equip(player, card, event, 2);
}

static const char*
with_another_aura_attached_to_it(int who_chooses, int player, int card, int targeting_player, int targeting_card)
{
  /* not quite target_is_already_enchanted() from future_sight.c - you'd want it to exclude {targeting_player,
   * targeting_card}, so you can use the target_definition_t unchanged in enchant() to implement it zapping off */
  /* ... */
}
int
card_daybreak_coronet(int player, int card, event_t event)
{
  /* Daybreak Coronet   |W|W
   * Enchantment - Aura
   * Enchant creature with another Aura attached to it
   * Enchanted creature gets +3/+3 and has first strike, vigilance, and lifelink. */

  attached_gets_pt(player, card, event, 3, 3);
  attached_has_kw(player, card, event, KEYWORD_FIRST_STRIKE);
  attached_has_spkw(player, card, event, SP_KEYWORD_VIGILANCE | SP_KEYWORD_LIFELINK);

  target_definition_t td;
  return enchant(player, card, event, td_function(target_creature(&td, player), with_another_aura_attached_to_it));
}

Compare that last, especially, to the current implementation for Daybreak Coronet.

[drool66]

Fixed in d693305