planetmaker wrote:

Personally I haven't touched network code yet, for a reason, so take my words with a grain of salt:
Just changing that means touching network compatibility; likely you'll have to introduce a new network protocol version:
You'll have to look at the master server that it treats it correctly, depending on the client version it communicates with. You'll also have to check that such change doesn't increase the size of any network packet beyond its allowed limits.

This comment was imported from FlySpray: https://bugs.openttd.org/task/5326#comment11578

Eagle_rainbow wrote:

Granted to all what you said. However, I see also another option to not affect the network protocol: Apparently, the problem only occurs with the gamelog. The gamelog just "reuses" the constant for the length of the revision attribute (see gamelog_internal.h, line 45 as of r24576). Why not just extending the field length manually there. The constant for the length is called "NETWORK_REVISION_LENGTH". Thus, why not having a "GAMELOG_REVISION_LENGTH"... This could be a quick fix to it, but on the other hand introduces a little inconsistency.
BTW: That's also the reason why I don't understand that you have put the problem to category "Network"... but, as I am a newbie - you'll know what you are doing :)

This comment was imported from FlySpray: https://bugs.openttd.org/task/5326#comment11581

Alberth wrote:

Afaik, the revision (which includes the branch name), gets transmitted over the network for game version identification. There is also other data in that packet, and the text got the space that was left, which is unfortunately not much.
Making it longer is not possible, since the network packet may not arrive, or not arrive in one piece, in that case.

The issue is however not serious, the message is just a notice that not all text is copied. Since this will happen at every machine the game identification still works. Also, since it is a debug notice for versions that are built from a branch, only developers ever see the message.

Fixing the issue looks like overkill to me tbh, extending the network protocol just for experimental builds to prevent an information message is just tmwftlb. There are bigger problems in OpenTTD.

This comment was imported from FlySpray: https://bugs.openttd.org/task/5326#comment11582

frosch wrote:

Extending the name in the gamelog sounds reasonable. Telling the exact version from a savegame can be very useful.

This comment was imported from FlySpray: https://bugs.openttd.org/task/5326#comment11585

frosch wrote:

Attached diff removes the limit for the revision text in the gamelog.

Attachments

• gamelog_untruncated_revision.diff (4.65 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/5326#comment11588

Eagle_rainbow wrote:

Just checked the diff by frosch. It would fix my problem.

This comment was imported from FlySpray: https://bugs.openttd.org/task/5326#comment11590

dihedral wrote:

could you also please try this with network server + connecting a client using the multiplayer lobby?

This comment was imported from FlySpray: https://bugs.openttd.org/task/5326#comment11592

frosch wrote:

Who does your question address?

The patch does not change the version which is sent across the network. The packet has to truncate the string. However, the receiving side also does only compare the truncated versions.

So:
* OpenTTD titlebar and intro GUI show the revision with a higher limit on the length.
* With the patch the gamelog also stores the revision with a much higher limit.
* Revisions sent over the network remain truncated.
** The client and openttd.org only display the truncated version.
** The compatibility check only compares the truncated versions.

This comment was imported from FlySpray: https://bugs.openttd.org/task/5326#comment11594

Eagle_rainbow wrote:

Though I agree to frosch's statement, I still did a brief test with the patch in a multiplayer environment:
* git version on trunk with patch from above applied; one server via LAN + one additional local client --> works
* official release 1.2.2 source code, patch applied (has problems due to savegame version, but the rest was applied successfully), connect to public server on the list with compatible version --> works

Thus, I'd say, that this patch does not introduce an obvious side effect to the multiplayer code.

This comment was imported from FlySpray: https://bugs.openttd.org/task/5326#comment11603

This is still an issue, and I imagine it can cause undetected compatibility issues with network games.

After discussing this on IRC, I think we've agreed that the network server_revision field being limited to 15 bytes (of which one is consumed as string terminator) is too short for this brave new git world.

I think there is also enough room in the UDP packet to expand the server_revision field a bit, perhaps to 25 or 30 bytes, which should protect against most matched-mismatch issues, without lowering the max number of NewGRFs allowed. Checking the code, it looks like expanding the field is even possible without bumping the game_info_version number, as older versions will just truncate the string on receiving a too long one.

Closed in favour of #7021