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.

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