mtgjson / mtgjson3

MTGJSON repository for Magic Cards
http://mtgjson.com
Other
545 stars 102 forks source link

Nemesis wrong set code #156

Closed fenhl closed 6 years ago

fenhl commented 8 years ago

The official set code for Nemesis, as used on wizards.com, is NEM, not NMS.

lsmoura commented 8 years ago

This is stated on the JSON file itself.

  "code": "NMS",
  "gathererCode": "NE",
  "magicCardsInfoCode": "ne",
  "oldCode": "NEM",
fenhl commented 8 years ago

What's the source for the current code being NMS?

lsmoura commented 8 years ago

NMS is the code used on mtgjson. I don't really know if it ever was NMS, but I don't believe it's worth changing mtgjson set name at this point.

tooomm commented 8 years ago

Maybe @Sembiance knows why we have a different code for Nemesis?

Just by logic, it seems bad to have some custom variance just for a single set. I can imagine this affects and breaks many services that use your file, because they expect it to use the official codes for all sets (which you do in all other cases,no?) ?

dgant commented 8 years ago

There are reasons to change it, and reasons to keeping it the same.

The downside to fixing it is potentially breaking existing clients by changing data which could reasonably be assumed to be invariant. The MTGJson set code is the best ID for set data (that I know of, at least), since it's unique by set and every set has one.

Sembiance commented 8 years ago

Greetings

I don't have any specific historical notes as to why I chose NMS for the MTGJSON Nemsis code, nor do I remember anything specific about this set.

Back then, when choosing which code to use, I used the set code from gatherer.wizards.com whenever possible. However for older sets (including Nemesis) Gatherer only used two-character names, "NE" in this case. So for those sets I just used whatever 3-character code was most popular/common in other websites, wikis and MTG software.

Today, I just found this article: http://magic.wizards.com/en/articles/archive/ask-wizards-august-2004-2004-08-02

I wasn't aware of this article until just now, but it appears to be an official set of codes from Wizards for older sets, including Nemesis which is indeed listed as 'NEM' instead of 'NMS'.

As @dgant mentioned though, changing it now would cause some big headaches for existing MTGJSON consumers, especially since the 'id' field is based on setCode, so that would cause these to be different as well. One could introduce a new field 'officialCode' that is only present if different than "code" and set it for nemesis. Just a judgement call I guess for those currently running the mtgjson project :)

fenhl commented 7 years ago

Given that the set code for Commander's Arsenal was fixed in 2.8.3 (a minor release, a year after the set was added to MTG JSON), I think this issue should be revisited.

thg2k commented 6 years ago

+1 to reopen the bug for discussion at least

fenhl commented 6 years ago

@ZeldaZach: I'd be very much in favor of this, but leave the decision on reopening to you.

ZeldaZach commented 6 years ago

So looking at it again and seeing how we've done this in the past, I'd be in favor of reopening this issue, but we'd have to keep backwards compatibility intact. As such, I'm proposing to reopen this issue with the note that it will be implemented in v4

fenhl commented 6 years ago

Of course, when we do fix this, we should also fix Portal Second Age.

thg2k commented 6 years ago

Maybe make use of git branches for this? So we can fix it already and start addressing the issues.

I don't see anything wrong in also having a "compat" and "stable" separate release channels.. like 3.x and 4.x (beta maybe?) for early adopters..

ZeldaZach commented 6 years ago

@thg2k https://github.com/mtgjson/mtgjson4

thg2k commented 6 years ago

A different repository? I didn't see that coming.. why did you choose to do so instead of branches? That means separate issues and more difficult merging and cherry-picking :(

ZeldaZach commented 6 years ago

You can ask @lsmoura Why we're on a separate one, not sure myself

tooomm commented 6 years ago

See https://github.com/mtgjson/mtgjson4_temporary/issues/52