Topic: Incorporating changes from Standard

[Ranran]

@Matthew-Thanks, it seems to work fine.

[ceeac]

For Visual Studio, there are some extensions like this one which automatically removes trailing whitespace on save.
(The fact that, as far as I know, a full-blown IDE like Visual Studio does not ship with an option to automatically remove trailing whitespace is a major WTF in itself, imo.)

[Ranran]

I think I've identified and resolved a problem that didn't compile. Please check pull request #268.


1) You changed the function type in the header file, but not the cc file.
I'm wondering if this didn't give an error in mingw.

2) New files were added but it couldn't compile with MSVC as you only updated the Makefile and the project file for standard. (r7811)


Almost I just imported Phystam's commits as is, but I think you can merge the previous import work up to r8125.
Thanks to Phystam for his work so far.

I happened to notice that Phystam didn't incorporate the commit that saves the opened dialog even if quit the game by exit (r7642), and I incorporated it, but basically I just picked commits by Phystam and resolve conflicts and fixed errors.  So note that I did almost no such checking.

Now I am trying to incorporate the following patch:
https://forum.simutrans.com/index.php/topic,16536.0.html
This patch moves the system colors to 16bit.
Almost all parts including the changes in Extended are replaced, but in some reason, simutrans cannot load the system colors from simuconf.tab, such as cursor color, window title bar color, etc.
I temporary pushed the current progress of the work to https://github.com/phystam/simutrans-extended/tree/merge-from-standard120-2-2

EDIT:
I'm currently working on resolving r8134 conflicts. I think it's easier to convert only when displaying than to attach color_idx_to_rgb to all.
This is likely to be time consuming as it requires testing with a huge amount of changes.

EDIT2:
It looks like it was still in the works and I get over 1000 errors on compilation. There were many color variables and misspellings that weren't replaced yet. (´・ω・｀)

EDIT3:
r8134 is a major change and must be incorporated separately later. It turns out that this change requires the save to be distinct and the version number to be incremented.

[Phystam]

Thank you, ranran.

EDIT:
I'm currently working on resolving r8134 conflicts. I think it's easier to convert only when displaying than to attach color_idx_to_rgb to all.
This is likely to be time consuming as it requires testing with a huge amount of changes.

Probably your solution is better than the original one. Is it okay for making differences in the code between Standard and Extended? (but no differences when executing, right?)

[prissi]

The color... was then done by search and replace and then the few occasions where it is not possible, were put back.

But anything than a release may have serious errors, although not 1000

[jamespetts]

Thank you very much for your work on this, Ranran: this is extremely helpful. I am a little wary of incorporating such large changes before the issue relating to the losses of synchronisation can be tracked down, since, if this merge introduces any new loss of synchronisaiton error, it would make both that error and the original error orders of magnitude harder to track down; and given the presence of the existing error, there is no reliable way of testing whether this introduces any new error, since we do not know whether any loss of synchronisation is caused by the existing or new error.

I am not sure whether Freddy, Freahk, Ceeac or any others who have worked on loss of synchronisation errors in the past have any views on this in particular; it would be helpful to have input on this question, since I am keen on making use of this work, but do not want to risk making the loss of synchronisation error harder to find.

[freddyhayward]

Though I cannot say for sure, I support these changes in principle and don't think they should make synchronisation errors harder to find.

[jamespetts]

Though I cannot say for sure, I support these changes in principle and don't think they should make synchronisation errors harder to find.

The particular problem is if these changes introduce a new synchronisation error: it is not possible to test for this without first fixing the existing error, as we cannot know whether any given loss of synchronisation is caused by the existing error or a new error.

Similarly, if this were to introduce a new loss of synchronisation error, a step taken to change the code either to try to fix the current error or temporarily to disable a part of the code to narrow it down may not result in a noticeable change if these changes were to introduce a different loss of synchronisation error undetected.

Merging changes from Standard is immensely complex and has been the cause of loss of synchronisation errors in the past. Are you able to see any specific way of ameliorating the specific problems set out above to allow us to integrate these changes earlier than we fix the loss of synchronisation?

[Ranran]

Is the sync error currently occurring only on the bridgewater server?
If so, another server will be able to see the desync if this change is the cause.

[jamespetts]

Is the sync error currently occurring only on the bridgewater server?
If so, another server will be able to see the desync if this change is the cause.

I believe that there has been a report on the Discord channel of the loss of synchronisation happening also on the Stepehenson-Seimens server.

[freddyhayward]

I believe that there has been a report on the Discord channel of the loss of synchronisation happening also on the Stepehenson-Seimens server.

Phystam has also reported a number of synchronisation errors on his server.

[Sirius]

I agree with James thoughts.
If those changes introduce any additional desyncs, it will take much longer to realise this.

Anyway, imho this should not slow down the progress of incorporating changes from standard.
In general, changes from standard are usually rather well-tested, so it is unlinkely those introduce new issues on their own.

I clearly support incorporating these changes.

I believe that there has been a report on the Discord channel of the loss of synchronisation happening also on the Stepehenson-Seimens server.

I do not think so. It was noticed that the desyncs are likely related to "no route" vehicles, so Freddy tested this with an old stephenson-siemens save, where a lot of such convoys existed as a result from introducing the light rail electrification constraint.

Stephenson-siemens server seems to run very stable with barely any desyncs.

[jamespetts]

Stephenson-siemens server seems to run very stable with barely any desyncs.

"Barely any" suggests that there are some; if this server simply has the condition that causes these losses of synchronisation occur much less frequently, which would not be surprising, as this is a much smaller game world, then, given that the losses of synchronisation are in any event quite infrequent, it is not unexpected they would be extremely infrequent on the Stephenson-Seimens server, and therefore conform to the description of "barely any", especially as that server seems to be played less at present. Thus, that there are at least some losses of synchronisation on the Stepenson-Seimens server means that we cannot be sure that the losses of synchronisation are caused by anything that is present on the Bridgewater-Brunel server (and Phystam's server) but not on the Stephenson-Seimens server.

[Ranran]

I reduced r8134 merging compilation errors to 140, but those errors seem to be due to files unrelated to this commit.
The error is similar to the error reported by james in post #95 in this thread.

In Visual Studio, I get the following errors:

This error was due to additional files related to sqapi not being registered in the project file. Therefore I suspect Pyhstam may have missed sqapi-related commits this time as well. (´・ω・｀)

At least r7844 has greatly reduced this error.


EDIT:
Currently, the file related to api_command exists in the master branch, but it seems to be just sanity because it is not described in Makefile or project file. This means that a set of features doesn't work like standard, but if you include it, it won't compile with an error.
The incorporating and maintenance related to this seems to have been omitted. I wonder if it's useless to maintain this feature if it doesn't fix it.

[Phystam]

Sqapi is not maintained so far, so it is already almost broken. James and Acarlotti said that, so my work does not include the changes in sqapi functions from a certain commit.
And I do not use the VS and I cannot understand the configure files of VS. I'm sorry if I missed it.

[jamespetts]

Phystam is correct - because of the very large amount of work to incorporate the scripting API into Extended and because this is not a core feature of the intended open-ended Extended gameplay, this has not been fully integrated and currently does not work. I have not removed it entirely in case somebody might want to make it work in Extended, but that would be a large job and I am not sure how worthwhile that that would be in terms of the resulting gameplay quality. I wonder whether there should be some code comments in Extended to this effect?

[freddyhayward]

Phystam is correct - because of the very large amount of work to incorporate the scripting API into Extended and because this is not a core feature of the intended open-ended Extended gameplay, this has not been fully integrated and currently does not work. I have not removed it entirely in case somebody might want to make it work in Extended, but that would be a large job and I am not sure how worthwhile that that would be in terms of the resulting gameplay quality. I wonder whether there should be some code comments in Extended to this effect?

I don't see the purpose in retaining the code in extended. It makes development more complex for no benefit at all, and if somebody wanted to integrate it in the future, they could simply copy the code from standard. That would likely be far easier than attempting to resurrect the current mess.

[jamespetts]

I don't see the purpose in retaining the code in extended. It makes development more complex for no benefit at all, and if somebody wanted to integrate it in the future, they could simply copy the code from standard. That would likely be far easier than attempting to resurrect the current mess.

Thank you - that is a helpful suggestion. Can I check what others' views of this are, especially those who contribute to coding?

[Phystam]

1. almost everyone wants to compete with real players instead of AI players.
2. no one can/wants to make scenario files currently.
So I agree that we remove Sqapi scripting features from Extended.

[jamespetts]

1. almost everyone wants to compete with real players instead of AI players.
2. no one can/wants to make scenario files currently.
So I agree that we remove Sqapi scripting features from Extended.

That is helpful - thank you. No. 1 certainly reflects my own preferences and project aims for Extended. Real humans are much more complex and fun than any AI that anyone is realistically likely to be able to write for Simutrans-Extended in the foreseeable future.

[Ranran]

I'm trying to merge r8303 - "Unicode support with long path, DSG flavor", but I suspect it lacks the required libraries. Any information on this?

[ceeac]

If you mean the build failures, you can check the build logs:

Code: [Select]

simsound.cc:196:68: error: cannot pass object of non-trivial type 'std::string' (aka 'basic_string<char>') through variadic method; call will abort at runtime [-Wnon-pod-varargs]
          dbg->warning("midi_init()","can't open file '%s' for reading.", full_path);
                                                                          ^
You need to use full_path.c_str().

Code: [Select]

===> LD  ../build/default/makeobj-extended/makeobj-extended
/usr/lib/gcc/x86_64-w64-mingw32/9.2.1/../../../../x86_64-w64-mingw32/bin/ld: ../build/default/utils/log-makeobj-extended.o:log.cc:(.text+0x75b): undefined reference to `dr_fopen(char const*, char const*)'
You need to use fopen() instead dr_fopen() in log.cc. This error was fixed in r8308 (commit 14e70ca0ef). I believe something similar is required for Nettool.

[Ranran]

I'm currently merging standard120.3, but I don't know how to set the correct Makefile for each compiler, so I'm not sure how to fix it. It seems that it is useless even if it is the same as standard except the link. (´・ω・｀)
I would appreciate any help with this. Thank you in advance.


It compiles correctly with my MSVC. However, new files for miniupnpc and freetype are required.

github branch is here
https://github.com/Ranran-the-JuicyPork/simutrans-extended/tree/std-r8434-TT-fonts

EDIT:
Currently only mingw can't build

EDIT2:
Ceeac solved this problem. Thank you very much. 

[jamespetts]

Thank you to both of you for this - this is much appreciated.

As noted, we have an unfortunate problem with merging this at present, which is the need to track down the loss of synchronisation issue before merging in changes that have a non-zero chance of introducing an independent loss of synchronisation problem, and thus making the existing one exponentially harder to find.

The difficulty is that the current loss of synchronisation issue has so far proven intractable, and is thus likely to take an extremely long time (many, many months) of intensive work to resolve: because the time to reproduce the issue is currently many hours, I can only test one small change a day to narrow down the issue, and, given that many hundreds of small changes need to be tested to narrow down the issue effectively, the amount of time that it will take to resolve this may be quite extreme.

If anyone is able to help with the loss of synchronisation issue (including finding a faster reliable reproduction of the loss of synchronisation on a local server/client), that might greatly expedite the ability to merge in the excellent work that is being done here in relation to incorporating the changes from Standard.

Thank you all for your work on this.

[Ranran]

As I reported in another thread, I'm currently working on a standard's commit merge with a GUI focus.
I leave a post so I don't forget, as there will be so many changes in the end. Because I don't remember much about these anymore. (´・ω・｀)

There will be three changes in the GUI coding in the near future.

(1) r8134(120.2.2) - The color definition is extended from 8bit to 16bit.
Therefore, it is necessary to deal with the change of variables and functions. This is what I am working on now and is almost complete.

(2) r8434(120.3) - Support for truetype fonts and large fonts.
Players will be able to select their favorite font in settings. You can easily change the font from the setting dialog.
This will require revising the layout and variables.
Now that the basic import is complete, I'll pick up the related commits.

(3) r8653 - GUI overhaul
This requires a rewrite of the all GUI code. Create a GUI to place components in tables and cells. This requires a lot of work and an extended-specific GUI can require a lot of work. This is why some people advised to wait until this stage for making a new GUI.

The patches are roughly divided into the above three parts.

The ones before r8134 are repairs of what phystam was previously doing.
Here is the one that incorporates the commits related to GUI and drawing from (1) to (2) and (3) regardless of (2).
The standard version is 120.2.2. It's ready to merge.
https://github.com/Ranran-the-JuicyPork/simutrans-extended/tree/std-r8134-fix

This hasn't changed much in appearance, and I don't think there are any major problems at the moment.

Code: [Select]

Release of 120.1.2: (r7720 on 7-Jan-2016):
ADD: configurable compass for minimap, rotation tool, and main screen

Release of 120.2: (r8077 on 12-Feb-2016):

Release of 120.2.2: (r8163 on 31-3-2017):
FIX: Combo boxes now work much more reliable
ADD: All configurable colors now rgb internally
The standard version corresponds to 120.2.2.
With a small change, the UI layout is reloaded when restarting even with auto save. (For some reason it is not listed there.)
Phystam may be more familiar with the changes in this patch.

[Ranran]

After (1)r8134, as already explained, In order to reach the GUI overhaul, I am mainly focusing and merging on commits related to GUI and drawing.
The commits related to system and sqapi are omitted. However, at first glance it seems that something important is often already captured by A Carlotti.
I would be grateful if Phystam or someone interested in doing this would do the leftover commits.
However, in the end, many commits were incorporated because incorporating is unavoidable if it affects them. (´・ω・｀)

It should be noted that sqapi related files are corrupted from the beginning with extended, and incorporating changes related to them may make compilation impossible. That was the reason why phystam got stuck last time, and it happened once after that. This can be related to multiple major changes.

(2) r8434(120.3) - Support for truetype fonts and large fonts.
Players will be able to select their favorite font in settings. You can easily change the font from the setting dialog.
This will require revising the layout and variables.
Now that the basic import is complete, I'll pick up the related commits.

github branch is here:
https://github.com/Ranran-the-JuicyPork/simutrans-extended/tree/std-r8434-TT-fonts
This includes commits just before the GUI overhaul (r8653). It contains a great many changes. I think we need to check carefully.
Many network-related changes have been incorporated.
You will need freetype.dll and miniupnpc.dll to build it.

Code: [Select]

Release of 120.3 (r8503 on 15-June-2018):
ADD: new blue-white theme
ADD: Support Truetype fonts on windows up to 19 pts. For MAC and Linux support there is a way needed to find the font path.
ADD: Server can be now started at the UI when loading a game.
ADD: switch -easyserver on commandline to host a game with dynamic IP behind a router
FIX: added Unicode file path support for Windows builds

Release of 120.4 (r8588 on 17-Sep-2018):
FIX: properly handle dualstack servers by using both IPv4 and IP6 addresses (if no symbolic name is available)

Regarding themes, many themes other than the above will be added. Also, the existing folder contains a broken theme that needs to be removed. It can cause a crash.



What I haven't taken:

Code: [Select]

CHG: pedestrian movement, new dat parameter:
offset (of pedestrian image relative to right tile boundary, 0..255, default 20, 0 = on the right edge of tile, 128 = on center line of tile
(r8254, 8255) This may be related to the strange pedestrian movement we reported earlier. But there are already many differences between extended and standard. There will also be changes to makeobj.

Code: [Select]

ADD: (HyperSim) sorting vehicles in depot
ADD: sorting in depot (HyperSim)
The structural difference is too big for me to merge. We have to write the code almost on your own...

Code: [Select]

ADD: (THLeaderH) option to hide the revenue income messaging at stops
This will be done after the incorporation of r8843 from the viewpoint of saving time and effort.  EDIT: Merged into r8653 branch.

Code: [Select]

ADD: More factory locations (shore, river, forest) to the existing city, water, land
This adds new location requirements to the industry. The industry is complicated and involves various factors, which can be difficult and was not incorporated.

Code: [Select]

OPT: significantly improved the performance of changing underground mode/slice when playing on very large maps
(r8561) I don't know much about these codes because of the large amount of changes.
The underground display on the bridge water server took a long time, but will that be improved?
According to Freddy this was already merged into the master branch.



The font size is selectable, but some dialogs don't handle this correctly.
For example, the line spacing may be too narrow, or the symbol position may be displayed upwards.
standard has stayed in this version for a long time, but we don't have to stay in this version for a long time. So I think it's useless to spend a lot of time fixing it.
Therefore, this patch is incomplete.

And there is one more broken part. The buttons on the save / load screen do not work. Therefore, you have to enter the text directly. (´・ω・｀)
I don't know how to fix this, but the reason it was caused is that the structure of this screen is very different between standard and extended.
The code has been rewritten by Bernd Gabriel and this dialog is tabulated.
In pakselector, as I pointed out earlier, the broken load addons button was repaired with incorporating from standard, but on the other hand, in the loadsave dialog, the file button is now broken.
Many GUIs will be tabulated by the GUI overhaul that comes after this. So I thought Bernd Gabriel's table code would be useless anymore, so I didn't fix it at this stage.

I would appreciate it if you could check this patch and the next patch and let us know what you think.


merged commit lists up to r8500. It does not include what is already merged.

Code: [Select]

7611
7844 7865 7870 7908 7911 7932 7937 7939
8075 8123 8125 8134 8138 8151 8154 8159 8169 8176 8181 8187 8193 8197
8200 8203 8204 8205 8207 8210 8213 8217 8221 8223 8231 8271 8279 8287
8305～09 8312 8314 8315 8316 8317 8319～21 8324 8325 8335 8338 8343 8345 8365 8373 8379 8382 8384 8386 8396 8399
8400 8401 8402 8403 8405 8423 8428 8431 8433 8434-44,8447-50 8451 8453～55 8457 8459～61 8465 8466 8474 8476 8477 8478 8484 8488 8489 8490 8495 8496


[freddyhayward]

The underground display on the bridge water server took a long time, but will that be improved?

I had already merged that patch. It improved performance quite a lot, but introduced a new visual bug. Somehow, your branch seems to have improved the performance as well, but I'm not sure.
I have tested your branch and there are many visual bugs: stop labels, UI text (until theme is changed), and cursor are all discolored. Windows are too large and UI elements have too much spacing. I hope these and other issues can be resolved as this branch will be very valuable in the long-term.

[Ranran]

Thank you for testing it.

cursor are all discolored.

Can I ask what the cursor is pointing to?

EDIT:
The tile cursor is specified by cursor_overlay_color for each theme. The default (undefined) is red.
I don't know where the source for the extended standard theme is, but it's probably due to the theme settings.
(EDIT2) That dat file seems to be missing.

stop labels, UI text (until theme is changed). Windows are too large and UI elements have too much spacing.

Can you explain these? Extended's standard theme settings may be corrupted (slate blue one). I don't know where the source is. I don't think extended has supported the theme properly so far.
(EDIT2) The processing at the first startup may not be correct.

It also certainly looks like there is no color correspondence for Bernd Gabriel's table code.

[freddyhayward]

here are some screenshots:

[jamespetts]

Thank you for your work on this - I have been working on test merging this. I have been able to merge some of the earlier changes successfully, and these have been pushed to my merge-from-standard-september-2020 branch for testing. Initial testing suggests that this is stable.

However, I have not been able to compile any later version than this. I have tried merging the latest version into the merge-from-standard-september-2020 branch, but the result is uncompilable. In particular, I seem to be missing two header files, miniupnpc.h and upnpcommands.h.

[Roboron]

In particular, I seem to be missing two header files, miniupnpc.h and upnpcommands.h.

You need libminiupnpc. If compiling with MINGW, you will need to compile libminiupnpc yourself (it seems to be broken). Otherwise install libminiupnpc-dev/miniupnpc/miniupnpc-devel if you are on Debian/Arch/Fedora&OpenSUSE.

[jamespetts]

You need libminiupnpc. If compiling with MINGW, you will need to compile libminiupnpc yourself (it seems to be broken). Otherwise install libminiupnpc-dev/miniupnpc/miniupnpc-devel if you are on Debian/Arch/Fedora&OpenSUSE.

On my development system at home, I use Visual Studio; I notice that the continuous integration builds on Github also fail with this branch, so something will need to change somewhere to allow these to work.

[Ranran]

I seem to be missing two header files, miniupnpc.h and upnpcommands.h.

I'm not sure if it needs all or some of the attached files.

And after r8434 branch, work is still in progress.
There are 8134 branches that appear to be stable, 8434 and 8653 that have incorporated many changes and need to be checked, and 8653 is in the works. The 8434 has some problems as explained above.

[jamespetts]

Thank you for that. Can I check what the licence is for miniupnpc so that I know whether these files can be incorporated into the Github repository?

Edit: It appears that, even with that .zip file, the file miniupnpc_declspec.h is still missing.

[Ranran]

Code: [Select]

MiniUPnP Project
Copyright (c) 2005-2017, Thomas BERNARD
All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are met:

    * Redistributions of source code must retain the above copyright notice,
      this list of conditions and the following disclaimer.
    * Redistributions in binary form must reproduce the above copyright notice,
      this list of conditions and the following disclaimer in the documentation
      and/or other materials provided with the distribution.
    * The name of the author may not be used to endorse or promote products
  derived from this software without specific prior written permission.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
POSSIBILITY OF SUCH DAMAGE.You can download it from here.
http://miniupnp.free.fr/files/


EDIT:
The problem Freddy pointed out seems to be that the color of the cursor is rewritten when the setting dialog is opened.
EDIT2:
It was because Phystam was ignoring some lines in the first merge operation.
I have not yet investigated the line spacing size of the display setting dialog and scrolled_list.
EDIT3:
The size of the display setting dialog is a standard's new specification, not a bug.
Opens a window with the default size. This dialog will change significantly in the near future after all.
Under the new specifications, the font size will change, so fixed dialog sizes will not match.
Also, the r8653 patch has completely redesigned the display setting dialog.


The missing file may be in this
https://simutrans-germany.com/files/upload/miniupnpc.zip