The International Simutrans Forum

 

Author Topic: [patch] synchronize lines  (Read 4616 times)

0 Members and 1 Guest are viewing this topic.

Offline smu42

  • *
  • Posts: 37
  • Languages: German, English
[patch] synchronize lines
« on: January 14, 2010, 01:46:26 PM »

In the German simutrans forum, I made a suggestion for a patch.
http://www.simutrans-forum.de/forum/thread.php?threadid=5131

The basic idea is to have another option beyond "month wait time" and "min load" in the linemanagement window, which could be called "synchronize with line", where a line (or lines) could be selected.

When enabled, a convoy will wait until another convoy of the selected line arrives, thus "synchronizing" with the other line. This condition will be OR-ed with the other two (wait time, min load). This means that a train will start if ONE of the possibly three conditions are true.

I wrote a patch that does part of what i described:
In this patch there is NO GUI change. A convey at a station where "wait time" and "min load" is enabled also starts, when ANY other train arrives.

I tried to change the GUI (it seems obvious how to do it), but I always got strange bugs like crashing when pressing ESC. Basically the GUI (gui_fahrplan.cc) would need a component to select a line.
If someone would do this or give me one or two hints (I read the coding_styles.txt) how to do that, I would be very thankful.

The patch contains some other very small changes, that make life easier when using eclipse CDT. These changes are usefull too, but they don't have anything to do with the basic idea. I can separate this in two patches, if somebody finds that useful.

The patch (svn diff) is uploaded:
http://simutrans-germany.com/~patches/upload/smu42.synchronizelines.patch

An executable for Linux is uploaded here (7.9MB too big for http://patches.simutrans-germany.com/) :
http://87.230.77.206/svenu/simutrans/sim_patch_syncline1.zip

A savegame for a quick test (pak64) can be found here:
http://87.230.77.206/svenu/simutrans/debug.sve


Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9559
  • Languages: De,EN,JP
Re: [patch] synchronize lines
« Reply #1 on: January 14, 2010, 02:03:54 PM »
This will not be included into trunk, as it forces users to use lines. Furthermore, waiting for a certain number of load should achieve the same for passengers.

Programming the UI is a little bit difficult. HOwever, without seeing what you changed a comment on what needs to be imporved is hard. (And use "strip sim" to remove the symbol table from your uploaded packages would be a good idea.)

And those ifdefs are not needed, only if the shell in question does not use make. Compiling with wrongs files should give an error; as it does now.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4601
  • Languages: EN, DE, AT
Re: [patch] synchronize lines
« Reply #2 on: January 14, 2010, 03:02:03 PM »
Please provide patches only with the relevant changes.

Why not implement all the checks in convoi_t::laden() directly (instead of simworld.cc)?

Also loading convois in first-in-first-loaded style per line would be nice.

@prissi: Imho, using lines is the only possibility to manage larger networks without losing control. And forcing to use lines would save a lot of double code in rebuild_destinations et al.

Offline Fabio

  • Devotee
  • Administrator
  • *
  • Posts: 2898
  • The Pak128 Guy
    • Visit me on Facebook
  • Languages: EN, IT, RO, FR
Re: [patch] synchronize lines
« Reply #3 on: January 14, 2010, 03:14:27 PM »
@prissi: Imho, using lines is the only possibility to manage larger networks without losing control. And forcing to use lines would save a lot of double code in rebuild_destinations et al.

Could it possible to save individual schedules as lines, to achieve this?

Offline Spike

  • *
  • Posts: 1361
  • First Simutrans Developer and Graphics Artist
Re: [patch] synchronize lines
« Reply #4 on: January 14, 2010, 03:20:59 PM »
Lines work badly for ships, and often cause a lot of ships to run on the same square.

Offline smu42

  • *
  • Posts: 37
  • Languages: German, English
Re: [patch] synchronize lines
« Reply #5 on: January 14, 2010, 06:48:55 PM »
Quote
This will not be included into trunk, as it forces users to use lines.

I don't think it is a good idea to play simutrans without lines. I never did after my first game. However, I can write additional code to make it possible to wait for a specific convoy. Shouldn't be that difficult.
I never thought of including this as it is into the trunk. I posted it to be reviewed and to get hints.


Quote
Furthermore, waiting for a certain number of load should achieve the same for passengers.

It does something similar, but not the same. The main reason was aesthetical (and more realism).


Quote
And use "strip sim" to remove the symbol table

I didn't know the "strip" program. Very nice :) (I usually don't program C/C++).


Quote
HOwever, without seeing what you changed a comment on what needs to be imporved is hard.

I did in one sentence:
Quote
Basically the GUI (gui_fahrplan.cc) would need a component to select a line.
If someone would write GUI code to save the selected line (or convoy) in some variable and give me that patch, I would do the rest. This variable should probably be one in the struct in linieneintrag.h (near the TODO comment).



Quote
And those ifdefs are not needed, only if the shell in question does not use make. Compiling with wrongs files should give an error; as it does now.

I know they are not needed, but they are helpful in an IDE. One only has to set the correct DEFINES (allegro, sdl, x11...) in the IDE (like int the config.default file) and it works. I think nobody should have a problem with that. Next time I will provide seperate patches.


Quote
Why not implement all the checks in convoi_t::laden() directly (instead of simworld.cc)?

Because that wouldnt work. convoi_t::laden() changes the state of the convoi. I need to check the state of both convois in the synchronization-check. The result would vary on the order of the convois. (But good point, I ran into that problem after starting to program).


Quote
Also loading convois in first-in-first-loaded style per line would be nice.

Do you mean that the first convoi should finish loading before the second convoi starts loading? I wouldn't find that very helpful.


The stripped sim executable:
http://simutrans-germany.com/~patches/download.php?file=sim.zip

The cleaned up patch:
http://simutrans-germany.com/~patches/upload/synclines_only.diff

Offline skreyola

  • *
  • Posts: 337
  • Trivial Troubleshooter
Re: [patch] synchronize lines
« Reply #6 on: January 14, 2010, 07:55:11 PM »
I'd love to be able to synchronize lines. It's so frustrating to have two buses on intersecting lines just miss each other because the waiting bus left a nanosecond before the coming bus unloaded.

@prissi: Waiting for load is fine, but if the other convoy arrives empty, or nearly empty, the waiting convoy is stuck there (possibly leading to excess waits on another part of the network.

@Hajo: Ships need a choose buoy that acts like a choose sign for road/rail stations. I know that's a ton of extra work, so I just let the ships stack up on a single tile. They represent 1 (or 1/4) km^2, after all. :)

I'd support this for either the main trunk or the experimental branch. It's a good idea.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4601
  • Languages: EN, DE, AT
Re: [patch] synchronize lines
« Reply #7 on: January 15, 2010, 06:24:23 AM »
Quote
If someone would write GUI code to save the selected line (or convoy) in some variable and give me that patch, I would do the rest. This variable should probably be one in the struct in linieneintrag.h (near the TODO comment).
You may want to look, how this is done in gui/fahrplan_gui.cc fahrplan_gui_t::action_triggered for the tasks "change wating level", "promote to line" etc.

I do not like, that the check is done in karte_t::step in each step for each convoi.

Why not do the check, when a convoi arrives in a station? I mean, the convoi that arrives at a station, checks whether someone waits for him, and tells these waiting convois to go on. That check could be placed in convoi_t::hat_gehalten after the loading/unloading stuff.

Offline smu42

  • *
  • Posts: 37
  • Languages: German, English
Re: [patch] synchronize lines
« Reply #8 on: January 15, 2010, 12:30:40 PM »

Quote
I'd support this for either the main trunk or the experimental branch. It's a good idea.

I would prefer it in the main trunk. Thanks for your support.


Quote
I do not like, that the check is done in karte_t::step in each step for each convoi.

Me neither.

Quote
Why not do the check, when a convoi arrives in a station? I mean, the convoi that arrives at a station, checks whether someone waits for him, and tells these waiting convois to go on. That check could be placed in convoi_t::hat_gehalten after the loading/unloading stuff.

This would not work, if two convoys are synchronized with each other at the same station (which by the way is the case in the savegame debug.sve).
But if the check would be done not only for the waiting convoys but also for the arriving convoys, this could work. I'll give it a try at the weekend.


Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4601
  • Languages: EN, DE, AT
Re: [patch] synchronize lines
« Reply #9 on: January 15, 2010, 01:00:27 PM »
This would not work, if two convoys are synchronized with each other at the same station (which by the way is the case in the savegame debug.sve).
I think you are right here. Both arriving and waiting should check. But then some optimization is needed. Maybe add a list of (convoihandle_t, linehandle_t) at each station, which holds the convois that wait for a specific line.

What happens if a convoi waits for a line that does not serve the station? Or if the line was changed not to halt there anymore?

Some improvement in convoi_t::wait_for_other_line(): you could speed this up by using const slist_tpl<tile_t> & haltestelle_t::get_tiles() instead iterating over all possible convois.

And you should really provide some basic gui stuff. Otherwise nobody can test the patch.

Offline smu42

  • *
  • Posts: 37
  • Languages: German, English
Re: [patch] synchronize lines
« Reply #10 on: January 15, 2010, 03:53:00 PM »
Quote
Or if the line was changed not to halt there anymore?

I think a nice, user friendly solution would be the following: Whenever the user changes or deletes a line and some other line would break by this action a test would be started. If the test fails, the user would be warned ("this actions also changes Line '42'. Do you want to continue?"). If he continues, the synchronization of the corresponding halts at the corresponding lines would be disabled.
(However, such dialog seems uncommon to me in simutrans)

Quote
What happens if a convoi waits for a line that does not serve the station?

Same applies here, so this situation would never occur.


Quote
And you should really provide some basic gui stuff. Otherwise nobody can test the patch.

I will try to do that.


Quote
Some improvement in convoi_t::wait_for_other_line(): you could speed this up by using const slist_tpl<tile_t> & haltestelle_t::get_tiles() instead iterating over all possible convois.

Ok, I didn't know the tiles had a reference to the convoys. I will look at that.


Quote
But then some optimization is needed. Maybe add a list of (convoihandle_t, linehandle_t) at each station, which holds the convois that wait for a specific line.

Seems reasonable, I will think about that.


Another thing to consider is the saving of the gamestate, I will do that later.


Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4601
  • Languages: EN, DE, AT
Re: [patch] synchronize lines
« Reply #11 on: January 15, 2010, 06:31:48 PM »
Yes saving is will be an issue here, networking too. But first things first: GUI.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9559
  • Languages: De,EN,JP
Re: [patch] synchronize lines
« Reply #12 on: January 15, 2010, 08:46:04 PM »
Well, I do not want this into the trunk for the reason to avoid cluttering of the schedule dialog. Just look at the OpenTTD ones, and you know why there are not "only unload" etc. settting, even though the game machine could handle that.

Moreover, from an economic point of vue, this serves no purpose, that is not better fulfilled by waiting for a certain load percentage. The situation would be different in simutrans experimental, as there the total traveling time is important and thus even premature depature may make sense.

The only purpose of such a function would be to avoid parral loading of similar convois for one line in the same station. But I fixing this (which I currently do) would be much better.

As such I feel this patch belong to Simutrans experimental. (But the way, you could just check for arriving convoi counter of the station and only check for arrival when it changes, saving a lot of CPU.)

Offline Spike

  • *
  • Posts: 1361
  • First Simutrans Developer and Graphics Artist
Re: [patch] synchronize lines
« Reply #13 on: January 17, 2010, 04:41:04 PM »
As such I feel this patch belong to Simutrans experimental.

This looks like a good suggestion to me too. STE is geared towards detailism and realism and this patch is right in this line.

Offline jamespetts gb

  • Simutrans-Extended project coordinator
  • Moderator
  • *
  • Posts: 18745
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: [patch] synchronize lines
« Reply #14 on: January 20, 2010, 08:52:25 PM »
I have just had a chance to read this thread (I have been very busy in the last two weeks or so). This is indeed something that might be worthwhile for Simutrans-Experimental. Smu - do you have access to Github? If you create a branch on Github of the Simutrans-Experimental code and produce a branch with your new feature, I could take a look at it (although possibly not for a while, as I have a large backlog of difficult integration of changes from Simutrans-Standard to deal with first).

Offline smu42

  • *
  • Posts: 37
  • Languages: German, English
Re: [patch] synchronize lines
« Reply #15 on: February 04, 2010, 04:50:03 PM »
Quote
Smu - do you have access to Github?

I do now, username is "smu42".

I'm kind of busy in the next few weeks, but I will provide a git patch in the by the end of march at the latest.