Skip to content
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

The CMC of a land is 0 not NULL #138

Closed
bakert opened this issue Oct 10, 2016 · 7 comments
Closed

The CMC of a land is 0 not NULL #138

bakert opened this issue Oct 10, 2016 · 7 comments
Labels
+ bug * magic Magic-related code module / upstream

Comments

@bakert
Copy link
Member

bakert commented Oct 10, 2016

I'm not sure about Planes or Avatars, NULL kind of makes sense there. But a land doesn't have an undefined CMC. It has a well-defined CMC of 0.

This is a bug in the mtgjson data. They have a ticket open that has not been particularly well received: mtgjson/mtgjson#118

This ticket is to prompt us to either (best) fix upstream or add a workaround.

@bakert
Copy link
Member Author

bakert commented Oct 10, 2016

Actually, I already work around this is cmc<2 and cmc>=0 etc. so it's not really worth having our own ticket. Ignore.

@bakert bakert closed this as completed Oct 10, 2016
@silasary
Copy link
Member

This has been resolved upstream (3.10.2). We should remove our workaround.

@bakert bakert reopened this Jul 25, 2017
@bakert
Copy link
Member Author

bakert commented Aug 2, 2017

There's still two NULL CMC faces.

 mysql> SELECT name FROM face WHERE cmc IS NULL;
 +---------------------------+
 | name                      |
 +---------------------------+
 | Westvale Abbey            |
 | Ormendahl, Profane Prince |
 +---------------------------+
 2 rows in set (0.00 sec)

I've logged mtgjson/mtgjson#435

@bakert
Copy link
Member Author

bakert commented Aug 2, 2017

Diff to remove our hack when we're ready is just:

diff --git a/magic/multiverse.py b/magic/multiverse.py
index 895f5851..afba5e49 100644
--- a/magic/multiverse.py
+++ b/magic/multiverse.py
@@ -91,8 +91,6 @@ def update_database(new_version):
     for _, s in sets.items():
         insert_set(s)
     check_layouts() # Check that the hardcoded list of layouts we're about to use is still valid.
-    # mtgjson thinks that lands have a CMC of NULL so we'll work around that here.
-    db().execute("UPDATE face SET cmc = 0 WHERE cmc IS NULL AND card_id IN (SELECT id FROM card WHERE layout IN ('normal', 'double-faced', 'flip', 'leveler', 'token', 'split'))")
     rs = db().execute('SELECT id, name FROM rarity')
     for row in rs:
         db().execute('UPDATE printing SET rarity_id = ? WHERE rarity = ?', [row['id'], row['name']])

@bakert bakert added * magic Magic-related code module / upstream + bug labels Aug 2, 2017
@bakert
Copy link
Member Author

bakert commented Nov 1, 2017

I commented on the task I created with a probably-good idea of how to fix it. If the mtgjson team don't respond I'll send a pull request.

@bakert
Copy link
Member Author

bakert commented Nov 9, 2017

Submitted a PR for this mtgjson/mtgjson#486

@bakert
Copy link
Member Author

bakert commented Nov 9, 2017

PR was merged and released.

412abf5 removes our workaround.

@bakert bakert closed this as completed Nov 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
+ bug * magic Magic-related code module / upstream
Projects
None yet
Development

No branches or pull requests

2 participants