Add UUIDs to the Tokens File

[SlightlyCircuitous]

Progress towards #306. Fixes #306.

As discussed in the Discord, I have updated BruebachL's script to fallback to using the UUID found in the Scryfall URL if a match can't be found in MTGJSON, which means the vast majority of tokens will get unique UUIDs with this PR. This still leaves 78 tokens without UUIDs, because they either have no printing at all, a printing not on Scryfall, or custom token art.

Interestingly, these tokens actually seem to work fine in my testing, even the ones that have unique printings in other sets (see, for instance, the Black Cat token from APC). I am not sure how that would interact with custom tokens though, so maybe they still need unique UUIDs despite this. I am happy to adjust this PR to give them UUIDs or make a subsequent one, as needed.

I have attached the modified version of the script I used to generate the UUIDs. Thank you, BruebachL!

[SlightlyCircuitous]

This PR now adds a uuid property to each set line by taking the UUID from the Scryfall URL in all cases.

[SlightlyCircuitous]

I added sequential UUIDs (0, 1, 2, 3, etc) to token entries with art but no Scryfall URL since as I understand it, it doesn't super matter what the value is as long as it's unique. 11 tokens still don't have the uuid property, but they also don't have any art at all so I assume they don't actually need it. Have not seen any problems while testing so far.

[ebbit1q]

I'd prefer the made up uuids to be a bit more complex, in my opinion the client should be able to work without the uuid and send a different token like set num over the protocol, this'd make things much easier.

[SlightlyCircuitous]

Alright, I can make the made up UUIDs longer and put some dashes in or something. I just don't want to make them the same format as Scryfall's because I don't want to have to check the whole file for uniqueness.

I can definitely add the set number to the tokens file to support something like that, but I think changing the client protocol is beyond the scope of my current ability.

[ebbit1q]

this seems fine now, they don't necessarily need to have numbers and dashes, it's just an arbitrary string though

[SlightlyCircuitous]

Is there anything else you would like me to do with this PR or is it ready to go? for the made up ones, I suppose I could hash the name of the token instead of doing them sequentially if you wanted a less arbitrary identifier.

Also, I have a version of the tokens file with a num attribute containing the Collector Number (like the cards file), if you would like that in another PR. I assume the 67 cards with no art on Scryfall don't need this attribute because you couldn't do a lookup with it anywhere anyway.

[SlightlyCircuitous]

The made up UUIDs are now a sha256 hash of the token entry name (including spaces). Each should now be both deterministic and unique. This, of course, only works for one UUID per token entry, but there should never be a case where we need more than one made up UUID per entry.

[ebbit1q]

what's the benefit of hashing at all if the uuid could have been the input of the hash just the same?

[SlightlyCircuitous]

I was under the assumption that the UUIDs should be a number and you seemed unhappy with arbitrarily or sequentially assigned strings of numbers. I figured that hashing the name would give a UUID that was both non-arbitrary and a number. You also suggested hashing to create UUIDs, though you suggested hashing the props rather than the name.

I was also under the impression that it didn't actually matter what these made up UUIDs were and that I could just put anything, but that doesn't seem to be the case. Please just tell me exactly what you want the made up UUIDs to be and I will make them that. I would really just like to get support for printing selections for tokens in the xml.

[BruebachL]

I was under the assumption that the UUIDs should be a number and you seemed unhappy with arbitrarily or sequentially assigned strings of numbers. I figured that hashing the name would give a UUID that was both non-arbitrary and a number. You also suggested hashing to create UUIDs, though you suggested hashing the props rather than the name.

I was also under the impression that it didn't actually matter what these made up UUIDs were and that I could just put anything, but that doesn't seem to be the case. Please just tell me exactly what you want the made up UUIDs to be and I will make them that. I would really just like to get support for printing selections for tokens in the xml.

Ebbits point here, as far as I understand it, is that if you have a hashing function into which you only feed one variable, you have essentially just created a 1:1 mapping function, rather than a hash.

The point of hashing is usually to convert multiple variable length data points into one fixed length string (or number or hex representation of that number).

The main benefit of hashing is when two items share one or more specific attributes (name or set name for example) but differ in another (collector number) which then allows you to differentiate these items because they have a different hash (which would not be possible if you simply mapped name <-> hash value).

[ebbit1q]

I apologise, I'm not trying to be annoying or anything, your choice for the mapping of the uuid simply provided me with some insight, I thought about it and in my opinion the tokens that don't have an uuid simply don't need an uuid, for the same reason as you picked just the card name to hash as an uuid: they don't have any alternate printings and as such will all show as the same printing regardless

[ebbit1q]

the reason why I was unhappy with just sequential numbers is because they could lead to conflicts easily, the needs to be unique throughout the entire installation as far as I'm aware

[SlightlyCircuitous]

I was under the assumption that the UUIDs should be a number and you seemed unhappy with arbitrarily or sequentially assigned strings of numbers. I figured that hashing the name would give a UUID that was both non-arbitrary and a number. You also suggested hashing to create UUIDs, though you suggested hashing the props rather than the name.
I was also under the impression that it didn't actually matter what these made up UUIDs were and that I could just put anything, but that doesn't seem to be the case. Please just tell me exactly what you want the made up UUIDs to be and I will make them that. I would really just like to get support for printing selections for tokens in the xml.

Ebbits point here, as far as I understand it, is that if you have a hashing function into which you only feed one variable, you have essentially just created a 1:1 mapping function, rather than a hash.

The point of hashing is usually to convert multiple variable length data points into one fixed length string (or number or hex representation of that number).

The main benefit of hashing is when two items share one or more specific attributes (name or set name for example) but differ in another (collector number) which then allows you to differentiate these items because they have a different hash (which would not be possible if you simply mapped name <-> hash value).

Yeah, that makes sense. Was just kind of grasping at straws trying to respond to the comments, tbh.

[SlightlyCircuitous]

I apologise, I'm not trying to be annoying or anything, your choice for the mapping of the uuid simply provided me with some insight, I thought about it and in my opinion the tokens that don't have an uuid simply don't need an uuid, for the same reason as you picked just the card name to hash as an uuid: they don't have any alternate printings and as such will all show as the same printing regardless

It's OK. It was just a bit frustrating seeing this remain unmerged for so long and not really understanding why or what needed to change. But, I get that you were trying to make sure it didn't cause unexpected problems. I've removed the made up UUIDs entirely now.

[tooomm]

I see there was quite some progress made here! Nice.

The motivation seems to be the printing selection however.
I want to use the opportunity and remind that the token file was using the current, very weird workaround (adding spaces to token names to make them differ from each other and unique) for a quite a while.

With UUID's that would be solved and we discussed such a solution some time ago.
See #286 and Cockatrice/Cockatrice#5961.

Hashing names as they are right now would not help with unique ID's, and we can not change to correct token names without spaces.

Now that UUID's are worked on, can we solve two at one here and do UUID's in a way that we can use normal token names and reference them via their ID's instead in the client at one point?

[BruebachL]

Regardless of it being even possible right now (which I am not entirely sure of), doing this would necessitate extensive code changes which would extend beyond this PR and delay it even further.

(There's also the point of this sort of breaking inter-client compatibility once we start referencing things by other identifiers, which should be handled with care and deliberation)

[SlightlyCircuitous]

Regardless of it being even possible right now (which I am not entirely sure of), doing this would necessitate extensive code changes which would extend beyond this PR and delay it even further.

(There's also the point of this sort of breaking inter-client compatibility once we start referencing things by other identifiers, which should be handled with care and deliberation)

I agree. While I do find the spaces after the name solution a little clunky, it is really far outside of the scope of this PR (and even this repo) to make those changes. I would prefer to hold the scope of this PR to strictly supporting the existing code changes for getting tokens into the printing selector and closing the issue that I linked in the original post. I feel the same way about adding set num support to the file: I like the idea, and I can work on getting there, but it's something that can be thought about and done in other PRs after this is merged.

[NexGenKirabi]

I was meaning to ask about this I made a script that auto assigns uuids (starting at 100001 and going up per instance the set is mentioned) to the folder but I didn't know some tokens actually had their own already. The script appears to work but I wanted to know your thoughts.

[Psithief]

I was meaning to ask about this I made a script that auto assigns uuids (starting at 100001 and going up per instance the set is mentioned) to the folder but I didn't know some tokens actually had their own already.

If you're not already doing this, I'd recommend reading up on how to advertise your method of generating the uuid in the string itself.
https://en.wikipedia.org/wiki/Universally_unique_identifier#Format