Topic: Schedule features: technical discussion

[jamespetts]

The stack priorites for coupling vehicles would be just the same, it is the "negative" stack priorities, ie what vehicles to uncouple that is what I am talking about.
Remember, we are only talking about uncoupling vehicles, not coupling vehicles, because I believe that is already taken care of with your approach. The only thing I want is that it is the correct car to be detached.

I am not sure that I fully understand this distinction here, as the consist order specifies only the makeup of the consist after recombination, so whether any given vehicle is coupled or uncoupled is indeterministic at the point when the consist orders are written.

I do not really understand what you mean by "negative" stack priorities: the stack is just an ordered list of vehicle type descriptors in slots. There is only one stack priority. The idea is that the algorithm simply checks (1) the existing consist; and (2) the vehicles available either loose or in joining consists to make up something that matches the description of the consist in the consist order, in order of priority. If vehicles are uncoupled it is because the algorithm has determined that they do not form part of the end state consist as specified in the consist order (and, in so far as there are stacks and lower priority vehicles are included, in so far as the relevant vehicles are available).

[Ves]

Yes, the consist order specifies the makeup of the convoy leaving the station, however, it needs to be calculated which vehicle to be detached somehow, and that is what I called the negative stack priority. It would simply be the difference between the makeup of the consist order before arrival and after departure. Those vehicles that where in the old consist order but not in the new, are the ones that is least suited in the stack of priorities.
What I am suggesting is that when it determines which those least suited vehicles are from the stack of priorities for the new consist orders, it should also look for the good inside the vehicles. If the good has this particular stop as its "intermediate _shunting" stop, it would be the least suited vehicle per definition, even though there might be another vehicle that would otherwise be the least suited.

That would mean that if the consist order of the convoy leaving the station is made up from as fast vehicles as possible, but the train, when approaching the station has a slow vehicle in it, it would drop of a fast vehicle, if that has the good with the "intermediate_shunting" set for this station.

[jamespetts]

My apologies if I am being a bit dim, but I still do not understand fully what you seek to achieve. Are there any circumstances in your preferred algorithm in which, in my example, a slow vehicle would continue down the main line if a fast vehicle were available? If not, how is this functionally different to simply silently and instantly re-arranging the goods inside the train at the junction station?

[Ves]

The end goal is that good should never have to rearrange.

Let me try to do an example:

Lines:
"Branch line X - B"
"Main line A - B - C - D"
"Branch line C - Y"

Consist orders:

"Branch line X - B"
X -> B - [Power unit] - [piece good] - [piece good]
Target at B:  "Main line A - B - C - D"

"Main line A - B - C - D"
A -> B - [Power unit] - [piece good, fastest possible] - [piece good, fastest possible]
Target at B:  none
B -> C - [Power unit] - [piece good, fastest possible] - [piece good, fastest possible] - [piece good, fastest possible] - [piece good, fastest possible]
Target at C:  "Branch line C - Y"
C -> D - [Power unit] - [piece good, fastest possible] - [piece good, fastest possible]

"Branch line C - Y"
C -> Y - [Power unit] - [piece good] - [piece good]

Vehicles:
Beside the power units, which we dont bother with in this example, we own:
[Piece good car, 40km/h]
[Piece good car, 40km/h]
[Piece good car, 80km/h]
[Piece good car, 80km/h]


So, the scenario:

The power unit on line "Branch line X - B" is at station "X", and according to its consist order, it is supposed to fit two piece good cars, with no further constraints or priorities. At station "X", these two cars are located: [Piece good car, 80km/h] and [Piece good car, 80km/h], which are the two fastest cars that we own.
The power unit departs from "X" with those two cars, which is full of piece good to "Y".

At station "A", the convoy is assembled for line "Main line A - B - C - D". It will consist of the power unit and [Piece good car, 40km/h] and [Piece good car, 40km/h], since there are no faster vehicles available at "A". Those two cars are running empty.
When the convoy arrives at station "B", it fullfills its consist order by fetching the missing vehicles from the branch line convoy, and it is now ready to travel to station "C" with all four cars.

At station "C", is where it gets difficult:
The consist order for line "Main line A - B - C - D" departure from "C" tells it to have only two cars attached, for which its priorities is to have the fastest cars possible.
However, the two fastest cars available are currently loaded with good to "Y", so they are disquallified, and hence given the lowest priority in any of the slots for the departure from "C" to "D".
That would result in the two slow cars becoming the "fastest" cars available, and therefore they are choosen to be kept in the convoy for the departure from "C" to "D".

Does this makes sense?


What would happen if the consist order from "C" to "D" looked like this (added one more car):
C -> D - [Power unit] - [piece good, fastest possible] - [piece good, fastest possible] - [piece good, fastest possible]
Answer: Since the cars loaded with good to "Y" only was given the lowest priority, one of those cars would get detached, and the other would remain in the convoy. That would be a player mistake.

[jamespetts]

Do I understand, then, that the answer to the first question in my previous post is "yes"? Do I understand correctly that the algorithm that you suggest is simply that any vehicle with goods loaded for a destination not served by the current schedule of the train connected to the consist order would, irrespective of the stack priorities in the consist order, be used in that consist only if nothing else is available?

I can foresee some real difficulties with this, at least many of which fit into the chaotic consequences category. Firstly, the schedule of the convoy on which the consist order is currently operating may not contain the final destination of the vehicle in question, as it may at a later stop be split from the current consist and re-combined with a different consist to reach its destination (and this may happen an unlimited number of times). In this case, I can foresee failures by the algorithm to attach the expected vehicles in the way that is intended by the player. Secondly, there is no "fastest available" datum in the consist orders. Instead, there is a datum for minimum speed. To get something like "fastest available", one would have to stack several different sets of rules, each with progressively lower minimum speeds. Thirdly, if you had imagined that this would work, not just on stack priorities, but on the actual rules themselves (as the reference to "fastest available" suggests), then it is not clear how this might work without creating the possibility of forming an unworkable train (e.g., one that is too heavy for its route) that is contrary to the player's intention and explicit instruction.

[ACarlotti]

It seems to me that in the above scenario it should be possible for the player to choose the outcome. Depending on the exact context, it might be more important to avoid transferring the goods from one vehicle to another at the station, or it might be more important to ensure a higher mainline running speed.

I think a lot of potential issues with malformed convoys can be mitigated by allowing a convoy to leave some vehicles to be added to a later instance of the schedule (and possibly the entire train). So for the case of a too heavy train (too many vehicles are being routed onwards onto one portion of the schedule), the player would specify a weight limit for the consist and if the vehicles being allocated to that portion of the schedule amount to a greater total weight, then the excess vehicles would be allocated to a new convoy running to the same schedule, together perhaps with a portion of the next convoy to arrive on the route.

[jamespetts]

Can I ask what you would imagine the data structures/algorithm for this choice being and what exactly people would be choosing?

[Ves]

Do I understand, then, that the answer to the first question in my previous post is "yes"? Do I understand correctly that the algorithm that you suggest is simply that any vehicle with goods loaded for a destination not served by the current schedule of the train connected to the consist order would, irrespective of the stack priorities in the consist order, be used in that consist only if nothing else is available?

Yes, it means yes to both questions.

I can foresee some real difficulties with this, at least many of which fit into the chaotic consequences category. Firstly, the schedule of the convoy on which the consist order is currently operating may not contain the final destination of the vehicle in question, as it may at a later stop be split from the current consist and re-combined with a different consist to reach its destination (and this may happen an unlimited number of times). In this case, I can foresee failures by the algorithm to attach the expected vehicles in the way that is intended by the player.

I dont think I follow. The good must know at what station it expects to change convoy, respectively to unload and load into another convoy? If you mean that the player has made a mistake somewhere and not included a car that the good can travel in, then that is a player mistake.

Secondly, there is no "fastest available" datum in the consist orders. Instead, there is a datum for minimum speed. To get something like "fastest available", one would have to stack several different sets of rules, each with progressively lower minimum speeds.

No, I made it simple for the demonstration purposes. I believe there is "prefer high speed" though.

Thirdly, if you had imagined that this would work, not just on stack priorities, but on the actual rules themselves (as the reference to "fastest available" suggests), then it is not clear how this might work without creating the possibility of forming an unworkable train (e.g., one that is too heavy for its route) that is contrary to the player's intention and explicit instruction.

Planning. The player needs to anticipate what vehicles are to travel the network. And frankly, if the player knows that a bunch of heavy cars are loaded with a particular good, the player will automatically anticipate that it is those particular heavy cars that will end up at the destination too, no matter what is written into the consist orders.

That is even very intriguing gameplay I think: If you know you are sending good to a factory high up in the hills, make sure the cars you send to the initial factory has good braking abilities. If you know that you are sending good that is meant to travel long distance on the shared main line, make sure the cars are fast.

GUI-wise it is very easy to display to the player that any loaded good has the highest (or at least to a certain degree) priority.

It seems to me that in the above scenario it should be possible for the player to choose the outcome. Depending on the exact context, it might be more important to avoid transferring the goods from one vehicle to another at the station, or it might be more important to ensure a higher mainline running speed.

I think a lot of potential issues with malformed convoys can be mitigated by allowing a convoy to leave some vehicles to be added to a later instance of the schedule (and possibly the entire train). So for the case of a too heavy train (too many vehicles are being routed onwards onto one portion of the schedule), the player would specify a weight limit for the consist and if the vehicles being allocated to that portion of the schedule amount to a greater total weight, then the excess vehicles would be allocated to a new convoy running to the same schedule, together perhaps with a portion of the next convoy to arrive on the route.

The only issue I can find is if in that example the slots for the rest of the main line had also a "minimum 80kmh" tag, discarding the two slow cars that where remaining in the convoy. If no other mechanism in place, that would halt the convoy there, only to leave when the branch line returns with the two, now emtpy, cars.
This is what I would call a player error. The player has made the rules too narrow, and should either not have a "minimum 80kmh" tag for the last bit, OR should have made sure that it is in fact the two slow cars that get loaded in the first place, for instance by placing "minimum 80kmh" on the line B->A in the reverse direction, assuming they are all empty.

I believe, however, that it is meant to be able to have empty slots as well, which I assume would make it possible to have make convoys more flexible in length.

[jamespetts]

I am beginning the process of reviving work on this set of features, as the critical issues on the master branch now seem to have been resolved. The first task was to merge in the latest changes from the master branch (the relevant branch is vehicle-management). I have now done this and tested it to the very limited extent that it compiles and runs by loading the Pak128.Britain-Ex demo game without crashing.

I have not tested this any more extensively than that at this stage. There may be some benefit in giving some consideration to some of the currently ongoing discussions about train consists and reversing and how they might interwork with the features on this branch. There may be something to be said for those who are doing work of that sort to inspect this branch to make sure that what is being done does not conflict with the work here.

A summary of where we are with the coding work on this branch is that we have the data structures for a large feature set, and the logic for a very small proportion of that feature set (e.g. ignoring choose signals in the schedules) implemented. Some of the UI work has been done (by Ves), but I cannot remember how far that this had got. Ves had also started work on an interesting vehicle manager concept to integrate into this project.

The next stage, beyond more basic testing of the merge, is likely to be to undertake some more theoretical design work to check how well that this integrates with the features being discussed with respect to reversing and some of the ideas that I had posited a short while ago to-day regarding how to keep a track of how many guards that trains need for economic purposes. We are still at an early enough stage that we can change fairly substantially how these features are to work, as most of the logic has not been implemented yet (and it does not currently matter if we modify the data structures and break saved games since there are no significant saved games created with this branch at this stage), but we will need to be fairly decisive within a reasonable time about the final design before work can commence on any necessary changes to the data structures and the coding of the logic and GUI.

Generally, the best sequence for coding is data structures > GUI > logic: that way, the GUI can be written so as to modify the data structures, and the logic can be easily tested using the GUI. The GUI can be modified if necessary after the implementation of the logic.

I know that Ves has been working on the GUI for this; I wonder whether Ranran and Ves might like to collaborate for continuing this work? Ranran's work on the GUI recently has been most helpful  and there is much to be said for some degree of integration of this with some other project on which Ranran is working.

In any event, the immediate next steps are:

(1) basic testing; and
(2) further discussion.

I do encourage others to test the vehicle-management branch to check for any major anomalies introduced by the merge (bearing in mind that most of the logic has not been written yet so most of the features will not function).

Ves - are you able to bring your UI work up to date with the latest code by eliminating merge conflicts?

[Ves]

Yes I am currently updating my branch to be the same as your vehicle-management, although I do have some severe merge errors for some reasons that I need to resolve...

One note to my work is that I probably had misunderstood the broadcasted and recieved bitfields. Currently it is only a combobox where you can select a number, but as I understood it you should be able to select multiple numbers. I have an idea how to do that in the GUI, but first these merge conflicts....

[jamespetts]

Excellent, thank you very much. Please let me know when you have completed the merging.

[Ves]

Well, I realize I have been making a mistake:

When coding the new Vehicle Manager, I did that in the same branch as the new schedule dialog without realizing that. That means that the Vehicle Manager will come over as well into the "main" veh-management branch.
Maybe that is not necesarily a problem, since the window is working fine in its current state, although unfinnished and only shows information.
The problem being however that I have had my setup target Win32 up until now, and when compiling it with x64 I get this error message when trying to open the window:

"Run-Time Check Failure #2 - Stack around the variable 'X' was corrupted"
I need to investigate that stack error. Any suggestion what might be causing that for 64 bit and not for 32 bit? For info 'X' is a char variable.

edit:
Oh, stupid me! I have just not given the char's enough space in certain cases!

[Ves]

So, Im finished merging and the branch can be found here:

https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/veh-managment-gui

But as I said above, the Vehicle Manager is also on that branch. I will split it in two eventually so that work can be done to them separately in the future...
If you want to have the vehicle manager button in the pakset right away, you should add the menuconf.tab that is linked here:
https://github.com/VictorErik/saved-games/blob/master/veh-managment-gui/menuconf.tab
The button is located right next to the Line Management button, although the best position might still not be found.
Also, I dont have any graphics for the button, so it borrows the graphics from the online play button...

edit:
I have now updated the schedule window to have proper bitfield broadcasters and recievers. It works by typing in the numbers you want to activate in a text field, and the textfield then sets the correct bits. The list of stops also correctly shows what bits have been selected for the particular stops.
That window is now being worked on in this new branch derived from your vehicle-management:
https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/veh-man-schedule-window
Note: You appear to already have my schedule window in your vehicle-manager branch, so there should be no merge problems with this and my vehicle manager if you wanted to have that as well.
Also note that the schedule window is rather messy now. I am thinking of how to group the settings the best way possible but there are maybe even more settings that will make it into the window eventually...

[jamespetts]

I have made a start on merging this branch back into the master branch. As anticipated, there were large numbers of merge conflicts. The priority now must be to get this branch working so that the important work of balancing can be progressed, so nothing other than critical bug fixes should take place on the master branch until this work has been completed.

I have put the merged code into a new branch, vehicle-management-new, to ensure that the original vehicle-management branch be not polluted with potentially conflicting code and to make it easier to work out the original intention of the code on this branch.

The current progress with this branch is that the code compiles and links, but the resulting executable crashes after loading a pakset: there is a saved game format incompatibility that prevents it from loading even older saved games and there is a UI related crash in tool_t::read_menu() if I load a pakset such as Pak128.Sweden-Ex which has no demo.sve.

There are a number of areas where I have noted that specific work needs to be done to bring the work done so far on this branch in line with the current UI system: I have marked all of these with "// UI TODO:" comments to make clear where in the code that these instances are.

I am unlikely to be able to undertake much further work on the code until the new year now, but if anyone would like to look at this, do have a look at the vehicle-management-new Github branch.

[Ranran]

One of the issues with running the code from the vehicle-management-new branch at present is that, on starting a pakset with no demo.sve, the game crashes on line 452 of simmenu.cc because tool is null. This looks to be a UI issue, although it is not immediately clear whether this relates to any of the UI changes discussed in this thread.

Perhaps another tool has been assigned to the planned tool number, or the variable name of the obsolete tool has changed.

EDIT:
I don't think LINE_WAYTOLL in get_stat_converted should be commented out.

[jamespetts]

Thank you for that - I have now uncommented LINE_WAYTOLL. This does not affect the above issue, but I presume that you did not imagine that it would.

Comparing the various tool definitions, we get, for dialogue tools:

vehicle-management-new

Code: [Select]

enum {
   // dialogue tools
   DIALOG_HELP = 0,
   DIALOG_OPTIONS,
   DIALOG_MINIMAP,
   DIALOG_LINEOVERVIEW,
   DIALOG_MESSAGES,
   DIALOG_FINANCES,
   DIALOG_PLAYERS,
   DIALOG_DISPLAYOPTIONS,
   DIALOG_SOUND,
   DIALOG_LANGUAGE,
   DIALOG_PLAYERCOLOR,
   DIALOG_JUMP,
   DIALOG_LOAD,
   DIALOG_SAVE,
   DIALOG_LIST_HALT,
   DIALOG_LIST_CONVOI,
   DIALOG_LIST_TOWN,
   DIALOG_LIST_GOODS,
   DIALOG_LIST_FACTORY,
   DIALOG_LIST_CURIOSITY,
   DIALOG_EDIT_FACTORY,
   DIALOG_EDIT_ATTRACTION,
   DIALOG_EDIT_HOUSE,
   DIALOG_EDIT_TREE,
   DIALOG_ENLARGE_MAP,
   DIALOG_LIST_LABEL,
   DIALOG_CLIMATES,
   DIALOG_SETTINGS,
   DIALOG_GAMEINFO,
   DIALOG_THEMES,
   DIALOG_SCENARIO,
   DIALOG_SCENARIO_INFO,
   DIALOG_LIST_DEPOT,
   DIALOG_LIST_VEHICLE,
   //DIALOG_SCRIPT_TOOL,
   DIALOG_TOOL_STANDARD_COUNT,
   // Extended entries from here:
   DIALOG_LIST_SIGNALBOX =0x0080,
   DIALOG_TOOL_COUNT,
   DIALOG_TOOL = 0x4000
};

vehicle-management

Code: [Select]

enum {
   // dialogue tools
   DIALOG_HELP = 0,
   DIALOG_OPTIONS,
   DIALOG_MINIMAP,
   DIALOG_LINEOVERVIEW,
   DIALOG_MESSAGES,
   DIALOG_FINANCES,
   DIALOG_PLAYERS,
   DIALOG_DISPLAYOPTIONS,
   DIALOG_SOUND,
   DIALOG_LANGUAGE,
   DIALOG_PLAYERCOLOR,
   DIALOG_JUMP,
   DIALOG_LOAD,
   DIALOG_SAVE,
   DIALOG_LIST_HALT,
   DIALOG_LIST_CONVOI,
   DIALOG_LIST_TOWN,
   DIALOG_LIST_GOODS,
   DIALOG_LIST_FACTORY,
   DIALOG_LIST_CURIOSITY,
   DIALOG_EDIT_FACTORY,
   DIALOG_EDIT_ATTRACTION,
   DIALOG_EDIT_HOUSE,
   DIALOG_EDIT_TREE,
   DIALOG_ENLARGE_MAP,
   DIALOG_LIST_LABEL,
   DIALOG_CLIMATES,
   DIALOG_SETTINGS,
   DIALOG_GAMEINFO,
   DIALOG_THEMES,
   DIALOGE_TOOL_COUNT,
   DIALOGE_TOOL = 0x4000
};

master

Code: [Select]

enum {
   // dialogue tools
   DIALOG_HELP = 0,
   DIALOG_OPTIONS,
   DIALOG_MINIMAP,
   DIALOG_LINEOVERVIEW,
   DIALOG_MESSAGES,
   DIALOG_FINANCES,
   DIALOG_PLAYERS,
   DIALOG_DISPLAYOPTIONS,
   DIALOG_SOUND,
   DIALOG_LANGUAGE,
   DIALOG_PLAYERCOLOR,
   DIALOG_JUMP,
   DIALOG_LOAD,
   DIALOG_SAVE,
   DIALOG_LIST_HALT,
   DIALOG_LIST_CONVOI,
   DIALOG_LIST_TOWN,
   DIALOG_LIST_GOODS,
   DIALOG_LIST_FACTORY,
   DIALOG_LIST_CURIOSITY,
   DIALOG_EDIT_FACTORY,
   DIALOG_EDIT_ATTRACTION,
   DIALOG_EDIT_HOUSE,
   DIALOG_EDIT_TREE,
   DIALOG_ENLARGE_MAP,
   DIALOG_LIST_LABEL,
   DIALOG_CLIMATES,
   DIALOG_SETTINGS,
   DIALOG_GAMEINFO,
   DIALOG_THEMES,
   DIALOG_SCENARIO,
   DIALOG_SCENARIO_INFO,
   DIALOG_LIST_DEPOT,
   DIALOG_LIST_VEHICLE,
   //DIALOG_SCRIPT_TOOL,
   DIALOG_TOOL_STANDARD_COUNT,
   // Extended entries from here:
   DIALOG_LIST_SIGNALBOX =0x0080,
   DIALOG_TOOL_COUNT,
   DIALOG_TOOL = 0x4000
};

SIMPLE TOOLS

vehicle-management-new

Code: [Select]

enum {
   // simple one click tools
   TOOL_PAUSE = 0,
   TOOL_FASTFORWARD,
   TOOL_SCREENSHOT,
   TOOL_INCREASE_INDUSTRY,
   TOOL_UNDO,
   TOOL_SWITCH_PLAYER,
   TOOL_STEP_YEAR,
   TOOL_CHANGE_GAME_SPEED,
   TOOL_ZOOM_IN,
   TOOL_ZOOM_OUT,
   TOOL_SHOW_COVERAGE,
   TOOL_SHOW_NAME,
   TOOL_SHOW_GRID,
   TOOL_SHOW_TREES,
   TOOL_SHOW_HOUSES,
   TOOL_SHOW_UNDERGROUND,
   TOOL_ROTATE90,
   TOOL_QUIT,
   TOOL_FILL_TREES,
   TOOL_DAYNIGHT_LEVEL,
   TOOL_VEHICLE_TOOLTIPS,
   TOOL_TOOGLE_PAX,
   TOOL_TOOGLE_PEDESTRIANS,
   TOOL_TRAFFIC_LEVEL,
   TOOL_CHANGE_CONVOI,
   TOOL_CHANGE_LINE,
   TOOL_CHANGE_DEPOT,
   UNUSED_WKZ_PWDHASH_TOOL,
   TOOL_CHANGE_PLAYER,
   TOOL_CHANGE_TRAFFIC_LIGHT,
   TOOL_CHANGE_CITY,
   TOOL_RENAME,
   TOOL_ADD_MESSAGE,
   TOOL_TOGGLE_RESERVATION,
   TOOL_VIEW_OWNER,
   TOOL_HIDE_UNDER_CURSOR,
   TOOL_MOVE_MAP,
   TOOL_ROLLUP_ALL_WIN,
   TOOL_RECOLOUR_TOOL_DEPRECATED,
   TOOL_ACCESS_TOOL_DEPRECATED,
   SIMPLE_TOOL_STANDARD_COUNT,
   // Extended entries from here:
   TOOL_CHANGE_ROADSIGN=0x0080,
   TOOL_SHOW_RIBI,
   TOOL_RECOLOUR_TOOL,
   TOOL_ACCESS_TOOL,
   TOOL_SHOW_SIGNALBOX_COVERAGE,
   TOOL_CONVOY_NAMEPLATES,
   TOOL_CONVOY_LOADINGBAR,
   SIMPLE_TOOL_COUNT,
   SIMPLE_TOOL = 0x2000
};

vehicle-management

Code: [Select]

enum {
   // simple one click tools
   TOOL_PAUSE = 0,
   TOOL_FASTFORWARD,
   TOOL_SCREENSHOT,
   TOOL_INCREASE_INDUSTRY,
   TOOL_UNDO,
   TOOL_SWITCH_PLAYER,
   TOOL_STEP_YEAR,
   TOOL_CHANGE_GAME_SPEED,
   TOOL_ZOOM_IN,
   TOOL_ZOOM_OUT,
   TOOL_SHOW_COVERAGE,
   TOOL_SHOW_NAME,
   TOOL_SHOW_GRID,
   TOOL_SHOW_TREES,
   TOOL_SHOW_HOUSES,
   TOOL_SHOW_UNDERGROUND,
   TOOL_ROTATE90,
   TOOL_QUIT,
   TOOL_FILL_TREES,
   TOOL_DAYNIGHT_LEVEL,
   TOOL_VEHICLE_TOOLTIPS,
   TOOL_TOOGLE_PAX,
   TOOL_TOOGLE_PEDESTRIANS,
   TOOL_TRAFFIC_LEVEL,
   TOOL_CHANGE_CONVOI,
   TOOL_CHANGE_LINE,
   TOOL_CHANGE_DEPOT,
   UNUSED_WKZ_PWDHASH_TOOL,
   TOOL_CHANGE_PLAYER,
   TOOL_CHANGE_TRAFFIC_LIGHT,
   TOOL_CHANGE_CITY,
   TOOL_RENAME,
   TOOL_ADD_MESSAGE,
   TOOL_TOGGLE_RESERVATION,
   TOOL_VIEW_OWNER,
   TOOL_HIDE_UNDER_CURSOR,
   TOOL_CHANGE_ROADSIGN,
   TOOL_SHOW_RIBI,
   TOOL_RECOLOUR_TOOL,
   TOOL_ACCESS_TOOL,
   SIMPLE_TOOL_COUNT,
   SIMPLE_TOOL = 0x2000
};

master

Code: [Select]

enum {
   // simple one click tools
   TOOL_PAUSE = 0,
   TOOL_FASTFORWARD,
   TOOL_SCREENSHOT,
   TOOL_INCREASE_INDUSTRY,
   TOOL_UNDO,
   TOOL_SWITCH_PLAYER,
   TOOL_STEP_YEAR,
   TOOL_CHANGE_GAME_SPEED,
   TOOL_ZOOM_IN,
   TOOL_ZOOM_OUT,
   TOOL_SHOW_COVERAGE,
   TOOL_SHOW_NAME,
   TOOL_SHOW_GRID,
   TOOL_SHOW_TREES,
   TOOL_SHOW_HOUSES,
   TOOL_SHOW_UNDERGROUND,
   TOOL_ROTATE90,
   TOOL_QUIT,
   TOOL_FILL_TREES,
   TOOL_DAYNIGHT_LEVEL,
   TOOL_VEHICLE_TOOLTIPS,
   TOOL_TOOGLE_PAX,
   TOOL_TOOGLE_PEDESTRIANS,
   TOOL_TRAFFIC_LEVEL,
   TOOL_CHANGE_CONVOI,
   TOOL_CHANGE_LINE,
   TOOL_CHANGE_DEPOT,
   UNUSED_WKZ_PWDHASH_TOOL,
   TOOL_CHANGE_PLAYER,
   TOOL_CHANGE_TRAFFIC_LIGHT,
   TOOL_CHANGE_CITY,
   TOOL_RENAME,
   TOOL_ADD_MESSAGE,
   TOOL_TOGGLE_RESERVATION,
   TOOL_VIEW_OWNER,
   TOOL_HIDE_UNDER_CURSOR,
   TOOL_MOVE_MAP,
   TOOL_ROLLUP_ALL_WIN,
   TOOL_RECOLOUR_TOOL_DEPRECATED,
   TOOL_ACCESS_TOOL_DEPRECATED,
   SIMPLE_TOOL_STANDARD_COUNT,
   // Extended entries from here:
   TOOL_CHANGE_ROADSIGN=0x0080,
   TOOL_SHOW_RIBI,
   TOOL_RECOLOUR_TOOL,
   TOOL_ACCESS_TOOL,
   TOOL_SHOW_SIGNALBOX_COVERAGE,
   TOOL_CONVOY_NAMEPLATES,
   TOOL_CONVOY_LOADINGBAR,
   SIMPLE_TOOL_COUNT,
   SIMPLE_TOOL = 0x2000
};


GENERAL TOOLS

vehicle-management-new

Code: [Select]

enum {
   // general tools
   TOOL_QUERY=0,
   TOOL_REMOVER,
   TOOL_RAISE_LAND,
   TOOL_LOWER_LAND,
   TOOL_SETSLOPE,
   TOOL_RESTORESLOPE,
   TOOL_MARKER,
   TOOL_CLEAR_RESERVATION,
   TOOL_TRANSFORMER,
   TOOL_ADD_CITY,
   TOOL_CHANGE_CITY_SIZE,
   TOOL_PLANT_TREE,
   TOOL_SCHEDULE_ADD,
   TOOL_SCHEDULE_INS,
   TOOL_BUILD_WAY,
   TOOL_BUILD_BRIDGE,
   TOOL_BUILD_TUNNEL,
   TOOL_WAYREMOVER,
   TOOL_BUILD_WAYOBJ,
   TOOL_BUILD_STATION,
   TOOL_BUILD_ROADSIGN,
   TOOL_BUILD_DEPOT,
   TOOL_BUILD_HOUSE,
   TOOL_BUILD_LAND_CHAIN,
   TOOL_CITY_CHAIN,
   TOOL_BUILD_FACTORY,
   TOOL_LINK_FACTORY,
   TOOL_HEADQUARTER,
   TOOL_LOCK_GAME,
   TOOL_ADD_CITYCAR,
   TOOL_FOREST,
   TOOL_STOP_MOVER,
   TOOL_MAKE_STOP_PUBLIC,
   TOOL_REMOVE_WAYOBJ,
   TOOL_SLICED_AND_UNDERGROUND_VIEW,
   TOOL_BUY_HOUSE,
   TOOL_BUILD_CITYROAD,
   TOOL_ERROR_MESSAGE,
   TOOL_CHANGE_WATER_HEIGHT,
   TOOL_SET_CLIMATE,
   TOOL_ROTATE_BUILDING,
   TOOL_REASSIGN_SIGNAL_DEPRECATED,
   GENERAL_TOOL_STANDARD_COUNT,
   // Extended entries from here:
   TOOL_BUILD_SIGNALBOX=0x0080,
   TOOL_REASSIGN_SIGNAL,
   GENERAL_TOOL_COUNT,
   GENERAL_TOOL = 0x1000
};

vehicle-management

Code: [Select]

enum {
   // general tools
   TOOL_QUERY=0,
   TOOL_REMOVER,
   TOOL_RAISE_LAND,
   TOOL_LOWER_LAND,
   TOOL_SETSLOPE,
   TOOL_RESTORESLOPE,
   TOOL_MARKER,
   TOOL_CLEAR_RESERVATION,
   TOOL_TRANSFORMER,
   TOOL_ADD_CITY,
   TOOL_CHANGE_CITY_SIZE,
   TOOL_PLANT_TREE,
   TOOL_SCHEDULE_ADD,
   TOOL_SCHEDULE_INS,
   TOOL_BUILD_WAY,
   TOOL_BUILD_BRIDGE,
   TOOL_BUILD_TUNNEL,
   TOOL_WAYREMOVER,
   TOOL_BUILD_WAYOBJ,
   TOOL_BUILD_STATION,
   TOOL_BUILD_ROADSIGN,
   TOOL_BUILD_DEPOT,
   TOOL_BUILD_HOUSE,
   TOOL_BUILD_LAND_CHAIN,
   TOOL_CITY_CHAIN,
   TOOL_BUILD_FACTORY,
   TOOL_LINK_FACTORY,
   TOOL_HEADQUARTER,
   TOOL_LOCK_GAME,
   TOOL_ADD_CITYCAR,
   TOOL_FOREST,
   TOOL_STOP_MOVER,
   TOOL_MAKE_STOP_PUBLIC,
   TOOL_REMOVE_WAYOBJ,
   TOOL_SLICED_AND_UNDERGROUND_VIEW,
   TOOL_BUY_HOUSE,
   TOOL_BUILD_CITYROAD,
   TOOL_ERROR_MESSAGE,
   TOOL_CHANGE_WATER_HEIGHT,
   TOOL_SET_CLIMATE,
   TOOL_BUILD_SIGNALBOX,
   TOOL_REASSIGN_SIGNAL,
   GENERAL_TOOL_COUNT,
   GENERAL_TOOL = 0x1000
};

master

Code: [Select]

enum {
   // general tools
   TOOL_QUERY=0,
   TOOL_REMOVER,
   TOOL_RAISE_LAND,
   TOOL_LOWER_LAND,
   TOOL_SETSLOPE,
   TOOL_RESTORESLOPE,
   TOOL_MARKER,
   TOOL_CLEAR_RESERVATION,
   TOOL_TRANSFORMER,
   TOOL_ADD_CITY,
   TOOL_CHANGE_CITY_SIZE,
   TOOL_PLANT_TREE,
   TOOL_SCHEDULE_ADD,
   TOOL_SCHEDULE_INS,
   TOOL_BUILD_WAY,
   TOOL_BUILD_BRIDGE,
   TOOL_BUILD_TUNNEL,
   TOOL_WAYREMOVER,
   TOOL_BUILD_WAYOBJ,
   TOOL_BUILD_STATION,
   TOOL_BUILD_ROADSIGN,
   TOOL_BUILD_DEPOT,
   TOOL_BUILD_HOUSE,
   TOOL_BUILD_LAND_CHAIN,
   TOOL_CITY_CHAIN,
   TOOL_BUILD_FACTORY,
   TOOL_LINK_FACTORY,
   TOOL_HEADQUARTER,
   TOOL_LOCK_GAME,
   TOOL_ADD_CITYCAR,
   TOOL_FOREST,
   TOOL_STOP_MOVER,
   TOOL_MAKE_STOP_PUBLIC,
   TOOL_REMOVE_WAYOBJ,
   TOOL_SLICED_AND_UNDERGROUND_VIEW,
   TOOL_BUY_HOUSE,
   TOOL_BUILD_CITYROAD,
   TOOL_ERROR_MESSAGE,
   TOOL_CHANGE_WATER_HEIGHT,
   TOOL_SET_CLIMATE,
   TOOL_ROTATE_BUILDING,
   TOOL_REASSIGN_SIGNAL_DEPRECATED,
   GENERAL_TOOL_STANDARD_COUNT,
   // Extended entries from here:
   TOOL_BUILD_SIGNALBOX=0x0080,
   TOOL_REASSIGN_SIGNAL,
   GENERAL_TOOL_COUNT,
   GENERAL_TOOL = 0x1000
};

Incidentally, I will be moving/have moved this discussion to the schedule features (technical discussion) thread as the details of integrating the vehicle-management branch into the current branch more properly belong there.
Edit: Incidentally, an initial analysis shows that the vehicle-management-new branch's tool definitions in simmenu.h are identical to those in the master branch, suggesting no changes between the old master and vehicle-management branches and that these merged cleanly.

[Ranran]

Can you check if the fix in this thread avoids that crash?

[jamespetts]

Can you check if the fix in this thread avoids that crash?

I am getting linker errors when I try to build this. In Linux, I get:

Code: [Select]

===> LD  simutrans/simutrans-extended
/usr/bin/ld: build/default/dataobj/schedule.o: in function `schedule_t::rdwr(loadsave_t*)':
schedule.cc:(.text+0x6bf6): undefined reference to `consist_order_t::rdwr(loadsave_t*)'
/usr/bin/ld: schedule.cc:(.text+0x6fde): undefined reference to `consist_order_t::rdwr(loadsave_t*)'
collect2: error: ld returned 1 exit status
make: *** [common.mk:27: simutrans/simutrans-extended] Error 1

I cannot be sure whether this is caused by this patch, however, since I have not previously compiled the vehicle-management-new branch on Linux and do not currently have access to my Windows machine and will not until after Christmas (as I am away from home with my "support bubble" for the festive season).

Can you confirm whether you can compile this branch before and after merging this commit?

Thank you for your work on this.

[Ranran]

Can you confirm whether you can compile this branch before and after merging this commit?

EDIT2:
It was both compileable. Also, after merging the changes, pak.sweden-EX is now bootable.
It is uncompileable on Linux because the Makifile is missing the line const_order_t.cc.

EDIT3:
It doesn't seem to be compatible with old save data. Therefore, it cannot be started if there is demo.sve.
Also, all save versions and paksets will not be readable.

It looks like there are a number of places where you have to change the color definition from (uint8) index to (PIXVAL) rgb.
For example, COL_BLACK must be color_idx_to_rgb(COL_BLACK) if PIXVAL is specified.
It should be noted that this is the same for player colors like get_player_color1().

Some dialogs are still partially old GUI engines. It's probably better to have a new GUI when making changes if possible. For example, UI TODO was in convoy detail, but that part was still being drawn by the old GUI engine.
Now that the font size is variable, we have to deal with it, and the new GUI engine reduces that effort.
The schedule-related GUI, which this patch is likely to have a significant impact on, unfortunately didn't merge well and remains mostly old GUI. I wasn't familiar with the code added to Extended so it was difficult to fix it correctly. For example, there are parameters related to the control tower. And it seems that it has changed a lot in standard now.
Until recently, schedule_list wasn't a new GUI for standard either. However, it seems that a new GUI is currently being developed.

EDIT4:
I've fixed some UI TODO parts.
Information about the overhaul date of convoy detail has been moved to the maintenance tab.
Fixed Makefile.
It's on my veh-mgr branch.

EDIT5:
The game pauses when opening the depot dialog or replace dialog. It seems to be overloaded. (EDIT6: This could only be railways.)
The depot dialog and the replace dialog are extremely unresponsive and lagging. (EDIT6: This could only be railways.)
For some reason, the list of selectable livery in the depot dialog was corrupted. Lists are displayed repeatedly. (EDIT6: It's broken on the railroad, but it doesn't seem to be broken on the bus. EDIT7: I couldn't reproduce this when I started a new map.)
The schedule gui has a broken layout, but I think it's a good idea to switch to a new GUI engine. The current code is hard to read. The code will be very slim because you don't have to calculate useless coordinates. Label declarations may also be omitted. I am afraid that if we miss this opportunity, we will not be able to change this in the future...

I think the gimme_stop_name and gimme_short_stop_name was moved from schedule_gui_t to schedule_t due to a standard's change.

EDIT8:
I was able to proceed with some work to switch schedule_gui to the new GUI engine. I'm still working on it. I'm not sure how to arrange the newly added items.

[jamespetts]

Excellent, thank you for this: this is most helpful. I have now managed to get this to compile on Linux, and your patch does indeed allow me to start the game using a pakset with no demo.sve. The saved game incompatibility I had noted above - this will need some careful and lengthy work to resolve when I return home. I have merged your veh-mgr branch and pushed this to the vehicle-management-new branch.

The work to integrate this is likely to be extensive, and, because I am now away from my normal development computer, I will not be able to work on this properly until the new year. However, this will be the coding priority when I return to it.

The UI elements definitely all need converting to the new UI system. Not much in the way of UI has been implemented for this branch so far (Ves had some some work, but that was on a separate branch, not merged), so most of the UI divergences were relatively minor and simply accommodating some of the more basic changes (although there were some differences to the code relating to liveries which I believe may have affected the UI more). It may be instructive to compare the vehicle-management branch and the master branch as it was in January 2018 to see what the changes were intended to be. I suspect that the sensible approach to the UI may well be to start again with the vehicle-management UI, resetting the UI on that branch to what it is in the current master versions and then re-implementing the UI for this branch using the new system.

In any event, thank you for your assistance with this - it is much appreciated.

[freddyhayward]

I have been trying to investigate why savegames would not load. I found that something was causing convois to attempt to load millions of departure times instead of hundreds as they currently do. However, I can't find a single change within convoi_t::rdwr that could remotely affect this at all.

[Ranran]

The schedule departure time structure seems to change with this patch.

Code: [Select]

// UI TODO: Make it clearer to the player that this is set in increments of 12ths of a fraction of a month.

[freddyhayward]

The schedule departure time structure seems to change with this patch.

Code: [Select]

// UI TODO: Make it clearer to the player that this is set in increments of 12ths of a fraction of a month.

I still cannot see how this could affect loading old saves. The problem seems to occur when the count is read incorrectly, before reading the times themselves.
EDIT: Found it! Loading schedules were checking for the wrong version.

[freddyhayward]

I can now load old savegames after the changes from this pull request: https://github.com/jamespetts/simutrans-extended/pull/318
EDIT: The main issue now is that schedules load incorrectly, although at least they now load.
EDIT: Schedules now load correctly.

[freddyhayward]

The work to integrate this is likely to be extensive, and, because I am now away from my normal development computer, I will not be able to work on this properly until the new year. However, this will be the coding priority when I return to it.

The UI elements definitely all need converting to the new UI system. Not much in the way of UI has been implemented for this branch so far (Ves had some some work, but that was on a separate branch, not merged), so most of the UI divergences were relatively minor and simply accommodating some of the more basic changes (although there were some differences to the code relating to liveries which I believe may have affected the UI more). It may be instructive to compare the vehicle-management branch and the master branch as it was in January 2018 to see what the changes were intended to be. I suspect that the sensible approach to the UI may well be to start again with the vehicle-management UI, resetting the UI on that branch to what it is in the current master versions and then re-implementing the UI for this branch using the new system.

May I ask what this means in more detail? I was able to uncomment some of the code under the TODO comments, and it seemed to behave correctly despite obvious alignment issues.

[jamespetts]

Thank you for that: that is most helpful. As you have probably discovered by now, when you get a list which tries to load millions of entries in circumstances where the list would not have anything like that number of entries in reality, that is usually because it is trying to read the wrong sort of data at that point in the load/save cycle, suggesting an error at an earlier point in the code path. Thank you very much for your work of fixing these: that is most helpful.

As explained above, I will not be able to do any significant work on this code until the new year, but I have tried to merge and compile this. Unfortunately, I get linker errors when compiling on Linux as follows:

Code: [Select]

===> LD  simutrans/simutrans-extended
/usr/bin/ld: simutrans/player/ai_goods.o: in function `ai_goods_t::create_ship_transport_vehicle(fabrik_t*, int)':
ai_goods.cc:(.text+0x1dad): undefined reference to `schedule_t::append(grund_t const*, unsigned short, unsigned char, short, bool)'
/usr/bin/ld: ai_goods.cc:(.text+0x1e66): undefined reference to `schedule_t::append(grund_t const*, unsigned short, unsigned char, short, bool)'
/usr/bin/ld: simutrans/player/ai_goods.o: in function `ai_goods_t::create_road_transport_vehicle(fabrik_t*, int)':
ai_goods.cc:(.text+0x2701): undefined reference to `schedule_t::append(grund_t const*, unsigned short, unsigned char, short, bool)'
/usr/bin/ld: ai_goods.cc:(.text+0x27bc): undefined reference to `schedule_t::append(grund_t const*, unsigned short, unsigned char, short, bool)'
/usr/bin/ld: simutrans/player/ai_goods.o: in function `ai_goods_t::create_rail_transport_vehicle(koord, koord, int, int)':
ai_goods.cc:(.text+0x2d4a): undefined reference to `schedule_t::append(grund_t const*, unsigned short, unsigned char, short, bool)'
/usr/bin/ld: simutrans/player/ai_goods.o:ai_goods.cc:(.text+0x2dfb): more undefined references to `schedule_t::append(grund_t const*, unsigned short, unsigned char, short, bool)' follow
/usr/bin/ld: simutrans/player/simplay.o: in function `player_t::begin_liquidation()':
simplay.cc:(.text+0x26c8): undefined reference to `convoi_t::go_to_depot(bool, bool)'
/usr/bin/ld: simplay.cc:(.text+0x26de): undefined reference to `convoi_t::emergency_go_to_depot(bool)'
collect2: error: ld returned 1 exit status
make: *** [common.mk:27: simutrans/simutrans-extended] Error 1

I wonder whether we are missing a makefile entry somewhere...?
As to the UI issues, may I ask what part that you are asking about its meaning? I had not tested the commented out sections, but they seemed to be incompatible with the new UI system/code. If you find that they work, they may not be as incompatible as I had thought; I am not familiar with exactly what parts of the UI code have changed and what parts have remained the same.

[freddyhayward]

I have tried to merge and compile this. Unfortunately, I get linker errors when compiling on Linux as follows:

Are you able to compile the branch before my changes without that same issue? If so, I'm not sure what I have could have changed to cause it.

[freddyhayward]

I had not tested the commented out sections, but they seemed to be incompatible with the new UI system/code.

As far as I understand it, both the old and new systems work fine together. However, Ranran would have to confirm this.

[Ranran]

The UI has only changed on the surface and the functionality does not appear to work properly.
The layout is broken where the conflict occurred.
Some intended GUI display changes appear to be commented out due to conflicts that have occurred. For example, schedule.

As I have already reported, the livery display in some dialogs is corrupted.




This does not seem to happen all the time, but it seems to be caused by some operation.
I've said before that it might happen on the railroad, but it probably happens on all.

In this case, the livery selector will repeat the same list too.

[freddyhayward]

I made a tentative pull request that uncomments the schedule code and makes a few changes to make it work: https://github.com/jamespetts/simutrans-extended/pull/319

[Ranran]

I made a tentative pull request that uncomments the schedule code and makes a few changes to make it work:

This change is incorrect. Notice that the function is duplicated because the variables have been moved as described above. That is, there are duplicate functions.

[freddyhayward]

This change is incorrect. Notice that the function is duplicated because the variables have been moved as described above. That is, there are duplicate functions.

I tried again here: https://github.com/jamespetts/simutrans-extended/pull/320
edit: now here: https://github.com/jamespetts/simutrans-extended/pull/324

[Freahk]

I have put the merged code into a new branch, vehicle-management-new, to ensure that the original vehicle-management branch be not polluted with potentially conflicting code and to make it easier to work out the original intention of the code on this branch.

This sounds a little ungitish.
Just in case you didn't know, you can check out any commited state of a branch and you can name commits using git tag.
So if you want to "freeze" a branch in a specific state, e.g. to prevent it from "pollution" just to get some kind of a "backup" to rollback if something went wrong, all you have to do is to commit that state.
As you usually don't want to remember the exact commit id, you can simply give it a name using git tag.

[freddyhayward]

Having briefly read the prior discussion on this thread and parts of the code, the design seems extremely ambitious and complicated. I worry that it will take a near-infinite amount of time to be brought into a usable, let alone user-friendly state. Especially suspect is the idea of triggers, broadcasters and receivers. These seem to offer unnecessarily high levels of precision and complexity. I don't currently have an alternative design, but I hope that through further discussion here, either one can be developed or the current design sufficiently explained and justified.

[Ranran]

I updated this to a new GUI code style to repair the broken schedule UI layout. (The lower station list is not yet in the new GUI code style) And I changed some layouts.

 I would appreciate it if you could give your opinion on this.