taw / magic-search-engine

Search engine for Magic cards
MIT License
47 stars 19 forks source link

is:reprint doesn't match mtgjson for MH2 #194

Closed axxroytovu closed 1 year ago

axxroytovu commented 1 year ago

MTGJson includes an isReprint field in their files. Can this replace condition_is_reprint.rb?

MH2 rare cards are flagged as reprints even when they are not, because PMH2 was "released" earlier and the promo cards are being flagged as the original printing.

taw commented 1 year ago

What do they define as a "reprint" vs "not reprint"?

I don't recommend relying on is:reprint for anything, mtgjson data on release dates has been unreliable.

taw commented 1 year ago

By the way card age is currently defined in code as counting "first printing except PPRE set", due to past mtgjson issues (it used to be a mtgjson set with Prerelease promos, now it's split per-set). So I did a similar workaround before, the code just rot and no longer works.

taw commented 1 year ago

I also opened https://github.com/taw/magic-search-engine/issues/195 for related issues. There are probably a few more fields we might want to index.

axxroytovu commented 1 year ago

What do they define as a "reprint" vs "not reprint"?

I don't recommend relying on is:reprint for anything, mtgjson data on release dates has been unreliable.

It looks like they're pulling the reprint flag from Scryfall.

The reason I wanted to use the is:reprint tag is because the reprint/new boosterfun cards are mixed together for MH2 but appear in different slots. Soul Snare is conveniently placed at the end of the normal frame cards, but is nestled between Sanctifier en-Vec and Timeless Dragon for the old border cards. Manually pulling out each of the reprint boosterfun cards would be an absolute nightmare.

taw commented 1 year ago

There's pretty big difference between these two conditions (age>0, isReprint flag):

Sets affected by changes, losing flag:

Gaining flag:

taw commented 1 year ago

By the way are these the queries you're looking for?

axxroytovu commented 1 year ago

That would work, I didn't think of using Alt.

axxroytovu commented 1 year ago

After a spot check of your lists of gained/lost reprint flag, it looks like the scryfall version is the more accurate one. I haven't done an exhaustive check, but the flag does appear to be correct for all the ones I looked at.

taw commented 1 year ago

Well, their concept of a "reprint" is definitely not the obvious one. Like oc16 cards came literally in the same box as c16 cards, and somehow they're a "reprint" etc.

It looks like they just select something as first printing (earliest non-promo printing?), and mark every other printing as a reprint regardless of relative date.

Maybe that's more useful than literal "earliest printing by date".

taw commented 1 year ago

I restored is:reprint to its intended functionality it used to have:

This doesn't match mtgjson/scryfall, but I think this should match what I'd describe as a reprint.

It should deal with cases like prerelease promos, OCxx cards etc. correctly.

Are there any problems with this system? We have an option to switch to using mtgjson flag if this doesn't work.

axxroytovu commented 1 year ago

It seems like a good system. I would add "dedicated reprint sets" to the list of sets that cannot have first printings, like STA, BRR, DBL, or masters sets. I think the only card this should affect is Abundant Harvest, which chronologically appeared in STA before it's official "release" in MH2.

taw commented 1 year ago

I guess it would make sense to treat STA like other promo sets for this, but there's no flag for that like st:reprint yet.

What else would qualify for st:reprint? It's basically all done here, and that's really messy. It would be nice to clean it up at some point.

axxroytovu commented 1 year ago

I think the general consensus for the naming of the bonus sheet sets has been "masterpiece", so that might be a good thing to add. That would be STA, BRR, MUL, EXP, MPS, and ZNE

The other easy flag is "masters", which apart from Cryptic Spires has entirely been reprints.

Edit: There's probably no point in adding masters sets to the reprint logic. If they do print a new card in a masters set there's a bigger problem than just here so that should already be covered by the logic.

taw commented 1 year ago

I might clean it up at some point. A few of them are used in the code (like standard, modern, promo, funny, custom), a lot of them drive integration test logic (like booster, deck), some might be useful for people searching (like st:commander), and a lot more are basically pointless (I doubt anyone used st:"premiere shop" even once).

Like many things in the indexer, it's another leftover from mtgjson changing its mind multiple times.

If you need any specific addition to the list, feel free to PR it. Otherwise it's low priority cleanup task.

axxroytovu commented 1 year ago

Yeah it's low priority now that you've updated the logic above. MH2 seems to be working minus some bugs that I'm struggling with. Wizards why do you need to have rarity shifted borderless cards they're such a pain.

taw commented 1 year ago

I'll close it as good enough for now.