-
Notifications
You must be signed in to change notification settings - Fork 798
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC] Refactor promo sets, add missing sets as listed on Scryfall #6190
Conversation
462315c
to
6de35a7
Compare
If set have non booster cards then you must setup
Yes, you must create one file per set. Java files loaded automaticity by client/server on startup and generates full sets/cards database. Database is basic source info about sets and cards. Moreover each set have different settings for boosters and other special code for booster generating (like special lands, partners, etc).
If scryfall marks it as standlone set then yes -- you can add it. All sets have SetType settings. That's uses for legality check in main formats. If you setup it with
Nope. Use that settings if set contains cards with SAME name (like multiple lands or normal and bonus versions of the card). You will get error in tests (MageVerify) if something wrong. MageVerify checks many settings for cards in sets. I strongly recommends to use it (IntelliJ can run that test from IDE -- just click on mage-verify project and choose run tests).
If you run by IDE then check java version in project settings. It must be 1.8 (java 8) only. Newer version do not supports. |
This comment has been minimized.
This comment has been minimized.
I'm pretty sure that as long as the expansion has the right |
If scryfall split it then xmage must do same (I known there are little amount of cards in that sets, but that's easy to support, e.g. for auto-generators, checks, images downloader, etc). When thats sets will be completed then I can modify sets search dialog to add filter (shows all or only basic sets).
As theelk801 above: xmage collects card names for legality checks. If any legal set contains card then you can use same card from any other set.
That's not critical. If one card in set then you can remove NON_FULL_USE_VARIOUS (e.g. you can remove it from battlebond's land). BTW: there are possible another settings like full art cards or special BFZ frame (that's info for M15 render mode).
Well, try to clean up:
That's error is system related. Maybe something wrong with java or paths. |
Battlebond has the tag because the automated script I used to generate the file always adds it to basics. |
3b03d2b
to
e71734d
Compare
Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/ScryfallImageSupportCards.java
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
I’ve seen the CI links, but normally I run them locally, because it’s faster. But now something else: mage/Mage.Verify/src/main/java/mage/verify/MtgJson.java Lines 28 to 50 in 5000020
The map translates pARL to ARENA . Now that ARENA is removed, I wanted to verify how MtgJson handles the other sets (PAL99 -PAL06 ). According to that block, the initial p is lower case.Then I downloaded the JSON document from https://mtgjson.com/json/AllSets.json.zip and loaded it into Python for inspection. I noticed that all sets are fully UPPER case, thus it seems that the translation is partly outdated.
I’ll add the verification of that code to the TODO list above. Maybe most of those items can be removed later. >>> import json, pprint, pathlib
>>> mtg_data=json.loads(pathlib.Path("./AllPrintings.json").read_text())
>>> for set_ in mtg_data.keys():
... if set_.startswith("p"):
... print(set_)
>>> pprint.PrettyPrinter(compact=True).pprint(list(mtg_data.keys()))
['10E', '2ED', '3ED', '4BB', '4ED', '5DN', '5ED', '6ED', '7ED', '8ED', '9ED',
'A25', 'AER', 'AKH', 'ALA', 'ALL', 'AMH1', 'ANA', 'APC', 'ARB', 'ARC', 'ARN',
'ATH', 'ATQ', 'AVR', 'BBD', 'BFZ', 'BNG', 'BOK', 'BRB', 'BTD', 'C13', 'C14',
'C15', 'C16', 'C17', 'C18', 'C19', 'CED', 'CEI', 'CHK', 'CHR', 'CM1', 'CM2',
'CMA', 'CMB1', 'CMD', 'CN2', 'CNS', 'CON', 'CP1', 'CP2', 'CP3', 'CSP', 'CST',
'DD1', 'DD2', 'DDC', 'DDD', 'DDE', 'DDF', 'DDG', 'DDH', 'DDI', 'DDJ', 'DDK',
'DDL', 'DDM', 'DDN', 'DDO', 'DDP', 'DDQ', 'DDR', 'DDS', 'DDT', 'DDU', 'DGM',
'DIS', 'DKA', 'DKM', 'DOM', 'DPA', 'DRB', 'DRK', 'DST', 'DTK', 'DVD', 'E01',
'E02', 'ELD', 'EMA', 'EMN', 'EVE', 'EVG', 'EXO', 'EXP', 'F01', 'F02', 'F03',
'F04', 'F05', 'F06', 'F07', 'F08', 'F09', 'F10', 'F11', 'F12', 'F13', 'F14',
'F15', 'F16', 'F17', 'F18', 'FBB', 'FEM', 'FNM', 'FRF', 'FUT', 'G00', 'G01',
'G02', 'G03', 'G04', 'G05', 'G06', 'G07', 'G08', 'G09', 'G10', 'G11', 'G17',
'G18', 'G99', 'GK1', 'GK2', 'GN2', 'GNT', 'GPT', 'GRN', 'GS1', 'GTC', 'GVL',
'H09', 'H17', 'HA1', 'HHO', 'HML', 'HOP', 'HOU', 'HTR', 'HTR17', 'HTR18',
'ICE', 'IMA', 'INV', 'ISD', 'ITP', 'J12', 'J13', 'J14', 'J15', 'J16', 'J17',
'J18', 'J19', 'J20', 'JGP', 'JOU', 'JUD', 'JVC', 'KLD', 'KTK', 'L12', 'L13',
'L14', 'L15', 'L16', 'L17', 'LEA', 'LEB', 'LEG', 'LGN', 'LRW', 'M10', 'M11',
'M12', 'M13', 'M14', 'M15', 'M19', 'M20', 'MB1', 'MBS', 'MD1', 'ME1', 'ME2',
'ME3', 'ME4', 'MED', 'MGB', 'MH1', 'MIR', 'MM2', 'MM3', 'MMA', 'MMQ', 'MOR',
'MP2', 'MPR', 'MPS', 'MRD', 'NEM', 'NPH', 'OARC', 'OC13', 'OC14', 'OC15',
'OC16', 'OC17', 'OC18', 'OC19', 'OCM1', 'OCMD', 'ODY', 'OE01', 'OGW', 'OHOP',
'OLGC', 'ONS', 'OPC2', 'OPCA', 'ORI', 'OVNT', 'P02', 'P03', 'P04', 'P05',
'P06', 'P07', 'P08', 'P09', 'P10', 'P10E', 'P11', 'P15A', 'P2HG', 'PAER',
'PAKH', 'PAL00', 'PAL01', 'PAL02', 'PAL03', 'PAL04', 'PAL05', 'PAL06', 'PAL99',
'PALP', 'PANA', 'PARC', 'PARL', 'PAVR', 'PBBD', 'PBFZ', 'PBNG', 'PBOK', 'PC2',
'PCA', 'PCEL', 'PCMD', 'PCMP', 'PCY', 'PD2', 'PD3', 'PDGM', 'PDKA', 'PDOM',
'PDP10', 'PDP11', 'PDP12', 'PDP13', 'PDP14', 'PDRC', 'PDTK', 'PDTP', 'PELD',
'PELP', 'PEMN', 'PF19', 'PF20', 'PFRF', 'PG07', 'PG08', 'PGPX', 'PGRN', 'PGRU',
'PGTC', 'PGTW', 'PHEL', 'PHJ', 'PHOP', 'PHOU', 'PHPR', 'PHUK', 'PI13', 'PI14',
'PIDW', 'PISD', 'PJAS', 'PJJT', 'PJOU', 'PJSE', 'PKLD', 'PKTK', 'PLC', 'PLGM',
'PLNY', 'PLPA', 'PLS', 'PM10', 'PM11', 'PM12', 'PM13', 'PM14', 'PM15', 'PM19',
'PM20', 'PMBS', 'PMEI', 'PMH1', 'PMOA', 'PMPS', 'PMPS06', 'PMPS07', 'PMPS08',
'PMPS09', 'PMPS10', 'PMPS11', 'PNAT', 'PNPH', 'POGW', 'POR', 'PORI', 'PPC1',
'PPOD', 'PPP1', 'PPRE', 'PPRO', 'PR2', 'PRED', 'PREL', 'PRES', 'PRIX', 'PRM',
'PRNA', 'PROE', 'PRTR', 'PRW2', 'PRWK', 'PS11', 'PS14', 'PS15', 'PS16', 'PS17',
'PS18', 'PS19', 'PSAL', 'PSDC', 'PSLD', 'PSOI', 'PSOM', 'PSS1', 'PSS2', 'PSS3',
'PSUM', 'PSUS', 'PTC', 'PTG', 'PTHS', 'PTK', 'PTKDF', 'PUMA', 'PURL', 'PUST',
'PVAN', 'PWAR', 'PWCQ', 'PWOR', 'PWOS', 'PWP09', 'PWP10', 'PWP11', 'PWP12',
'PWPN', 'PWWK', 'PXLN', 'PXTC', 'PZ1', 'PZ2', 'PZEN', 'RAV', 'REN', 'RIN',
'RIX', 'RNA', 'ROE', 'RQS', 'RTR', 'S00', 'S99', 'SCG', 'SHM', 'SLD', 'SOI',
'SOK', 'SOM', 'SS1', 'SS2', 'STH', 'SUM', 'TBTH', 'TD0', 'TD2', 'TDAG', 'TFTH',
'THB', 'THP1', 'THP2', 'THP3', 'THS', 'TMP', 'TOR', 'TPR', 'TSB', 'TSP', 'UDS',
'UGIN', 'UGL', 'ULG', 'UMA', 'UNH', 'USG', 'UST', 'V09', 'V10', 'V11', 'V12',
'V13', 'V14', 'V15', 'V16', 'V17', 'VIS', 'VMA', 'W16', 'W17', 'WAR', 'WC00',
'WC01', 'WC02', 'WC03', 'WC04', 'WC97', 'WC98', 'WC99', 'WTH', 'WWK', 'XLN',
'ZEN'] |
6500029
to
00f808f
Compare
This comment has been minimized.
This comment has been minimized.
d3b75b6
to
3779880
Compare
3779880
to
4dbd300
Compare
6a28872
to
900e130
Compare
This comment has been minimized.
This comment has been minimized.
Nope, do not add non english sets.
|
Found some TODOs in start topic:
Yes, if set don't have boosters then you don't need that settings.
If you can then yes. But it's not necessary. That's file uses for card code template generating and other perl scripts.
|
This comment has been minimized.
This comment has been minimized.
…yfall IP ban workaround for massive download)
I thought of a backwards compatibility issue with this: This PR removes and alters some sets. This will be incompatible with previously downloaded images. The removed sets won’t harm much, as their image archive will just linger on disk forever without being used. |
Yes, I know. It will be in what's new docs (about images clean up and/or re-download recommendations). It's ok if some old promo sets will be stores in zips/folders forever (until images folder clean up). |
* removed duplicated set (Clash Pack); * added images download for Eighth Edition Box, Ninth Edition Box;
…y, Launch Party, Media Inserts, Super Series)
* Duels of the Planeswalkers Promos set split to multiple sets (scryfall style); * Fixed Masterpiece Series sets name;
* Fixed broken direct links download; * Fixed outdated card numbers in some sets; * Fixed all non-downloadable images from scryfall source;
… card numbers then it will be fixed and saved automaticity);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now it finally done. XMage got ~200 new sets and ~6000 new reprints with that PR. Thanks for contribute.
During the conversation in issue #6180 @JayDi85 indicated that refactoring the old sets is possible, because it does not totally destroy already existing deck files. #6180 (comment)
So I started to go through all sets listed on Scryfall and add them to XMage, from old to new.
The intention is to fix all old sets and promotional releases.
Warning: total missing sets: 256, with missing cards: 11596
Progress report
TODO
These tasks contain a bit more work than simply adding a new class describing the set.
mage/Mage.Sets/src/mage/sets/Starter2000.java
Lines 25 to 29 in 68e5932
https://github.com/magefree/mage/blob/master/Mage.Verify/src/main/java/mage/verify/MtgJson.java
It seems that newer JSON documents now have fully UPPER case set abbreviations.
A static HashMap that provides a lookup-table to update the user decks should be sufficient.If Yes: https://scryfall.com/sets/ana https://scryfall.com/sets/pana https://scryfall.com/sets/ha1
Added sets
All Sets are added up to the recently announced SLU (Secret Lair: Ultimate Edition).
Exceptions
The Celebration Cards (PCEL) set is basically silver-bordered, except for two cards, which have actually functional and implementable rules text. These cards are illegal in all formats.
These non-English sets are not added. Adding these will allow players to force the display of non-English card images. Therefore I opted to omit them for now.
Notes
Please review the added classes, as they are mostly automatically generated (see below). I’ve checked them for plausibility, but I can’t guarantee that the automated process works in all cases. There are some bits I’m modifying in each class, like setting the Expansion type, booster content, and fixing all missing/wrongly generated references IntelliJ finds.
Adding all cards of a set by hand is tiresome and error prone (I tired out after 10 or so cards), so I’ve written a small script that can automatically generate Java class source files for XMage sets. It uses the card database code I’ve written for my project https://github.com/luziferius/MTGDeckConverter to build the class code. The script code used is included below for reference.
(Side note: Although the code I use is licensed under GPLv3+, the output of a GPL-licensed program, even if it is source code, is not covered by the GPL, so it is OK to include the generated code files in XMage.)
The database has to be seeded once, using a snippet like this: