Version of OpenTTD

Current master

Context

In the course of diagnosing a multiplayer desync in my branch reported by a player and GRF author, I noticed that Vehicle::motion_counter is not synchronised between clients.
As the specific desync which was reported would not be detected or cause any issue in trunk, the details of that desync are omitted from this issue.

Expected result

Vehicle::motion_counter is stored in the savegame.

This is made available to NewGRFs via vehicle variable 0x46.
Use of this NewGRF variable in a sensible context (i.e. excluding all the various callbacks which are re-executed on load) should not be able to cause a desync.

Actual result

Vehicle::motion_counter is not stored in the savegame, it is initialised to 0 on load.
The field in struct Vehicle is not labelled with NOSAVE, like other fields which are not saved.
The specification does not indicate that this variable may take different values on different network clients.

This is made available to NewGRFs via vehicle variable 0x46, with no restrictions on use.
Any use of this variable in any context which affects the game state could cause a desync.

Steps to reproduce

Note the absence of motion_counter from src/saveload/vehicle_sl.cpp.
Note that Vehicle::motion_counter is exposed to NewGRFs by means of vehicle variable 0x46.

Suggested solutions

This could involve one or more of:

1. Vehicle::motion_counter is saved/loaded like other vehicle fields.
2. Vehicle::motion_counter is tagged NOSAVE in struct Vehicle.
3. The NewGRF (and NML) specifications should indicate that vehicle variable 0x46/motion_counter is multiplayer unsafe in all contexts except vehicle animations.
4. NewGRF variable 0x46 should return 0 in all contexts except retrieving a vehicle image to prevent uses which can cause desyncs.