-
Notifications
You must be signed in to change notification settings - Fork 741
Add new duplicate cards directory to deal with duplicate test cards and implement correct Fast // Furious card in UNK #8520
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
Conversation
I believe the current consensus is to use @Jetz72 's variant system (as seen for Garbage Elemental) to address these overlaps. The idea in that process is, in part, to fold all the cards in |
Has this been attempted before? I remember thinking these name collisions seemed a bit different from regular functional variants since the two cards with the same name were completely disparate from one another, unlike other cases where the variant represents a tweak to the original. They're also meant to have separate legalities; I'm hoping that proves resolvable when working on Alchemy stuff. |
Not that I know of. However in discussion with @paulsnoops about name overlaps, such as No Way Out (MID) and No Way Out (UNK) , I was given the impression the situation would be more straightforward to manage if card names weren't pre- or suffixed. But while I was at it, might as well try and "glue" two cards together. I do have a notion that the variant system can hew a bit closer in scripting practice to the alternate system: in having all-caps separator that doesn't need to be commented out and a flag that tells the game what to do. Something like
|
Maybe we should replace the name as a primary key and change it to a proper ID field? That way we can have multiple cards with the same name and not have a prefix or suffix in the way for CARDNAME and what not. |
For now, could we just remove Fast // Furious from Unknown Event as it's not actually the correct card in master? (and also moving steamflogger boss to eternal) |
…t//furoious to eternal under unsupported variant until variant specific legality is possible
Following up, I was able to implement both cards in one file using the variant system, however they don't seem to be separable in terms of legality; since it's in UKN, the only way to play with any variant is to enable non-legal cards. To be clear, my main goal with this PR was to fix the fact that the legal card fails to load when disabling non-legal cards. Implementing the correct test-card was just a nice addition on the side. If the preferred solution is a refactor of the card loading system (e.g. based on card ID instead of just name) the I will leave the addition of the test card to a later PR and just fix the legal Fast // Furious issue with this PR. To address this, I have the logic for UNK Fast // Furious under Variant $B however I'm leaving it as $UNSUPPORTED the UNK set to avoid loading an illegal card when non-legal cards are disabled, but I am moving said unsupported variant to the "eternal" category of UNK so as to not prevent the loading of the legal cards. This means UNK Fast // Furious will be unavailable in forge for the time being (no different from current state, as it's not the correct card), but can easily be added in when variant specific legality is implemented. |
@Jetz72 @dracontes Any stopping this from being merged? I think this implementation is fine until there's a better way to differentiate variants of the same card. |
sorry, just adding a reason beside the comment, quick re-approve? |
The purpose of this PR is to introduce a new duplicate cards folder to the cardsfolder directory for managing cards with duplicate names (none of which are tournament legal, but are still real cards).
The folder will include mostly test cards as they are implemented, such as:
Red Herring in Mystery Booster CMB1 (homonymous with MKM).
Pick your Poison in Mystery Booster CMB1 (homonymous with MKM)
Unquenchable Fury in Battle the Horde (homonymous with Neon Dynasty)
Fast // Furious in Unknown Event (homonymous with Modern Horizons 2 - MH2)
The last card, Fast // Furious, is the only one present in forge currently, however it was actually the MH2 card in functionality, both making it incorrect, and also causing the MH2 card to not load if one disabled non-legal cards in the settings (see issue link below).
My proposal is to adopt a similar naming convention to that of rebalanced alchemy cards by prepending T- to the card name. I have added the correct "Fast // Furious" playtest card under the name "T-Fast // T-Furious", since cards are loaded once no matter how many sets they appear in. This way, disabling non-legal cards doesn't prevent the loading of the tournament legal Fast // Furious.
Sidenote: also moved Steamflogger Boss to the eternal section of UST setlist so as to fix the issue linked below.
Fixes: #8495