[fixed]Liquimetal Coating destroy artifact interaction

[gnomefry]

Describe the Bug:

After using Liquimetal Coating to add artifact to a planeswalker's types, some destroy artifact effects can destroy the planeswalker. Others cannot.

A Viashino Heretic ability can destroy the artifacted PW.

The instant Shatter can destroy it. Ditto for Shattering Pulse.

Hoard-Smelter Dragon ability can also destroy the artifacted PW.

On the other hand

Acidic Slime enter the battlefield effect cannot destroy it

Sunder Shaman combat ability is unable to destroy it

Nullmage Shepherd cannot destroy it. (Like the previous two, it claims 'illegal target')

Gorilla Shaman fails as well. No message -- when you choose the PW it cancels your activation process.


Which update are you using? (date, name)Which type? (duel, gauntlet, sealed deck)

Holidays 2021 ad1a6f8 - gauntlet

What exactly should be the correct behavior/interaction?

Destroy the artifact

Are any other cards possibly affected by this bug?

Ugh. No doubt many.

[Aswan jaguar]

I confirm that all cards that can also target enchantments besides artifacts, find planeswalkers that were made also artifacts, as illegal targets.

Gorilla Shaman fails as well. No message -- when you choose the PW it cancels your activation process.

Gorilla Shaman works fine you just didn't have enough mana to destroy the target so it fizzled. If you have enough mana it will destroy the artifact planeswalker.

[drool66]

It has to do with how planeswalkers are handled in is_what() and get_type_with_iid(). I honestly don't know if there is a perfect solution, but I think I've come on a better solution:

Code: Select all

   int typ;
   if( player == -1 ){
      typ = cards_data[card].type;
   }
   else{
      typ = get_card_data(player, card)->type;
   }

   if ((test_type & TARGET_TYPE_PLANESWALKER) && is_planeswalker(player, card)){
      return 1;
   }

   if (test_type & TYPE_ENCHANTMENT){
      if (player != -1 && (typ & TYPE_PERMANENT) && check_special_flags2(player, card, SF2_ENCHANTED_EVENING)){
         typ |= TYPE_ENCHANTMENT;
      }
      else if ((test_type & TYPE_PERMANENT) != TYPE_PERMANENT && is_planeswalker(player, card)){
            typ &= ~TYPE_ENCHANTMENT;
      }
   }

I think the only thing this misses is PWs that are turned into enchantments by something that uses create_a_card_type() rather than SF2_ENCHANTED_EVENING. The only thing I can think of off the top of my head that would do that would be One with the Stars on a Gideon, but I'm sure there are at least a few more. Anyone mind checking the logic?

[Korath]

There's a couple cards like Mirage Mirror that target an "artifact, creature, enchantment, or land" that'll look like a TYPE_PERMANENT and thus improperly be let through for a monotype planeswalker.

Making planeswalkers be their own type in Shandalar was orders of magnitude less work than piggybacking it on enchantments has been in Manalink, though I'm afraid I don't remember the specifics. It's worth experimenting with.

[drool66]

It looks like I can clear the "Subtype" and "Subtype 2" fields in Manalink.csv (easy), mark all the planeswalkers & tribals | 1<<8 / 1<<9 respectively in "card type" (mildly time-consuming), change the card_data_t and type_t structs to match Shandalars in that regard(easy), and redefine legendary to read "new types" & 0x2001 (easy). Does that sound about right?

[Korath]

Manalink.csv yields card_ptr_t, not card_data_t. card_data_t::subtype is the "Family" column in ct_all.csv; it's currently used in C for legendary and enchant world, and in numerous places in Magic.exe for Dwarf (Goblins of the Flarg), Urza's (the three lands in the Urzatron), Zombie (Zombie Master), basic land types (Sanctimony, targeting), and in some effect card functions which may or may not still be used by something.

Replacing the data is easy. That's just a matter of updating and running xml-to-csv.pl and doing the usual manual cleanup after; I should really make my documentation for that legible. Doing the whole thing manually isn't worth consideration.

The suggestion to make legendary be new_field & 0x2001 is perplexing for a number of reasons. Why give it two bits? Why the overlap with the Modifies Casting Cost column, which is new_field & 0x1? And the field's only eight bits wide, so & 0x2000 will always be unset. Oh, new types & 0x2001. Rather, new types == 0x2001, so just look for SUBTYPE_LEGEND in its usual place, card_ptr_t::types[]. It and SUBTYPE_WORLD should already be properly set for everything.

The time-consuming part - besides removing the remaining exe uses of card_data_t::subtype - is removing the reams and reams of subhacks supporting the original planeswalkers-are-enchantments-with-an-extra-flag-somewhere-else hack. Expect a bunch of regressions.

[drool66]

Manalink.csv yields card_ptr_t, not card_data_t. card_data_t::subtype is the "Family" column in ct_all.csv; it's currently used in C for legendary and enchant world, and in numerous places in Magic.exe for Dwarf (Goblins of the Flarg), Urza's (the three lands in the Urzatron), Zombie (Zombie Master), basic land types (Sanctimony, targeting), and in some effect card functions which may or may not still be used by something.

Ok, I think I've got all of these covered to now check card ptr subtypes, and anything left that uses exe targeting shouldn't use any of the card data subtypes, but yeah, hard to be 100% sure.

Replacing the data is easy. That's just a matter of updating and running xml-to-csv.pl and doing the usual manual cleanup after; I should really make my documentation for that legible. Doing the whole thing manually isn't worth consideration.

I have changes ready for xml-to-csv.pl & ct2exe.c, but I can't test them because I get an error in fetch-gatherer.pl for Henzie "Toolbox" Torrie that I can't figure out how to bypass or fix. (I think I remember there's special consideration for Kongming "Sleeping Dragon" somewhere, just not sure where) I've made local changes to manalink.csv & ct_all.csv using macros, but that's obviously unsustainable.

he time-consuming part - besides removing the remaining exe uses of card_data_t::subtype - is removing the reams and reams of subhacks supporting the original planeswalkers-are-enchantments-with-an-extra-flag-somewhere-else hack. Expect a bunch of regressions.

I'm getting there; almost done I think. deck.dll and drawcardlib.dll were time-consuming.

[Korath]

Ok, I think I've got all of these covered to now check card ptr subtypes, and anything left that uses exe targeting shouldn't use any of the card data subtypes, but yeah, hard to be 100% sure.

The targeting-related part is called from C as well. It's in sub_47EF10(), which looks like

Code: Select all

int
sub_47EF10(int player, int card)
{
  return (cards_data[get_card_instance(player, card)->internal_card_id].subtype == 13) ? 1 : 0;
}

Whether the call is dead or not is anyone's guess.

I have changes ready for xml-to-csv.pl & ct2exe.c, but I can't test them because I get an error in fetch-gatherer.pl for Henzie "Toolbox" Torrie that I can't figure out how to bypass or fix. (I think I remember there's special consideration for Kongming "Sleeping Dragon" somewhere, just not sure where) I've made local changes to manalink.csv & ct_all.csv using macros, but that's obviously unsustainable.

Commit your changes to a wip/ branch and I'll try to find time to take a look.

I don't think ct2exe should actually need to change - we can just use a plain number for the upper short of Type, née Family, rather than separating it out into the easier-to-edit one-bit-per-column of the lower short. This'll also make it a little cleaner to revert the whole system should that prove necessary.

I don't think there's any special cases for Kongming left, nor the other cards with quotes in them (Pang Tong, "Young Phoenix"; "Ach! Hans, Run!"; and "Rumors of My Death . . ."). I didn't have to do anything special for Henzie when building the New Capenna csvs, though I did watch for it. This suggests a toolchain issue on your end; what's the exact error message?

[drool66]

what's the exact error message?

Code: Select all

set=Streets of New Capenna Commander
open 'html/NCC/Henzie "Toolbox" Torre.html': Invalid argument at fetch-gatherer.pl line 1741

Gatherer wouldn't respond to pings from perl on WSL so I installed Strawberry Perl & ran from Powershell.

I don't think ct2exe should actually need to change - we can just use a plain number for the upper short of Type, née Family, rather than separating it out into the easier-to-edit one-bit-per-column of the lower short. This'll also make it a little cleaner to revert the whole system should that prove necessary.

You mean keep the ct_all fields consistent, but now the "Family" field = 256(ie. 1<<8) for Planeswalker and 512 for Tribal? That sounds good. I didn't dig too deep into Shandalar but it looks like you have some information encoded in those final six bits too, so that might be useful later on. So we keep the card_data struct as-is and let ::type bleed two bits into ::subtype. If that is the case I would probably want to rename ::subtype to something like "other_types".

[drool66]

Commit your changes to a wip/ branch and I'll try to find time to take a look.

Of course I messed this up. Very sorry, I thought I had it right.

[Korath]

OK. It's just the last four commits that were intended not to be on manalink/master, correct? I've reverted those ("git revert 76ee56816..ea0b18380"), switched to a new branch ("git checkout -b wip/type-planeswalker"), and re-committed on that branch ("git cherry-pick d3b3832b9", "git cherry-pick fc3547737", "git cherry-pick fb80004ab", "git cherry-pick dfd1b3d4c").

The error with Henzie is indeed a toolchain problem; it looks like Strawberry Perl, or maybe just your installation of it, isn't able to create filenames with quotation marks in them. Probably not anything else disallowed in basic dos filenames, either. You can work around it by removing 'Streets of New Capenna Commander' from %followup; sets only need to be there for one run of fetch-gatherer.pl in order to generate new flavor text (so you can remove 'Streets of New Capenna' too), and individual card files won't be created otherwise. The real solution's to make sure the cache files are named legally, of course.

If card_data_t::type is widened to 16 bits over subtype, and ct_all.csv is left in the same format, then bit 9 of type is bit 1 of Family, and bit 10 is 2.

[drool66]

I got everything working and committed in 2a0c6f2

[drool66]

Everything here tests ok, marking it fixed