magefree / mage

Magic Another Game Engine
http://xmage.today
MIT License
1.91k stars 777 forks source link

Refactor: many sets contains wrong block and parent set info #10184

Open JayDi85 opened 1 year ago

JayDi85 commented 1 year ago

Some sets contains wrong info about block (~134) and parent set info (~358). It's not a critical, but must be fixed someday. Block info used to group sets in the GUI choose dialog. Parent set info used to generate a booster.

Search that code to enable verify test:

Logs sample for wrong block:

Error: set with wrong blockName settings: 10E - Tenth Edition (blockName = null, but must be Core Set)
Error: set with wrong blockName settings: 2ED - Unlimited Edition (blockName = null, but must be Core Set)
Error: set with wrong blockName settings: 2X2 - Double Masters 2022 (blockName = Reprint, but must be null)
Error: set with wrong blockName settings: 2XM - Double Masters (blockName = Reprint, but must be null)
Error: set with wrong blockName settings: 3ED - Revised Edition (blockName = null, but must be Core Set)

Logs sample for wrong parent set:

Error: set with wrong parentSet settings: WTH - Weatherlight (parentSet = MIR, but must be null)
Error: set with wrong parentSet settings: WWK - Worldwake (parentSet = ZEN, but must be null)
Error: set with wrong parentSet settings: XANA - Arena New Player Experience Extras (parentSet = null, but must be ANA - ArenaNewPlayerExperience.getInstance())
Error: set with wrong parentSet settings: ZNC - Zendikar Rising Commander (parentSet = null, but must be ZNR - ZendikarRising.getInstance())
brunofpinheiro commented 1 year ago

Is anyone working on this? I would like to do it if that's ok.

xenohedron commented 1 year ago

I'm not aware of anyone working on this. Go for it!

xenohedron commented 1 year ago

Okay I'm starting to look at this and I don't think the mtgjson verify really corresponds to how the fields are used internal to xmage:

Block name

Parent set

So what would happen if we switch meaning of parent set to mtgjson meaning instead?

So I don't think we should change parent set without more research and possible refactoring (e.g. rename current field to basicLandsFromSet and check it manually). Block info should be safe to adjust.

xenohedron commented 1 year ago

I tested the GUI when applying just the block name adjustments. It's not an improvement, as all the recent sets without blocks get buried in the list rather than in chronological order. That part of GUI could use a rework to provide additional categorization.

JayDi85 commented 1 year ago

BTW deck editor contains outdated "sets choose dialog" with some limitation (it can't use non standard names for sets). But it has additional checkboxes mode. shot_230812_092230

Images download dialog contains modern "choose sets dialog" with any text usage and filter/search. So that blocks info can be added later in the new choose sets dialgs as additional text (like release date) or as additional combo/filter. shot_230812_092437