Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vehicle::motion_counter is not stored in the savegame #8589

Closed
JGRennison opened this issue Jan 18, 2021 · 0 comments
Closed

Vehicle::motion_counter is not stored in the savegame #8589

JGRennison opened this issue Jan 18, 2021 · 0 comments

Comments

@JGRennison
Copy link
Contributor

@JGRennison JGRennison commented Jan 18, 2021

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant