Incorporating changes from Standard

[Andarix]

Changing windows-latest to windows-2019 in the CI fix it. For some reason it is not able to find MSVC in windows-2022.

Windows Server 2022 (I think VS 2022 because it was also announced) has a newer version of MSVC than Windows Server 2019 (VS 2015). And so the paths should differ.

[jamespetts]

Pull request #503 incorporates the "Heavy Mode" desync debugging mode and some fixes regarding desyncs. Hopefully there should be fewer of them now, but I could not test absolutely everything. Please check before merging the changes because the savegame version is increased.

EDIT: Windows CI is failing because

Thank you for your work on this (and to the others, too).

Before I incorporate this, can I check what Heavy Mode is in this context?

[ceeac]

This makes debugging easier and optionally produces two save files at the exact sync_step the desync occurred, at the cost of severely reduced performance.
To enable Heavy Mode, both client and server must be started with the -heavy 1 command line flag.
For details, see the original thread here:
https://forum.simutrans.com/index.php/topic,21211

[jamespetts]

I have tried to merge this, but I get a "missing endif" error on compiling referring to line 777 of the makefile. Does this assume that I have merged prior pull requests from Ranran in this project?

[ceeac]

No, the error was unrelated - Should be fixed now.

[jamespetts]

I am afraid that I am still getting the same error:

Code Select Expand

$ make -j3
Makefile:777: *** missing 'endif'. Stop.


[jamespetts]

Pull request #485 is for standard's commits r9872 to r10076 related to translator and symsys coordination. and unification of formatting with standard,  also translation from amber to yellow.

I am afraid that I get a compile error when trying to build this:

Code Select Expand


gui/trafficlight_info.cc: In constructor 'trafficlight_info_t::trafficlight_info_t(roadsign_t*)':
gui/trafficlight_info.cc:42:27: error: 'class roadsign_t' has no member named 'get_ticks_yellow_ns'; did you mean 'get_ticks_amber_ns'?
   42 |    amber_ns.set_value( s->get_ticks_yellow_ns() );
      |                           ^~~~~~~~~~~~~~~~~~~
      |                           get_ticks_amber_ns
gui/trafficlight_info.cc:48:27: error: 'class roadsign_t' has no member named 'get_ticks_yellow_ow'; did you mean 'get_ticks_amber_ow'?
   48 |    amber_ow.set_value( s->get_ticks_yellow_ow() );
      |                           ^~~~~~~~~~~~~~~~~~~
      |                           get_ticks_amber_ow
gui/trafficlight_info.cc: In member function 'void trafficlight_info_t::update_data()':
gui/trafficlight_info.cc:110:32: error: 'class roadsign_t' has no member named 'get_ticks_yellow_ns'; did you mean 'get_ticks_amber_ns'?
  110 |  amber_ns.set_value( roadsign->get_ticks_yellow_ns() );
      |                                ^~~~~~~~~~~~~~~~~~~
      |                                get_ticks_amber_ns
gui/trafficlight_info.cc:111:32: error: 'class roadsign_t' has no member named 'get_ticks_yellow_ow'; did you mean 'get_ticks_amber_ow'?
  111 |  amber_ow.set_value( roadsign->get_ticks_yellow_ow() );
      |                                ^~~~~~~~~~~~~~~~~~~
      |                                get_ticks_amber_ow
make: *** [common.mk:55: simutrans/gui/trafficlight_info.o] Error 1
make: *** Waiting for unfinished jobs....

[jamespetts]

Oh, I'm so sorry.In standard, this was implemented as yellow, but in extended, I think there may have been a history of it being intentionally made into amber, which is why I revert it a few days ago. The cause is that the revert is clearly incomplete. I apologize for the lack of confirmation.
Can I ask what the color notation should be?

For traffic lights, the colour should be "amber". For railway signals, the colour should be "yellow". In the UK at least, the actual colours of traffic lights on roads are distinctly different to railway signals: roads have always had red/amber/green, whereas railways have red/yellow/green. (For reference, amber is a yellowish orange).

[jamespetts]

Thank you for the clarification. I think I have restored it and now it is testable.

Excellent, thank you: now incorporated.
Edit: Unfortunately, I now get a merge conflict with Ceeac's heavy mode branch.

[ceeac]

I am afraid that I am still getting the same error:

Could not reproduce. I have now rebased the branch onto the current master branch to resolve the merge conflict and the issue does not occur there for me either (commit 6c60afa91).

[jamespetts]

Could not reproduce. I have now rebased the branch onto the current master branch to resolve the merge conflict and the issue does not occur there for me either (commit 6c60afa91).

Thank you. I have now rebased, but I am afraid that we are now getting a save format incompatibility issue with the latest master: games saved in the latest master can't be opened in the heavy mode branch, and vice versa.

[ceeac]

games saved in the latest master can't be opened in the heavy mode branch

That was a bug and should be fixed now.

and vice versa.

That is not a bug because the save game version is incremented in the heavy mode branch.

[jamespetts]

That was a bug and should be fixed now.
That is not a bug because the save game version is incremented in the heavy mode branch.

Excellent - now incorporated. Thank you.

[jamespetts]

There seems to be a problem in compiling with MSVC, because the current changes for heavy mode have not been applied to MSVC. I was able to add the file specification and compile on my own (I just added a link to the file for debug mode), but I don't know if that's correct because I don't understand the conditions correctly.

Did you have to add a .cc file to the project? If you did and that solved the problem, then this should be a relatively straightforward fix. Can I ask what file it was that you added?

[jamespetts]

I had to make at least the following changes to get the build to work on MSVC.
https://github.com/Ranran-the-JuicyPork/simutrans-extended/commit/36c9cd51899d71719fa440e14df0b2b521f722ee

Thank you - that is very helpful.

[jamespetts]

We are getting a consistent loss of synchronisation error occurring on all online server games, as reported here which cannot be reproduced on a local client/server pair. As the "heavy mode" patch is the only thing that affects either the simulation itself or client/server synchronisation, I strongly suspect that this is the cause, but have not looked into it in sufficient detail to know how this causation might occur. Ceeac - is there anything that you can think of that might be responsible for this?

[ceeac]

Probably different settings (simuconf.tab/settings.xml). Try starting two instances in singleuser mode, I managed to reproduce the desync this way. I have not investigated the root cause of the desyncs yet, though.

[jamespetts]

Probably different settings (simuconf.tab/settings.xml). Try starting two instances in singleuser mode, I managed to reproduce the desync this way. I have not investigated the root cause of the desyncs yet, though.

Hmm - different simuconf.tab settings ought not to cause losses of synchronisation, since the settings that make a difference to the synchronised game state ought to be transmitted with the saved game. I will have to test running two different computers on the same local network to see whether this produces loss of synchronisation.

[jamespetts]

As reported here, I have found the immediate cause of the problem: different versions of (the base) en.tab between server and client cause a loss of synchronisation.

This is very similar to a problem which I identified and solved in 2017: the town name fragments are randomised. At the time, they used simrand, not sim_async_rand, with the result that any divergence between the list of town name fragments on client and server would cause the random number state to diverge between them. I think that I fixed this by switching the town name randomisation to use sim_async_rand.

I am not sure whether that is the exact problem that has now recurred or whether something different but similar is behind this, but it is instructive to know what caused the previous very similar problem in order to understand how best to go about narrowing down the problem.

Ceeac and Ranran - have either of you altered anything in the commits that went in yesterday that related in any way to an interaction between translation texts and the random number generator?

[jamespetts]

https://github.com/jamespetts/simutrans-extended/commit/b9175ecc95e26bdad924eb3e74d1d696f19bb7a2
There are some changes related to translation texts. I'm not sure if that will affect it. Indirect effects, if any.

There is nothing obvious in that affecting RNG states - it is mainly multi-language support and translations that appear to be affected; code with which I am not particularly familiar.

[jamespetts]

https://github.com/jamespetts/simutrans-extended/blob/36c9cd51899d71719fa440e14df0b2b521f722ee/dataobj/objlist.cc#L1043

Thank you for that. I have now fixed this. However, I doubt that this is the cause of the problem, since, if the problem were faulty load/save code, desynchronisation would happen whether or not en.tab were the same.

[prissi]

I would strongly suggest to also update simevent.cc, siminteraction.cc and simsys_s2.cc to the latest revision, since the actual scrolling and event processing still had some errors.

[prissi]

There were still some errors, especially for tablets and draggign dialogues. Maybe nothing so obvious for mouse. (apart from infinite scrolling back for mouse).

[jamespetts]

Excellent, thank you for this. Apologies for the delay in considering this. I have now tested this, and it does appear to fix the scrolling issue, which makes the game much better to interact with. Now merged.

[jamespetts]

Having deployed this in the master branch, a problem has revealed itself: the server announce feature no longer appears to be working. I am able to connect using the net:bridgewater-brunel.me.uk in the load dialogue, and remain connected, but nothing reaches the announce server.

I have tested merging the 2202-some-fix branch and manually redeployed this on the server, but this does not appear to have assisted matters.

[jamespetts]

Thank you for this. I have tested this, but found that this reintroduces the scroll problem, so have not incorporated this for the present.

[jamespetts]

Apologies for the delay in response - the scroll problem was the problem described in another thread (I cannot immediately recall where) in which, in at least some cases (I could reproduce this on my Linux computer), scrolling a large game with slower performance resulted in very large sudden jumps in position, making it extremely difficult to move around the map or control one's position. This had been a problem for some time, but was fixed about a month ago, with one of your patches, I think. Pull request no. 530 reintroduced this problem.

Can I check whether pull request 530 is cumulative with 525, or whether the intention is that 525 be merged first?

In any event, thank you very much for the work that you have done to date on merging; it is a very large and difficult piece of work, and your dedication to this has greatly improved Simutrans-Extended.

[jamespetts]

Having tested again, the latest version of this appears not to have the scroll problem any longer, so I have incorporated this. I should note, however, that I cannot find any remove signal tool - is this because the tool icon/definition has not yet been added to the configuration files?

[jamespetts]

Pull request #570 incorporates minor changes from r10659 to 10719.

- Minor improvements to font setting UI by roboron
- Destruction tools ignore pedestrians. This could save Freddy from suffering. 
- Fixed some UI bugs
- Fixed bug in tunnel graphics when changing terrain
- Fixed an issue where factories would sometimes not register as power plants as intended

Thank you very much for this. Can I check whether this has been tested with the ex-15 branch for merge conflicts? I need to ensure that it merges properly with that branch before I can safely incorporate it into the master branch.

[jamespetts]

The ex15 branch has had issues a few months ago, so I'd prefer to resolve those first before merging.
I can't test it because I've had problems with boot in the past.

Thank you for letting me know. I believe that I have now fixed this - I should be grateful if you could test the compatibility of integration with the ex-15 branch before I incorporate this into the master branch. Thank you.

[jamespetts]

I've confirmed that pull request #570 and #580 work fine with the ex15 branch.
PR#582 is a minor bug fix unrelated to ex15, so I think it's fine.

Excellent, thank you for that. I have now incorporated these. No. 580 did need some adjustment to the load/save versioning, however.

[jamespetts]

PR#592 is a slight integration from standard. I'm taking a break from simutrans for a while, so I'll submit the commits I've completed so far.

Thank you very much for this, and apologies for the delay in responding. Can I ask whether this has been tested for Ex-15 branch compatibility?

[jamespetts]

This change has already been merged into the new branch for ex15 and has been tested.

Excellent, thank you: now incorporated.

[neroden]

Does someone have a summary of the current state of incorporation of changes from standard?
Is the documentation/cherry-picked-commits.txt file relevant to this question?

[ceeac]

The cherry-picked-commits.txt file lists most (all?) commits that Ranran cherry-picked from Standard, however he is taking a break from Simutrans at the moment. I have also submitted a few pull requests incorporating changes from Standard, mostly around improving code quality (automated tests) and tools for desync debugging ("heavy mode"). Many of the automated tests are still disabled though because the functionality is different between Standard and Extended.

I am currently working on a larger patch which incorporates the terraformer refactoring made in r10443 (this is mostly finished, should be ready by this or next weekend), and a port of the source reorganization script added in r10444. This should make incorporating further changes much easier again.