Unique ID mess in OpenTTD

unique-id-is-a-mess-proof-me-wrong.txt

UniqueIDs are named differently per content-type, so let's start with that:

NewGRF -> GRFID (char[4])
Scripts -> short_name (string of 4 letters)
Base Sets -> shortname (string of 4 letters)
Scenario -> .scn.id (a decimal value)
Heightmap -> .png.id (a decimal value)

They all have a slightly different way of storing their data, but they all end up as an uint32.

NewGRF GRFID:
https://github.com/OpenTTD/OpenTTD/blob/c8779fb311c2665d3fc45c18b2f3460cd998d179/src/newgrf.cpp#L6710
In NewGRFs it is stored as a uint32_t, where "ABCD" is stored on disk as 41 42 43 44, so becomes 0x44434241 (LE) as value.

https://github.com/OpenTTD/OpenTTD/blob/13cc8a0ceec90def39cbcb84135a0bf039793a6f/src/script/script_scanner.cpp#L239
In Scripts it is stored as a string. This is read in this special way. Here "ABCD" becomes 0x44434241.

https://github.com/OpenTTD/OpenTTD/blob/13cc8a0ceec90def39cbcb84135a0bf039793a6f/src/base_media_func.h#L58
In Base Sets it is identical to Scripts, just the variables are different. So here too, "ABCD" becomes 0x44434241.

https://github.com/OpenTTD/OpenTTD/blob/acb3d10832c92c9f93c3a4d50b00774274bac8c7/src/fios.cpp#L700
In Scenarios it is read as decimal. So the value "1234" becomes 0x000004d2.
Heightmaps are using the exact same code (despite the functions being called Scenario)


Now the issue: for BaNaNaS uploads, NewGRF unique_ids are byteswapped before storing. So "ABCD" is stored as 0x41424344.
The rest is stored as above.

https://github.com/OpenTTD/OpenTTD/blob/master/src/network/core/tcp_content.cpp#L120
This is the reason that works. Which ever came first, no clue. But it is wrong.



My suggestion is to add a bit of sanity to the world in BaNaNaS, and store the unique_id differently depending on content-type.
This means that for a NewGRF with GRFID "ABCD" it will read '41424344' as foldername.
But also for an AI with a short_name of "ABCD" it will read '41424344' as foldername.
The content_server will byte-swap correctly on receive and send, to make the world simple again.

NewGRFs store the id as char[4] in the file: 'ABCD'. OpenTTD reads this as LE integer, so it becomes 0x44434241.
Whenever OpenTTD prints the grfid on screen, it swaps the bytes before printing, so it looks the same as the char[4]

heightmaps: same as scenario ".id" and ".title"

So, there are two types:

• NewGRF, scripts and basesets work the same. They convert a char[4] into a LE-integer.
• Scenarios and heightmaps work the same. They parse a decimal string into a integer.

For foldernames:

• NewGRF, scripts and basesets should print the integer as hex in LE, so BSWAP before print.
• Scenarios and heightmaps have no precedent usage. It may make sense to use the decimal representation as folder name.

https://github.com/OpenTTD/OpenTTD/blob/master/src/network/core/tcp_content.cpp#L133
OpenTTD does not distinguish heightmaps from scenarios. Their ID range is shared.
Thus Bananas must not assign the same IDs to a heightmap and a scenario.

Updated initial gist, including the finding of a hidden byteswap:
https://github.com/OpenTTD/OpenTTD/blob/master/src/network/core/tcp_content.cpp#L120