remove entire column for transforming cards

davidchambers / tutor

JavaScript interface for the Gatherer card database

https://gatherer.wizards.com/

Do What The F*ck You Want To Public License

149 stars 18 forks source link

remove entire column for transforming cards #15

Closed nicknovitski closed 12 years ago

nicknovitski commented 12 years ago

I was wrong: looking at more cards (for example http://gatherer.wizards.com/Pages/Card/Details.aspx?name=Garruk,%20The%20Veil-Cursed), it seems that transforming cards are supposed to have rulings associated with the particular side they relate to, but some (including afflicted deserter/ransacker werewolf) just put everything on the left column.

nicknovitski commented 12 years ago

The alternative is to keep it as it is now, which will attach all rulings for both cards to each card.

davidchambers commented 12 years ago

I haven't yet wrapped my head around this.

It'd be great if you could resolve the merge conflict; I refactored this code in 7e7c6c8.

nicknovitski commented 12 years ago

Rebased.

Essentially, gatherer is inconsistent with how rulings are displayed with transforming cards. In the case of the Deserter/Ransacker, all of the rulings are written in the left column (cardComponent0), even ones that are clearly about the card in the right column (cardComponent1). But in the case of Garruk, the left column rulings are about the left column card, and the right column rulings are about the right column card.

When last we spoke, I had only looked at the Deserter/Ransacker, so took it as a representative case, but now I see that it's probably a data entry error of some kind.

So the choices are to either

keep it as it is in master now, where all rulings in both columns are returned with requests for either column, or
change it as in this pull request, where each request has separate associated rulings, but sometimes they're the wrong ones, or are missing entirely.

I'm leaning slightly towards the latter.

davidchambers commented 12 years ago

Do you have any idea of the number of cards that fall into each category? If there are just one or two weird ones I'm fine with this change, but if a significant proportion of transforming cards are affected I think it'd be better to include rulings for both sides in each response.

nicknovitski commented 12 years ago

Out of ~20 pairs of transforming cards which currently have one or more rulings, there 4 which, in my opinion, unambiguously exhibit this problem. One is afflicted deserter/werewolf ransacker, and the other three are:

davidchambers commented 12 years ago

Given that there are only a handful of weird cases, I'm happy to pull in these changes. Shall I go ahead and do so?

nicknovitski commented 12 years ago

Yeah, that's how I see it, too. Go ahead, I say.

nicknovitski commented 12 years ago

Wait! Woah! Hold on! Something's wrong with the diff.

nicknovitski commented 12 years ago

Okay, phew. I figured it out. Go ahead and merge. :)