mtgatracker / mtgatracker

MTGATracker is a deck tracker for MTG Arena, offering an in-game overlay that shows real time info about your deck in MTGA. It can also record & analyze your past matches to show personal aggregated gameplay history information, like lifetime wins/losses by deck, by event, etc.
https://mtgatracker.com
Other
259 stars 64 forks source link

Double-faced rare cards count multiple times towards missing collection #499

Closed Spencatro closed 5 years ago

Spencatro commented 5 years ago

image

Related to node mtga module, which will need some way to note if a card is double sided, so the progress calculator can know as well.

leestran1995 commented 5 years ago

I'm new around these parts but I'd love to dive into this when I get a chance tomorrow!

Spencatro commented 5 years ago

Howdy! I can think of at least three places that will require code changes, but possibly more.

  1. node-mtga objects will need a new member / property to distinguish if a card is "collectible" or not (example below)
    guilds_of_ravnica.addCard(new Card({ mtgaID: 68682, setNumber: 221, name: "assure__assemble", prettyName: "Assure // Assemble", cardType: "Instant Instant", set: "GRN", subTypes: "", colorIdentity: ['W', 'G'], colors: ['White', 'Green'], rarity: "Rare", cost: ['(G/W)', '(G/W)', '4', 'G', 'W'], collectible: true }))
    guilds_of_ravnica.addCard(new Card({ mtgaID: 68683, setNumber: 221, name: "assure", prettyName: "Assure", cardType: "Instant", set: "GRN", subTypes: "", colorIdentity: ['G', 'W'], colors: ['White', 'Green'], rarity: "Rare", cost: ['(G/W)', '(G/W)'], collectible: false }))
    guilds_of_ravnica.addCard(new Card({ mtgaID: 68684, setNumber: 221, name: "assemble", prettyName: "Assemble", cardType: "Instant", set: "GRN", subTypes: "", colorIdentity: ['G', 'W'], colors: ['White', 'Green'], rarity: "Rare", cost: ['4', 'G', 'W'], collectible: false }))
  2. The generation script for those objects is the best way to do it (instead of doing it manually)
  3. Since the node objects are derived from the python source, those will have to be updated first
  4. But again, those changes should live in the generation script (which tbh may or may not work in it's current state).

Or, if we can agree on what the end solution should look like in JS (since that's all that's really needed for this issue), we could skip all those steps, manually hack in the collectibility status of all existing cards, and then:

  1. In collectionRenderer.js do a if (!thisCard.collectible) continue; here.

That said, manually adding in collectibility status sounds like a major, manual, pain in the butt.

leestran1995 commented 5 years ago

Wow! Thanks for the pointers on where to start!

After poking around a bit I have great news, the json that the scripts are parsing (referenced here) includes an "isCollectible" flag! Seen here for Explosion // Expansion image

So (assuming the JS generation script does in fact work) we should be able to pull that from the JSON when we first parse through it and pass it along so it ends up in the final card list, and add the check to collectionRenderer.js.

I'll start working on it!

Spencatro commented 5 years ago

Whoops, probably should have warned you: this is going to involve publishing packages to pypi and npm, so I also started working on it this morning: https://github.com/mtgatracker/python-mtga/commit/9d0c861618fb5cb3a7214aed3855e24c8da95cf7 . Want to double check this while I generate the JS changes?

leestran1995 commented 5 years ago

I'm not exactly sure how to publish packages to pypi and npm so I guess it's good you took the reins! I'll keep an eye on everything (and I'm also online in the discord) and see if there's anything else I can take a stab at

Spencatro commented 5 years ago

Publishing isn't hard, but it requires my password ;)

npm package is published at 1.3.4. I'm going to take a break and eat something, if you want to play around with it and see if you can get the tracker changes in!

Spencatro commented 5 years ago

oh, you might also need the pypi package, so it is also now uploaded at 0.5.5

Spencatro commented 5 years ago

fixed in #500