Westvale Abbey and Ormendahl, Profane Prince have no CMC instead of CMC=0

[bakert]

All the other lands got fixed up recently but I guess being a double-faced card prevented this guy getting included.

[Frankacy]

I've also found that all Deckmaster (DKM) lands haven't been update either.

(Those, along with Westvale Abbey and Ormendahl, seem to be the only cards missing CMC. I've run through the rest)

[bakert]

I'm pretty sure what's going on here is a small problem with the broader fix for this issue in mtgjson/mtgjson@26ad621 aka Pull Request #413 by @gwax

This code has the double-faced card check the other side of itself for a cmc and use it if found.
Then it sets all remaining cards (including all lands) that don't currently have a cmc to cmc=0.

Because we check for the other face's cmc before setting lands to 0 it finds no cmc set when it does the first check. However if reverse the order of these checks the reverse of all dfcs will be set to cmc=0 before the double-faced check and it won't work.

I don't know this code well enough to say but is it acceptable to check if a card is a land? Or are we type-agnostic at this point in the code? One possibility is to apply the cmc=0 fix before the but only apply it where type='land'. Then the dfc fix will do the right thing for each card.

Another alternative is to set cmc=0 at the point where we slurp in lands initially and then only have the dfc fix at this stage. Not sure how I'd go about that but might be nicer.

@gwax or anyone – correct anything I've got wrong above and I'll send a pull request? Can I just check card.type contains 'land' here or is it not that simple? (Or feel free to fix :D)

[gwax]

There's a lot of funny ordering of operations in the code so I'm not surprised to see an edge case.

My inclination would be to preprocess and set CMC=0 for all unspecified cards (lands mostly) earlier in the process and then let everything else work itself out.

I'm happy to review if a PR is sent my way but I doubt that I'll have time to dig in myself.

[bakert]

#486 is my pull request. I think it needs some help getting over the line from an expert.

[bakert]

Fixed by #486 and verified for Ormendahl - closing. @Frankacy you might want to check this closes out the Deckmaster issue too?