taw / magic-preconstructed-decks

Magic: the Gathering Preconstructed Decks
6 stars 9 forks source link

Improved printing inference #55

Closed davidmartos96 closed 1 year ago

davidmartos96 commented 1 year ago

Related to #53

I believe there is already some logic to pick the best printing for a particular deck without explicitly specifying it. Could that bug from the PR have been prevented with an improved logic? Maybe treating the main set (LTR) and the commander set (LTC) in the same group?

Maybe the same fix can handle both bugs that were detected, and hopefully remain useful in the future.

Starter Kit (LTR): Contains a copy from the LTC set Commander decks (LTC): Contain the basic lands from the main set LTR

taw commented 1 year ago

So LTR and LTC mtgjson data is still incomplete, so I didn't bother to investigate, and it looks like LTR/LTC just don't follow established patterns here.

If LTR precon deck contains LTC cards, I believe that's the first such case, at least it never happened on paper, and it's still Arena only thing right?

In the other direction, deck indexer's logic is:

This would normally already work except LTR is the first not Standard legal set with associated Commander set, so LTC searches is MOM itstead of LTR. I don't expert MH3 to have MC3 etc., so I don't think this will happen again.

Anyway, for any set where this logic doesn't work, deck indexer has explicit set search list for this, and it's not many exceptions. Right now (in indexer/lib/deck_printing_resolver.rb):

  SetSearchList = {
    # otherwise it returns GRN and that's bad
    "gnt" => ["m19"],
    "c20" => ["iko"],
    "znc" => ["znr"],
    "jmp" => ["m21"],
    # Coldsnap had special set for Ice Age reprints
    "csp" => ["cst"],
    # S00 on Gatherer includes all 6ED reprints but mtgjson doesn't
    "s00" => ["6ed"],
    # It seems that Masters Edition 2 precons contained Masters Edition cards too
    "me2" => ["me1"],
  }

Adding entry "ltc" => ["ltr"], would be the most sensible solution here. The other way around is so unusual that I'd just tag it explicitly in Arena product that does it.

davidmartos96 commented 1 year ago

@taw Thanks for the info! In this particular case though, I would not treat the 2 starter decks as Arena. Wizards now call these kind of products "Starter Kit" as is. They are paper decks. https://www.youtube.com/watch?v=CDk1n0X5Zmc

taw commented 1 year ago

Well, it looks like paper LTR deck also contains LTC card Graypelt Refuge.

If it's also available on paper, then the decks are miscategorized, and shouldn't be listed as Arena.

davidmartos96 commented 1 year ago

Starter Kit 2022 it's when they switched the naming. As it says in the wiki: "Previously named Arena Starter Kit" https://mtg.fandom.com/wiki/2022_Starter_Kit

taw commented 1 year ago

OK, type of SNC and LTR Starter Kits changed. Anything else that needs changing?

davidmartos96 commented 1 year ago

I don't think so

taw commented 1 year ago

If everything is good, I'll close this.