[fixed]Chromatic Lantern v. Wastes

[Korath]

Describe the Bug:
Wastes can't produce colorless mana if there's a Chromatic Lantern on the bf.

Which card behaved improperly?
mana_producer()

Which update are you using? (date, name)Which type? (duel, gauntlet, sealed deck)
0801abee9 - (master) First round of drafts for DOM
(or drool66 head)

What exactly should be the correct behavior/interaction?
There should be a little grey ball for colorless mana in the middle of the dialog.

Are any other cards possibly affected by this bug?
All lands that can produce colorless.

---

This is technically my bug, since mana_producer()'s EVENT_ACTIVATE handler removes everything but (COLOR_TEST_ANY_COLORED | COLOR_TEST_ARTIFACT) instead of (COLOR_TEST_ANY | COLOR_TEST_ARTIFACT), but nothing's needed it until now.

PS, get_global_mana_color_addition() is a relatively expensive function and you're calling it entirely too often. Since it doesn't have any side effects,

Code: Select all

if( is_what(player, card, TYPE_LAND) && get_global_mana_color_addition(player)){
   clr |= get_global_mana_color_addition(player);
}

is just a pessimization of

Code: Select all

if( is_what(player, card, TYPE_LAND)){
   clr |= get_global_mana_color_addition(player);
}

and calling it unconditionally for every event in cards like Bazaar of Baghdad is right out. You probably want to call it just once during state-based effects or the start of mana counting and stash it in a global for each player, anyway, instead of doing it over and over for each mana producer.

PPS, I assume the new Chromatic Lantern is unfinished and you're aware cards that add more than one mana don't even sort of work. At least the ones I tried - Mishra's Workshop and Dwarven Ruins - don't.

PPPS, you'll find that getting mana counting right for cards that can add differing amounts of mana to be very difficult. (We already see this with Nykthos, Shrine to Nyx.) I suspect there isn't a polynomial solution, and it's why none of these cards are in Shandalar.

[FastEddie]

PPPS, you'll find that getting mana counting right for cards that can add differing amounts of mana to be very difficult. (We already see this with Nykthos, Shrine to Nyx.) I suspect there isn't a polynomial solution, and it's why none of these cards are in Shandalar.

Just shooting from the hip after a little thinking... Effects like Nykthos, Shrine to Nyx, namely choose a colour and add n mana of that fixed colour to your mana pool are still ok if you want polynomial time (as you implicitly said, I think). Effects like "choose a number n and add a different colour of mana (or some subset of the five colours plus maybe colourless) to your mana pool for each n" wouldn't work in polynominal time, but it would probably be doable if n were small (and the function doing the calculation is not called several hundred times during each turn). What would be ugly is if n would get big. Basically you have m^n possibilites where m is the number of colours you admit (so m=1,..,6).

[You have m possibilites for the first position, m for the second, so m*m in total and so on. Do this n times and you get m^n.]

Oh, and making n variable, say something like "add between 2 and 4 mana of any given colour" wouldn't help either... the above said still holds, a reasonably small n should be ok, but the effort is something like adding 5 mana in the example above (not quite but a reasonable upper bound).

[Korath]

It gets a lot worse when you have multiple Nykthos-equivalents. Say, full playsets of all the two-mana saclands plus a Stormtide Leviathan, so each can produce either (or whatever color) or .

[FastEddie]

That it actually not so bad as is equivalent to one arbitrary mana (either you have it or you don't, so there are only two choices). Take friend Reaper King as an example: it has "only" 2^5 possible casting cost variants as each coloured mana can be substituted by 2 colourless. That's not a show stopper.
To put this in the context of the previous post: you would have 11 different "colours" of mana, namely 5 regular, 5 double (i.e. and 4 others) and colourless. This would mean m=1,..,11, so you have to make sure n does not get too big. If you would go up to n in the previous case here n/3*2 (roughly) would be the limit.

It would probably make sense at some point in time to make a list of cards with those effects and go through the combinations to decide what is feasible and what is not. But definitly not now (for me at least, as I have much more pedestrian stuff to get my head around).

Edit: added reference to previous post.

[drool66]

It's definitely a WIP, but I hadn't considered the Wastes case and haven't tested multimana lands as much as I would have liked (I've been having machine issues lately) - just the karoos so far. I plan to go over every non-basic before release and I'm fairly confident I can get them all working.

Something's also gone haywire in my build to make it seem as though get_global_mana_color_addition() always returns COLOR_TEST_ANY_COLORED; I don't know if that's the case for you too.

Storing get_global_mana_color_addition() in a global is definitely what I want to be doing, but I don't know which to use - player bits can't hold this, and I filled those out anyway with ability disablers. Maybe something in the rules engine? I'm also looking at working in the Contamination/Infernal Darkness/Hall of Gemstone space, which requires a separate check for color overwrite and quantity overwrite. I included Infernal Darkness as a quasi-Contamination clone not realizing the difference in overwriting the quantity of mana produced. Point is, these will all require separate checks and need their own global variables if they are to be done.

I'm aware of some of the complexities in declare_mana_available/_maybe_hex(), and I'm not even going to pretend there aren't issues I'm not aware of. Some of these are covered just in mana_producer() declaring the color test bits available, but there are situations like an Orzhov Basilica+ Stormtide Leviathan probably declares mana available for Dimir Charm or Azorius Charm even though it can cast neither. I'm not sure what the solution is to things like this.

[Korath]

Storing global data in the Rules Engine is a hack, and there's really no need to deal with it. There's machinery in src/functions/ai.c to mark arbitrary data for savegame and ai backup and retrieval; look for BACKUP_AND_SAVE_LIST. (There's more hoops you need to jump through if you need to access it from the display thread, though.)

[drool66]

I came up with a framework for all this. Korath, I thought I'd run it by you before I finalize, test and commit.
Global variables are declared in manalink.h & ai.c and they are memset to zero in rules_engine.c > game_startup(). The relevant cards are checked in check_card_for_rules_engine() and return the new REF_MANA_OVERRIDE. Rules Engine checks for this, and if found runs get_global_mana_color_addition() on EVENT_COUNT_MANA; the function now loads the information to the globals. Mana sources then check the globals instead of calling the function each time they are activated.
Is rules engine the best place to run the function? For one thing, that means the function is run on every EVENT_COUNT_MANA when one or more of these cards are in the deck, even if they are not in play. This is preferable to running it from each mana source like before, but is there a way to bypass the rules engine card completely?

[Korath]

I wouldn't be concerned about running it once per count_mana() call, performance-wise. count_mana() already loops over all cards, and for each flagged EA_MANA_SOURCE (and some others), dispatches a full event which in turn loops over all cards.

You don't want to run it from the rules engine during EVENT_COUNT_MANA, though, since it has to be in place before you count mana for any other card. The ideal place to run is from count_mana() itself. Either move it entirely into C (by replacing the function in Magic.exe with a jmp to the C function; see src/patches/patch_move_into_c.pl - particularly its end - to see how) or hook it (by changing its calls from C to go to your own function, which ends in calling Magic.exe's version; and replacing Magic.exe's remaining call of count_mana() at 0x47226e with a call to your hook).

[drool66]

Ok, thank you very much! I have my new count_mana() ready, with requisite entries in patch_move_into_c.pl, ManalinkEh.asm, and I removed the entry for count_mana() from Manalink.lds.

A few things I'm unclear on, if you don't mind:

1) I copied count_mana() from your post on Wolf of Devil's Breach. Where would I find this otherwise, and how would I translate to C? Would I need to learn asm?
2) How are we editing Magic.exe these days? Specifically, how would I replace the call from reassess_all_cards() to count_mana()? Just any disassembler? And how is this searched in the first place?
[edit] So I see "call sub_472400" at 0x47226e. If my new function is at 20103e9, would I just change it to "sub_20103e9"?

I will need to run "perl patch_move_into_c.pl" with Magic.exe in the same directory, correct?

[Korath]

So I see "call sub_472400" at 0x47226e. If my new function is at 20103e9, would I just change it to "sub_20103e9"?

Yes, but that word "just" is misleading.

Code: Select all

$ objdump -M intel -d Magic.exe |grep 472400
  435247:       e8 b4 d1 03 00          call   0x472400
  47226e:       e8 8d 01 00 00          call   0x472400
  472400:       55                      push   ebp

(The last hit is the start of the function itself, and the first hit is inside recalculate_all_cards_in_play(), which has already been moved into C.)

You'll see that the actual bytes at 47226e don't have seem to have anything to do with "472400". What they actually are are the opcode for "call" (e8), followed by the difference between the target and the following instruction. 0x472400 - 0x47226e = 0x192; a call instruction is five bytes long, so subtract another 5 to get 0x18d; and you put that in as a little-endian 32-bit value to get 8d 01 00 00.

So to call 0x020103e9 from 0x47226e, you'd get 0x20103e9 - (0x47226e + 5) = 0x1b9e176, so "e8 76 e1 b9 01" if my math's right.

More cleanly, Manalink::Patch has a function call_to() that works just like jmp_to() to do this, so you should just run "call_to(0x47226e => 0x20103e9);" (Even easier than that, you could just use something like IDA that lets you edit in assembly mode, but that isn't reproduceable, so please don't.)

Whenever you edit Magic.exe like this, you'll want to save a full disassembly (from "objdump -M intel -d Magic.exe") first, take another after, and diff them to be sure that you did what you wanted and you didn't do anything else by accident. It's frightfully easy screw up the first and to do the second.

I will need to run "perl patch_move_into_c.pl" with Magic.exe in the same directory, correct?

Yes.

[FastEddie]

Here is another example just in case you didn't fully understand the call opcode: https://stackoverflow.com/questions/10376787/need-help-understanding-e8-asm-call-instruction-x86.

Little Endian (as opposed to Big Endian) means that 32bit (also 16bit and 64bit) values are stored starting with the lowest 8 bit first, i.e. 0x12345678 becomes 78 56 34 12. In Korath's example you have to add the leading zeros usually ignored, i.e. 0x18d becomes 0x0000018d or - adding some spaces to make it easier to read - 0x 00 00 01 8d. Which in Little Endian becomes 8d 01 00 00.

In case your hex math is a bit rusty, the Windows 10 calculator has a "programmer mode" where you can run calculations in hex and octal.

Whenever you edit Magic.exe like this, you'll want to save a full disassembly (from "objdump -M intel -d Magic.exe") first, take another after, and diff them to be sure that you did what you wanted and you didn't do anything else by accident.

Thanks for that. I for my part would have grabbed the next available disassembler and done exactly that...

[drool66]

Thank you again for the help & support, both of you. Everything is working as of yesterday, including:
Wastes - change (COLOR_TEST_ANY_COLORED | COLOR_TEST_ARTIFACT) to (COLOR_TEST_ANY | COLOR_TEST_ARTIFACT) - I don't think this should break anything else, right?
Mishra's Workshop - I had made an error in the dialog options
Dwarven Ruins et al. - I had to expand the function and pull out any options that would go to the added colors after the first choice is made

I'll go through all the nonbasics as soon as I figure out what I'm going to do about Blood Sun

The new count_mana() also has a provision for Elvish/Simian Spirit Guide, from here, and I think it will work properly with Contamination/Infernal Darkness, Hall of Gemstone, and maybe even Reality Twist/Naked Singularity. I'm dreading trying to get Nykthos working 100% properly, but it probably can be done.

I'm just cleaning some things up and I'll commit.

For the sake of documentation, the entry to patch_move_into_c.pl looks like this:

Code: Select all

##############
# count_mana #
##############
#472400:   55         push   ebp
#472401:   8b ec      mov   ebp, esp
#472403:   83 EC 0C 56 33   sub   esp, 0Ch
jmp_to(0x472400 => 0x20103e9);
call_to(0x47226e => 0x20103e9);


the Magic.exe diff looks like this:

Code: Select all

- 47226e:   e8 8d 01 00 00          call   0x472400
+ 47226e:   e8 76 e1 b9 01          call   0x20103e9
- 472400:   55                      push   ebp
- 472401:   8b ec                   mov    ebp,esp
- 472403:   83 ec 0c                sub    esp,0xc
- 472406:   56                      push   esi
+ 472400:   e9 e4 df b9 01          jmp    0x20103e9
+ 472405:   0c 56                   or     al,0x56
+                                  
+                               


and the new count_mana() and its helper functions looks like this:

Code: Select all

static int basic_subtype_to_color_test(subtype_t subtype){
   if( subtype == SUBTYPE_SWAMP)
      return COLOR_TEST_BLACK;
   if( subtype == SUBTYPE_ISLAND)
      return COLOR_TEST_BLUE;
   if( subtype == SUBTYPE_FOREST )
      return COLOR_TEST_GREEN;
   if( subtype == SUBTYPE_MOUNTAIN )
      return COLOR_TEST_RED;
   if( subtype == SUBTYPE_PLAINS )
      return COLOR_TEST_WHITE;
   return 0;
}

static inline card_instance_t*
in_play_or_in_play_and_invisible(int player, int card)
{
  card_instance_t* inst = get_card_instance(player, card);
  if (inst->internal_card_id != -1
      && (inst->state & (STATE_OUBLIETTED|STATE_IN_PLAY)) == STATE_IN_PLAY)
    return inst;
  else
    return NULL;
  // Neither the check against -1 instead of >= 0 or allowing STATE_INVISIBLE is meaningful in practice, so better to use in_play() instead
}

void count_mana(void){
   color_t c;
   for (c = COLOR_COLORLESS; c <= COLOR_ANY; ++c)
   {
//      global_cost_mod[c] = 0;
      raw_mana_available[0][c] = raw_mana_available[1][c] = 0;
   }

   raw_mana_available_hex[0][0] = raw_mana_available_hex[1][0] = -1;

//   raw_mana_may_be_spent_as_color[0][0] = raw_mana_may_be_spent_as_color[1][0] = -1;

   int player, card;
   for (player = 0; player <= 1; ++player){
      for (card = 0; card < active_cards_count[player]; ++card)
      {
         card_instance_t* inst = in_play_or_in_play_and_invisible(player, card);
         if ( inst )
         {
            if ((cards_data[inst->internal_card_id].extra_ability & EA_MANA_SOURCE) && !(inst->state & STATE_TAPPED))
               dispatch_event(player, card, EVENT_COUNT_MANA);

            switch (cards_data[inst->internal_card_id].id){

               case CARD_ID_SUNGLASSES_OF_URZA:
                  if( !(inst->state & STATE_TAPPED))
                     dispatch_event_to_single_card(player, card, EVENT_COUNT_MANA, -1, -1);
                  break;

               case CARD_ID_GLOOM:
                  dispatch_event_to_single_card(player, card, EVENT_COUNT_MANA, -1, -1);
                  break;

               case CARD_ID_PRISMATIC_OMEN:
               case CARD_ID_DRYAD_OF_THE_ILYSIAN_GROVE:
                  mana_colors_added[player] |= COLOR_TEST_ANY_COLORED;
                  break;

               case CARD_ID_JOINER_ADEPT:
               case CARD_ID_CHROMATIC_LANTERN:
                  if(!is_humiliated(player, card) ){
                     mana_colors_added[player] |= COLOR_TEST_ANY_COLORED;
                  }
                  break;

               case CARD_ID_URBORG_TOMB_OF_YAWGMOTH:
                  mana_colors_added[player] |= basic_subtype_to_color_test(get_hacked_subtype(player, card, SUBTYPE_SWAMP));
                  mana_colors_added[1-player] |= basic_subtype_to_color_test(get_hacked_subtype(player, card, SUBTYPE_SWAMP));
                  break;

               case CARD_ID_STORMTIDE_LEVIATHAN:
                  mana_colors_added[player] |= basic_subtype_to_color_test(get_hacked_subtype(player, card, SUBTYPE_ISLAND));
                  mana_colors_added[1-player] |= basic_subtype_to_color_test(get_hacked_subtype(player, card, SUBTYPE_ISLAND));
                  break;

               case CARD_ID_REALMWRIGHT:
                  mana_colors_added[player] |= color_to_color_test( inst->targets[4].player );
                  break;

               default: break;
            }
         }
         inst = in_hand(player, card);
         if (inst)
         {
            if (cards_data[inst->internal_card_id].id == CARD_ID_ELVISH_SPIRIT_GUIDE ||
               cards_data[inst->internal_card_id].id == CARD_ID_SIMIAN_SPIRIT_GUIDE)
               dispatch_event_to_single_card(player, card, EVENT_COUNT_MANA, -1, -1);
         }
      }
      if( is_id_in_grave(player, CARD_ID_RIFTSTONE_PORTAL ) )
         mana_colors_added[player] |= COLOR_TEST_GREEN | COLOR_TEST_WHITE;
   }

  // no cards still have code_pointer 0x413c80 or 0x422360, so the rest of this does nada; comment it out
/*  if (event_flags & EA_FELLWAR_STONE)
    for (int player = 0; player <= 1; ++player)
      for (int card = 0; card < active_cards_count[player]; ++card)
        {
          card_instance_t* inst = get_card_instance(player, card);
          if (inst->internal_card_id != -1
              && (cards_data[inst->internal_card_id].code_pointer == 0x413c80   // asm_card_star_compass()
                  || cards_data[inst->internal_card_id].code_pointer == 0x422360)       // asm_card_fellwar_stone()
              && in_play_or_in_play_and_invisible(player, card)
              && !(inst->state & STATE_TAPPED))
            dispatch_event_to_single_card(player, card, EVENT_VARIABLE_MANA_SRC, -1, -1);
        }*/
   return;
}

Notably, global_cost_mod[] and raw_mana_may_be_spent_as_color[] are commented out - these are Shandalar variables, correct? Is there a corresponding Manalink framework that should go there? Also, I'm unclear why invisible permanents are checked for this.

[Korath]

Wastes - change (COLOR_TEST_ANY_COLORED | COLOR_TEST_ARTIFACT) to (COLOR_TEST_ANY | COLOR_TEST_ARTIFACT) - I don't think this should break anything else, right?

In mana_producer() as discussed above, I assume. No, it shouldn't break anything. I very vaguely recall I had trouble at first getting the graphic for colorless to display, but I think that was just the matter of the file being misnamed (as many were). It should look like

choose_color_dialog.jpg (4.28 KiB) Viewed 2911 times

Be aware it won't show an option for artifact-only mana no matter what you do. If you want to run Mishra's Workshop through this, you'll have to add COLOR_TEST_COLORLESS before the dialog and translate it back to COLOR_TEST_ARTIFACT after.

jmp_to(0x472400 => 0x20103e9);
call_to(0x47226e => 0x20103e9);

You don't need to do both. If you're replacing count_mana() at 0x472400 with a jmp to your function in C, then the remaining exe call from reassess_all_cards() will end up in the same place.

The point of retargeting the call instead is so that you can leave 0x472400 untouched, which lets you call the original version from C instead of tracking down the addresses of...

Notably, global_cost_mod[] and raw_mana_may_be_spent_as_color[] are commented out - these are Shandalar variables, correct?

No. They're the manalink versions, and must be set.

global_cost_mod[] is support for Gloom, and is still consulted by spell cost and charging, and plenty of probably-entirely-replaced (but maybe not) cards' activation costs too. I don't think it's still set to nonzero by anything, though. It's an array of int[8] starting at 0x60a50c.

raw_mana_may_be_spent_as_color[][] is what implements Sunglasses of Urza (and what you should be using for things like Celestial Dawn and Mycosynth Lattice and effects like Daxos of Meletis that let you "spend mana as though it were any color/type" to pay costs, instead of converting those costs to generic). It's an array of int[2][11] starting at 0x60a4b4, and there's already a declaration for it in engine.c.

if ((cards_data[inst->internal_card_id].extra_ability & EA_MANA_SOURCE) && !(inst->state & STATE_TAPPED))

This is why cards that can activate (or trigger, I suppose, for an animated and tapped Mana Flare) for mana without tapping can't successfully declare it. If you're already checking CAN_TAP_FOR_MANA() or at least !(inst->state & STATE_TAPPED) in all the EVENT_COUNT_MANA handlers that logically need it, you can fix them all in one fell swoop by removing the tapped check here.

Code: Select all

   for (player = 0; player <= 1; ++player){
      for (card = 0; card < active_cards_count[player]; ++card)
      { [...]
            if ((cards_data[inst->internal_card_id].extra_ability & EA_MANA_SOURCE) && !(inst->state & STATE_TAPPED))
               dispatch_event(player, card, EVENT_COUNT_MANA);

            switch (cards_data[inst->internal_card_id].id){ [...]
               case CARD_ID_PRISMATIC_OMEN:
               case CARD_ID_DRYAD_OF_THE_ILYSIAN_GROVE:
                  mana_colors_added[player] |= COLOR_TEST_ANY_COLORED;
                  break;

               case CARD_ID_JOINER_ADEPT:
               case CARD_ID_CHROMATIC_LANTERN:
                  if(!is_humiliated(player, card) ){
                     mana_colors_added[player] |= COLOR_TEST_ANY_COLORED;
                  }
                  break;

               case CARD_ID_URBORG_TOMB_OF_YAWGMOTH:
                  mana_colors_added[player] |= basic_subtype_to_color_test(get_hacked_subtype(player, card, SUBTYPE_SWAMP));
                  mana_colors_added[1-player] |= basic_subtype_to_color_test(get_hacked_subtype(player, card, SUBTYPE_SWAMP));
                  break;
[more cases like these]


Still not soon enough. What happens if a player's opening hand is, in order, a Plains, a Joiner Adept, a Forest, and various other cards? The Plains ends up as card_instances[player][0], the Adept in [1], and the Forest in [2] (and that won't be affected by which is played first or anything else); during count_mana(), the Plains will get EVENT_COUNT_MANA, then the Joiner Adept will update mana_colors_added[], then the Forest will get EVENT_COUNT_MANA, and only the last will be all colors. It's even worse with Urborg, Tomb of Yawgmoth - player 1's Tomb won't ever be seen in time for any of player 0's lands - and worse than that for Riftstone Portal, which is only checked after every card that it could possibly affect has been processed.

The reason this isn't sticking out like a sore thumb in your tests is because you aren't initializing mana_colors_added[]. If you're doing that at the beginning of each turn or something like that, then lands won't update if you Murder a Joiner Adept or Stone Rain an Urborg, Tomb of Yawgmoth or whatever. If you're doing it in something like recalculate_all_cards_in_play(), it'll break subtlely when count_mana() is called from reassess_all_cards().

You really do have to search these cards out before dispatching EVENT_COUNT_MANA for anything. The clean way to do it is to dispatch an event before the loops here. In any case, checking for CARD_IDs directly, especially for cards on the bf, is a last resort after you've already made herculean efforts to avoid it. Formally, it's a layering violation; practically, it makes it impossible to add a functional reprint of a card just by assigning it the same implementation function, and depending where you're doing it it can break the becomes-a-copy-of-a-permanent mechanic. (It certainly breaks cards that copy abilities, though the only ones I can think of only copy "all activated abilities" or "all loyalty abilities".)

Also, none of these honor loss of abilities.

Also, I'm unclear why invisible permanents are checked for this.

Cards flagged both STATE_INVISIBLE and STATE_IN_PLAY are on the stack. They shouldn't be checked here. Use in_play() instead.

[drool66]

You don't need to do both.

Got it. Going with the jmp-only version.

global_cost_mod[] is support for Gloom, and is still consulted by spell cost and charging, and plenty of probably-entirely-replaced (but maybe not) cards' activation costs too. I don't think it's still set to nonzero by anything, though. It's an array of int[8] starting at 0x60a50c.

It looks to me like this is diminished & replaced with set_cost_mod_for_activated_abilities(). Gloom doesn't use it any more & it only shows up in magic-trace.c and patch_x_cost_artifacts.pl

raw_mana_may_be_spent_as_color[][] is what implements Sunglasses of Urza (and what you should be using for things like Celestial Dawn and Mycosynth Lattice and effects like Daxos of Meletis that let you "spend mana as though it were any color/type" to pay costs, instead of converting those costs to generic). It's an array of int[2][11] starting at 0x60a4b4, and there's already a declaration for it in engine.c.

Oh my gosh, I was running count_mana() from engine.c, but immediately below the section where this was defined. Thank you for pointing that out.

Still not soon enough.

I see what you're saying. (I had initialized my variables from produce_mana.c) What do you think of this?

Code: Select all

int mana_colors_added[2];
int mana_color_override[2][8];
int mana_quantity_override[2][8];

#define (a bunch of things including raw_mana_may_be_spent_as_color)

void count_mana(void){
   color_t c;
   for (c = COLOR_COLORLESS; c <= COLOR_ANY; ++c)
   {
//      global_cost_mod[c] = 0;
      raw_mana_available[0][c] = raw_mana_available[1][c] = 0;
      mana_color_override[0][c] = mana_color_override[1][c] = 0;
      mana_quantity_override[0][c] = mana_quantity_override[1][c] = 0;
   }

   raw_mana_available_hex[0][0] = raw_mana_available_hex[1][0] = -1;

   raw_mana_may_be_spent_as_color[0][0] = raw_mana_may_be_spent_as_color[1][0] = -1;

   mana_colors_added[0] = mana_colors_added[1] = 0;

   int player, card, mana_sources[2][MAX(active_cards_count[0], active_cards_count[1])], ms_count_0 = 0, ms_count_1 = 0;
   memset( mana_sources, -1, sizeof(mana_sources) );
   for (player = 0; player <= 1; ++player){

      for (card = 0; card < active_cards_count[player]; ++card)
      {
         card_instance_t* inst = in_play(player, card);
         if ( inst )
         {
            if ((cards_data[inst->internal_card_id].extra_ability & EA_MANA_SOURCE) && !(inst->state & STATE_TAPPED)){
               if( player == 0 ){
                  mana_sources[0][ms_count_0] = card;
                  ms_count_0++;
               }
               else{
                  mana_sources[1][ms_count_1] = card;
                  ms_count_1++;
               }
            }
            switch (cards_data[inst->internal_card_id].id){

               case CARD_ID_SUNGLASSES_OF_URZA:
                  if( !(inst->state & STATE_TAPPED))
                     dispatch_event_to_single_card(player, card, EVENT_COUNT_MANA, -1, -1);
                  break;

               case CARD_ID_GLOOM:
                  dispatch_event_to_single_card(player, card, EVENT_COUNT_MANA, -1, -1);
                  break;

               case CARD_ID_PRISMATIC_OMEN:
               case CARD_ID_DRYAD_OF_THE_ILYSIAN_GROVE:
                  mana_colors_added[player] |= COLOR_TEST_ANY_COLORED;
                  break;

               case CARD_ID_JOINER_ADEPT:
               case CARD_ID_CHROMATIC_LANTERN:
                  if(!is_humiliated(player, card) ){
                     mana_colors_added[player] |= COLOR_TEST_ANY_COLORED;
                  }
                  break;

               case CARD_ID_URBORG_TOMB_OF_YAWGMOTH:
                  mana_colors_added[player] |= basic_subtype_to_color_test(get_hacked_subtype(player, card, SUBTYPE_SWAMP));
                  mana_colors_added[1-player] |= basic_subtype_to_color_test(get_hacked_subtype(player, card, SUBTYPE_SWAMP));
                  break;

               case CARD_ID_STORMTIDE_LEVIATHAN:
                  mana_colors_added[player] |= basic_subtype_to_color_test(get_hacked_subtype(player, card, SUBTYPE_ISLAND));
                  mana_colors_added[1-player] |= basic_subtype_to_color_test(get_hacked_subtype(player, card, SUBTYPE_ISLAND));
                  break;

               case CARD_ID_REALMWRIGHT:
                  mana_colors_added[player] |= color_to_color_test( inst->targets[4].player );
                  break;

               default: break;
            }
         }
         inst = in_hand(player, card);
         if (inst)
         {
            if (cards_data[inst->internal_card_id].id == CARD_ID_ELVISH_SPIRIT_GUIDE ||
               cards_data[inst->internal_card_id].id == CARD_ID_SIMIAN_SPIRIT_GUIDE)
               dispatch_event_to_single_card(player, card, EVENT_COUNT_MANA, -1, -1);
         }
      }

      if( is_id_in_grave(player, CARD_ID_RIFTSTONE_PORTAL ) )
         mana_colors_added[player] |= COLOR_TEST_GREEN | COLOR_TEST_WHITE;
   }

   int source;
   for(source=0;source<ms_count_0;source++)
      dispatch_event(0, mana_sources[0][source], EVENT_COUNT_MANA);
   for(source=0;source<ms_count_1;source++)
      dispatch_event(1, mana_sources[1][source], EVENT_COUNT_MANA);

  // no cards still have code_pointer 0x413c80 or 0x422360, so the rest of this does nada; comment it out
/*  if (event_flags & EA_FELLWAR_STONE)
    for (int player = 0; player <= 1; ++player)
      for (int card = 0; card < active_cards_count[player]; ++card)
        {
          card_instance_t* inst = get_card_instance(player, card);
          if (inst->internal_card_id != -1
              && (cards_data[inst->internal_card_id].code_pointer == 0x413c80   // asm_card_star_compass()
                  || cards_data[inst->internal_card_id].code_pointer == 0x422360)       // asm_card_fellwar_stone()
              && in_play_or_in_play_and_invisible(player, card)
              && !(inst->state & STATE_TAPPED))
            dispatch_event_to_single_card(player, card, EVENT_VARIABLE_MANA_SRC, -1, -1);
        }*/
   return;
}

The clean way to do it is to dispatch an event before the loops here.

So have count_mana() dispatch a new event, say EVENT_MOD_MANA, and have, Urborg, for example, include:

Code: Select all

if(event == EVENT_MOD_MANA){
                      mana_colors_added[player] |= basic_subtype_to_color_test(get_hacked_subtype(player, card, SUBTYPE_SWAMP));
                  mana_colors_added[1-player] |= basic_subtype_to_color_test(get_hacked_subtype(player, card, SUBTYPE_SWAMP));
}


Shoot, that makes a lot of sense. How do you deal with timestamps in that case, say if Contamination and Hall of Gemstone are in play, or can you point me to an example? I tested cloning, using Vesuva on Urborg, Tomb of Yawgmoth, and Clone on Stormtide Leviathan, and they seem to be ok when the original leaves the bf.

Also, none of these honor loss of abilities.

This was intentional. My reasoning is that because of the layers system, the cards that change types (eg. Urborg, Prismatic Omen) apply their effects even when they themselves have lost all abilities. Cards that simply add mana abilities (eg. Chromatic Lantern, Joiner Adept) are checked for is_humiliated. (if we had an Yixlid Jailer flag, I would check Riftstone Portal against that)

Cards flagged both STATE_INVISIBLE and STATE_IN_PLAY are on the stack. They shouldn't be checked here. Use in_play() instead.

Ok, that's what I thought.

[Korath]

So have count_mana() dispatch a new event, say EVENT_MOD_MANA, and have, Urborg, for example, include:

Code: Select all

if(event == EVENT_MOD_MANA){
                      mana_colors_added[player] |= basic_subtype_to_color_test(get_hacked_subtype(player, card, SUBTYPE_SWAMP));
                  mana_colors_added[1-player] |= basic_subtype_to_color_test(get_hacked_subtype(player, card, SUBTYPE_SWAMP));
}


Shoot, that makes a lot of sense. How do you deal with timestamps in that case, say if Contamination and Hall of Gemstone are in play, or can you point me to an example?

dispatch_event(), and all its variants except the ones that say they only go to a single card, call card functions in timestamp order. That's one of the main reasons to call something from the dispatch_event() family instead of just writing a pair of for loops. (The other is that they stash certain global variables like affected_card_controller before calling card functions and restore them afterwards so that it's... mostly... safe to call dispatch_event_*() from other event handlers.) The downside is that it's always sent to all cards that aren't flagged STATE_OUBLIETTED and are flagged either STATE_INVISIBLE or STATE_IN_PLAY or both, and you can't change that without adding special cases for your event in dispatch_event_raw().

So you'd just have card_contamination() say "if (event == EVENT_MOD_MANA) override_color = COLOR_BLACK" (and sleight it, and deal with changing the amount too, and doing both players, and so on), and card_hall_of_gemstone() say "if (event == EVENT_MOD_MANA) override_color = whatever_was_picked", and whichever card has the latest timestamp sets the global last.

Or - especially if you're trying to compute a value that's unsafe to change until every card's handled the event, or if any of your handlers might themselves call a dispatch_event() variant - you can set your values in event_result instead, which will get returned by dispatch_event_*(). EVENT_DAMAGE_REDUCTION is a fairly minimal example of that.

This assumes that you want to evaluate them in timestamp order, of course. Normally replacement effects get whichever order the player whose object is being affected wants, so with a Contamination and a Hall of Gemstone set to green on the bf, each player gets to pick whether their Mishra's Workshop adds that's spendable only on artifacts or that's spendable only on artifacts every time they tap it. (Or maybe it's always only one mana, no matter the order you replace in. And Hall of Gemstone doesn't affect colorless mana. And Manalink can't represent mana with restrictions or additional abilities anyway other than artifact-spell-only colorless. Meh. Bad example, but you know what I mean.) Replacement effects are usually done with the trigger interface in Manalink. I think it'd be a usability nightmare here, and good luck getting the AI to deal with it.