The International Simutrans Forum

Development => Patches & Projects => Considered Patches => Topic started by: isidoro on February 04, 2009, 02:07:55 AM

Title: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 04, 2009, 02:07:55 AM
I talked with James Petts in a thread, whose subject I don't want to remember, about programming some replacing/upgrading/refurbishing related tools.  After some ten days of work, I would like to share with you what I've got so far.

The patch attached below is for r2260.  I have frozen there and will merge when done if needed.  The present version does, in fact, nothing.  I've split depot_frame functionality in two classes: depot_frame and gui_convoy_assembler.  The idea is to reuse the latter component in the replacing tool.  The only new thing players can see is that I've changed the actions button at the bottom right corner with a combo box.  The whole process has been quite complicated for me, so if you find errors, please tell.

The idea of doing this came from having a game with pak.german with 260 horse carriages to be replaced by buses.  I thought, how naïve myself, that perhaps programming a replacing tool would be easier than to replace the 260 vehicles.  I guess not.  Now, in the middle of the bridge, I will try to reach the other end...

EDIT: half of the patch was not in the file.  Please, look for the corrected version downstream...

Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: gerw on February 04, 2009, 08:25:50 AM
So the whole lower part of the depot is a gui_convoy_assembler? Seems to be very nice! What will be the end? 'Replace one horse carriage with one bus' or even 'replace 3 horse carriage with one bus'?

Edit: I've found a major bug: You forgot 'svn add gui/components/gui_convoy_assembler.*' ;) I hope you can fix it :)
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on February 04, 2009, 09:19:02 AM
Very interesting! I do like the combo box idea... I should be very interested in any progress.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: VS on February 04, 2009, 09:39:28 AM
You will want to make the combobox text field longer to allow for translations. And some sort of arrow pointing down would be nice, too, since it is common in comboboxes across most platforms (and where not, it is double arrow pointing up and down, so...).
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: HeinBloed on February 04, 2009, 11:11:17 AM
Funny coincidence as I only thought last night that this is a feature Simutrans must have. :) My ideas were:

- It should be possible to replace either single components (e.g. a certain rail car X in all convoys) or entire configurations (e.g. all trains with head W and cars XYZ).
- In order to make the transition appear realistic, one could add a new command "upgrade at depot" to the concerned convoys and at the same time set them to "no load". The upgrade command could be inserted after a whole cycle of the current schedule has passed since the convoy is guaranteed to be empty then. The convoy would then drive to the depot, get upgraded and pick up normal schedule again.

Maybe this can serve as an additional inspiration, I'm sure you already had your own plans.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 04, 2009, 01:21:13 PM
Thanks for your support.

Quote from: gerw on February 04, 2009, 08:25:50 AM
So the whole lower part of the depot is a gui_convoy_assembler? Seems to be very nice! What will be the end? 'Replace one horse carriage with one bus' or even 'replace 3 horse carriage with one bus'?

Edit: I've found a major bug: You forgot 'svn add gui/components/gui_convoy_assembler.*' ;) I hope you can fix it :)

Not exactly.  The convoy assembler is the image of the convoy plus the lower part.  There is a configurable hole in the middle.  The idea is not to be forced to change depot's present configuration.  My idea is to be able to replace only a certain number of convoys since it is very common that one bus can do the job of several horse carriages.  Thanks for the bug (half of the patch was out)...  I attach the corrected one below and delete the one above.

@James: Thanks

@VS: The size is the same of the old button, but can be easily changed.  The combo box was not made by me.  It is the standard combo-box in simutrans (used for line selection at the top of the window).  That, if wanted, would be a separate development.

@tttron: I don't believe in coincidence  :D .  As for your ideas: in the first phase at least only whole convoys may be replaced.  The "upgrade at depot" is already intended or something similar. 
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: Dwachs on February 04, 2009, 01:50:47 PM
Quote from: isidoro on February 04, 2009, 01:21:13 PM
@VS: The size is the same of the old button, but can be easily changed.  The combo box was not made by me.  It is the standard combo-box in simutrans (used for line selection at the top of the window).  That, if wanted, would be a separate development.
It should be possible to change the size of the combo-box, use this function:
void gui_combobox_t::set_groesse(koord gr);


Good luck with this patch! Something like this is surely needed.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: VS on February 04, 2009, 02:03:37 PM
So you could also use this component to search for a particular train?


OK, then all Simutrans comboboxes are broken :(
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: prissi on February 05, 2009, 09:44:02 PM
The Comboboxes work fine as they are ... the are broken only that they need specific action of the dialoge to close them ...
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: VS on February 05, 2009, 09:55:53 PM
prissi: Broken (not code) because you never know it is a combo box.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: prissi on February 05, 2009, 10:00:30 PM
That for there are the left/richt arrows ...
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 07, 2009, 11:29:13 PM
Here's an update.  The dialog without functionality yet.  And some bugs.  The dialog will open with a new button in the convoys window.  You can select the convoy that will replace current convoy.  Two buttons below allow to replace all convoys equal to this one in the current line and to replace all convoys equal to this one.

"Replace cycle" allows the player not to replace all but some convoys.  It is very simple.  If you choose 5 in replace and 3 in skip, 5 vehicles are replaced, 3 are left the same, 5 replaced, 3 left, and so on, until all convoys are exhausted.

You can see some bugs: for example convoy max speed, trains are not centered, etc.  It is a WIP.

EDIT: patch erased.  See next version.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on February 07, 2009, 11:34:27 PM
Looking promising so far!
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 08, 2009, 07:19:43 PM
Thanks, James.  In fact, you started the idea and spurred me on...  ;)

An update (I will erase previous patch, to save some space in the server).  In this update:

In the attach picture, you can see a train to be replaced by other.  All trains like this one in its line will be replaced too.  To be replaced trains will first go to depot, be replaced and restarted.  2/3 of trains in the line will be replaced, 1/3 will be sold.

And two questions I don't know how to deal with, if anybody has an idea:


EDIT: Patch erased.  See next version.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on February 08, 2009, 07:25:16 PM
Looking extremely promising! To answer your questions:

(1) in my view, the most user-friendly answer would be for it (a) to alert the user that the track is blocked; and (b) keep trying to reserve a section at an interval of, say, 10,000 ticks (10 seconds on a normal speed), and inform the player that it is doing that, too;

(2) this might be quite difficult; my provisional view is that it is enough for the convoy to say "cannot find route" in the usual way, but perhaps other have other ideas...?
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 09, 2009, 03:33:48 AM
Another version of the patch, first with some useful functionality.  Things are going quicker now:

Next step will be to really replace vehicles when they reach depot and are marked for replacement.  But enough for today.


EDIT: patch erased.  Please see next version.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on February 09, 2009, 09:07:54 AM
Goodness, this is excellent progress! I particularly like "retire". This could really reduce micromanagement!
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 10, 2009, 01:43:03 AM
Not so long crossing, as it seems...  :D

The second part was much easier than I thought.  The attached patch is a working version.  Now vehicles are sent to depot, replaced, and restarted if wanted.  Please consider this beta software.  It needs bug trapping.

It can be used to replace, sell, send to depot one, part or all of convoys belonging to a line.  Other use can be to ask a train to add a wagon without being compulsory to manual unload the vehicle, send it to the depot, add the wagon and restart it.

I hope you guys like it.

By the way, what year does the first bus appear in pak.german?

EDIT: The long waited-for bus arrived, but it was quite expensive.  I must wait for other newer still.  It is a good climber though.

Patch erased to save space.  Please see next version.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 15, 2009, 06:02:29 AM
Some corrected errors:

The patch should be more stable now.  The "electric problem" keeps unsolved though.

Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: gerw on February 15, 2009, 12:12:29 PM
I tried with r2325 and after fixing one failed hunk, it works fine.

One thing: Please replace the inclusion of header files in other headers by forward declarations, whenever it is possible (http://en.wikipedia.org/wiki/Forward_declaration). This will save compile time.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on February 15, 2009, 12:15:59 PM
I am missing replace_frame.h from the latest patch.

Edit: Never mind, found it.

Edit 2: gerw, what did you do to get it to complile? I have a great difficulty with header files, such that it produces thousands of errors. Specifically, grund.h, which includes simdepot.h (which, in turn, includes the new gui_convoi_assember.h) complains that the depot_t class is not defined. I have pored over gui_convoi_assember.h for a very long time and not found any syntax errors, so I am very confused.

Edit 3: I have managed to fix the problem by replacing the line, #include "gui/components/gui_convoy_assembler.h in simdepot.h with a forward declaration to gui_convoy_assembler. I am still not quite sure what caused the original problem, but, since this solution is preferable in any event, it seems satisfactory overall.

Edit 4: An interim idea for dealing with the electrics issue: simply have a button in the replace GUI with the label, "show electrics", which is on by default if the current convoy contains electrics, and off by default if it does not.

Edit 5: Another idea would be to suppress the "entered depot" dialogue box when vehicles enter a depot as a result of the vehicle replacer and they are set to leave the depot again automatically. Also, we need help text for the replace window.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: z9999 on February 15, 2009, 05:00:24 PM
My impression.

- Is it possible to show the number of target convoy to replace on replace window ? It is useful to set the value of replace cycle.
- Is it possible to show sum of cost to replace on replace window ?
- Is it possible to show a message window when all replace was done ?

Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: Fabio on February 15, 2009, 05:01:12 PM
these are great ideas!
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 15, 2009, 05:43:01 PM
Thank you for your answers.

@gerw: I'll try to fix the headers in the final polish.  I'm not good at it, though.  It is confusing for me.

@jamespetts: I'll try to crosscompile a version for windows from linux and post it in the patches repository if I success, for more people to try.  Nice suggestions about the enter depot message.  Regarding the help message window, when finished, I would ask for your help if you don't mind.  A native speaker will surely do better.

@z9999: very nice ideas.  First should be easy.  Second also, but that cost changes as vehicles get older and also there is a difficulty regarding already bought vehicles in depot which need not to be bought again.  Third I believe impossible.  Once marked for replacement, convois loose "contact" with each other.  There had to be a transaction id or something like that and things get much complicated.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: Fabio on February 15, 2009, 06:06:58 PM
Quote from: isidoro on February 15, 2009, 05:43:01 PM
Third I believe impossible.  Once marked for replacement, convois loose "contact" with each other.  There had to be a transaction id or something like that and things get much complicated.
well, i think this could be done monitorizing the line: if you set to replace with 10 new vehicles of type X and 3 xX are already in the line, you could have the feedback when the line contains 13 vehicles X.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: z9999 on February 15, 2009, 07:27:04 PM
Quote from: isidoro on February 15, 2009, 05:43:01 PM
Third I believe impossible.

I think so, too.  :) It's not a big broblem because I can see it in line management window. Thanks.

Another things.

- If I uncheck both "send to depot only" and "send to depot and restart" and click "Replace" button, button label changes to "Replacing" but convoy don't go to depot.
- If I don't select any convoy on replace window and click "Replace" button, button label don't change and convoy go to depot.

I think both of these cases, "Replace" button should be inactive.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on February 15, 2009, 08:57:38 PM
For anyone who wants to try this without having to compile from source, I have integrated it into Simutrans-Experimental, a binary version of which can be downloaded as described in this (http://forum.simutrans.com/index.php?topic=1302.0) thread.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 16, 2009, 04:15:53 AM
@fabio: good point.  I'll see if anything can be done

@z9999: first is intended. "Replacing" in the implementation means something like "marked to be replaced".  The idea is that if you have a very complicated line and you fear that some convoys go to the wrong depots, you uncheck both buttons, but convoys are marked for replace.  Later, when the convoys are near the good depot, you manually press "Go to depot" and they get replaced.  I'll have a look to the second.

@jamespetts: thanks for including in ST-exp.  Read on, though.

I've tried to crosscompile the windows executable from linux with the patch.  The result is in http://simutrans-germany.com/~patches/download.php?file=sim-wingdi-replacing-r2260.zip (http://simutrans-germany.com/~patches/download.php?file=sim-wingdi-replacing-r2260.zip).  I don't know if the process has succeeded since I don't have any Windows machine at home.  If any of you try it, please tell if it worked.

I you want a linux binary, just ask and I'll upload it.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on February 16, 2009, 08:54:20 AM
Isidoro,

my compilation with Windows, albeit the modified version with the forward declaration instead of the header file, works fine.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: prissi on February 16, 2009, 09:31:51 AM
The replace dialog did not even fit the screen in the starting size of simutrans. This should be fixed, since resizing also does not work properly. Also the actions by the buttons are completely incomprehensive.

Even more, it would make more sense to have this as a line tool. As part of convoi it does not really make sense, because a single convoi is easily replace manually (Send to depot => window pop up => replace: same effort as before, schedule is kept)

Also there is a Cancel button, which I do not want to appear. (THe cancel button of the save dialoge is actually a long time candidate to be gone anyway.)
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on February 16, 2009, 11:25:41 AM
Prissi,

it would be a serious mistake to abolish the cancel button on the save dialogue - it is easy to press SHIFT + S by mistake, and it would be silly if, every time that a player does that, he/she is forced to enter a save filename, wait for the game to save, and then have extra disc space taken up. Almost all programmes have cancel options for things of that nature, including saving files. Indeed, I am not aware of a single serious or professional piece of software that does not allow a user to cancel a mistake.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: Combuijs on February 16, 2009, 11:45:27 AM
Well, you can just close the dialogue if you don't want to save anything.

You will usually find the cancel button on a modal dialog. Simutrans does not have a modal dialogue. For instance when the save dialogue is up, you still can change things on the map, and in fact just ignore the dialogue. You can't do that with Microsoft Word!
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: Fabio on February 16, 2009, 12:02:36 PM
cancel buttons are easier to click on than X buttons to close windows, because they are bigger.
what's wrong with them, in your opinion, if i may ask?
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: prissi on February 16, 2009, 12:14:15 PM
The is the delete ley which close the topmost window anyway. It is usually only eating space and attention.

Cancel just do not make sense on a non-modal dialog, since no action started yet.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: VS on February 16, 2009, 12:23:12 PM
True. Since the window can be closed with mouse or keyboard, it makes less sense to have a dedicated button for that.

On the other hand, users may think the dialog must be used... and hitting that small X is hard.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on February 16, 2009, 01:03:56 PM
Very few users will know that the "delete" key closes a window - that is not normal behaviour. If "cancel" is just another button for closing the window, that both makes sense and is easy to use.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: VS on February 16, 2009, 01:14:14 PM
Umm, apart from delete, escape works there, too.

Escape, delete - close topmost window. Backspace - close all windows.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: Fabio on February 16, 2009, 01:52:31 PM
Quote from: VS on February 16, 2009, 12:23:12 PM
On the other hand, users may think the dialog must be used... and hitting that small X is hard.

this is the main point, thinking of newbies...
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: gerw on February 16, 2009, 04:49:10 PM
For me, the buttons "Show all" and "Show obsolete too" won't work either in depot nor in replacing frame.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 17, 2009, 12:57:53 AM
Quote from: prissi on February 16, 2009, 09:31:51 AM
The replace dialog did not even fit the screen in the starting size of simutrans. This should be fixed, since resizing also does not work properly. Also the actions by the buttons are completely incomprehensive.

I don't quite understand this first paragraph, prissi.  Do you mean that the dialog is too big?  In what way it doesn't resize properly?  Making it bigger, smaller?  Please be more specific.  Your last sentences is also incomprehensive for me.  Can you rewrite, please?


Quote from: prissi on February 16, 2009, 09:31:51 AM
Even more, it would make more sense to have this as a line tool. As part of convoi it does not really make sense, because a single convoi is easily replace manually (Send to depot => window pop up => replace: same effort as before, schedule is kept)

I don't agree with this.  If I want to replace a single convoy, I have to only do an action and then forget.  It's all automatic.  With your example, the vehicle can arrive at the depot some time after and even you may just have forgotten what to do with it (two actions separate in time, more micromanagement).   Even for replacing a full line, it is more comfortable this way: click on any of the vehicles of the line, then replace.  The other way, you have to look for the line, then find a suitable vehicle there, then replace.  Not much a difference.


Quote from: prissi on February 16, 2009, 09:31:51 AM
Also there is a Cancel button, which I do not want to appear. (THe cancel button of the save dialoge is actually a long time candidate to be gone anyway.)

The cancel button is easy to cancel.  :)   Regarding its goodness or badness, I have no opinion.  I'm not a GUI expert myself.


@gerw:  thanks.  I would look at it.  It was really a pain to separate the convoy builder functionality from the depot frame.  Some things may have been lost in the way.


EDIT:  new version of the patch with some of your suggestions.  More on the way:
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: prissi on February 17, 2009, 09:17:34 AM
The text on the buttons gives not hint what is funtion actually is. Especially what replace cycle options do is completely incomperhensive, and what is the difference between replace all and replace all in the line? Also there is the info missing how how the current convoi carriers and how long station is needs. Otherwise things will be really messy in the end.

The dialog does not fit the screen if you just start simutrans. The resize and some text is outside the actual screen (Just start pak64 and click on a ship). Resizing will allow to have some text outside of the windows.

The UI desgin is pretty messy imho, with lots of empty space. Maybe just put all text and options concerning replacing right of the main "depot" window.

And the drop down menu should cycle, I think there was an option for that, otherwise I need to add it.

And at least wernimans version never replaced a convoi, since never everybody got off the train.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: robofish on February 17, 2009, 10:20:04 PM
Quote from: prissi on February 17, 2009, 09:17:34 AM
The UI desgin is pretty messy imho, with lots of empty space. Maybe just put all text and options concerning replacing right of the main "depot" window.
I agree with prissi - also placing the convoice centered does not really fit the simutrans gui, where convois are always orientated at the left. I like prissi's idea of an extended depot window very much, but maybe just but both convois (old an new) in one line.

Concerning the line-tool discussion: It's surely faster to have it as a convoi dialog, but it breaks with the way simutrans usually handles this.
On the other hand it wouldn't be very handy as line tool, because there might be different convois on the same line and maybe you want to replace convois on more than one line ...
One always has the problem of selecting the "old" (to be replaced) convoi, if it's not a convoi tool ...

Anyway, very nice addition isidoro!
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 18, 2009, 12:06:04 AM
Quote from: prissi on February 17, 2009, 09:17:34 AM
The text on the buttons gives not hint what is funtion actually is. Especially what replace cycle options do is completely incomperhensive, and what is the difference between replace all and replace all in the line? Also there is the info missing how how the current convoi carriers and how long station is needs. Otherwise things will be really messy in the end.

Prissi, consider that this is a working approximation to test functionality, not a final polished version.  If this is to come into trunk, some refinements should be done, but I need to know in advance.  Otherwise, it is not worth the effort.  I hope you understand.

To the replace cycle, a hint can be to gray it out unless "replace line" or "replace all" is selected.  This, together with a good help window may serve.  What do you think?

"replace all in line" is as incomprehensible as "show all", "all" what?  That's the tooltip for.  In the latter, it says: "Also show vehicles that can't be used for the selected action".  In the former, I wrote "Replace all convoys like this belonging to this line"

You're right.  Some important info is missing, but the problem is that it can clutter things up.  Where to put number of convoys and station length of all convoy?  Under the image?  I'll try it, together with z9999 ideas about cost and number of matching convoys.


Quote from: prissi on February 17, 2009, 09:17:34 AM
The dialog does not fit the screen if you just start simutrans. The resize and some text is outside the actual screen (Just start pak64 and click on a ship). Resizing will allow to have some text outside of the windows.

I now understand.  I didn't check with ships.  I'll try to fix.


Quote from: prissi on February 17, 2009, 09:17:34 AM
The UI desgin is pretty messy imho, with lots of empty space. Maybe just put all text and options concerning replacing right of the main "depot" window.

And the drop down menu should cycle, I think there was an option for that, otherwise I need to add it.

And at least wernimans version never replaced a convoi, since never everybody got off the train.

Yes. Locations should be reworked.  The main problem I face is the info of vehicles.  That empty space in the middle of the window is no good at all.  Ideas?  Perhaps move it to the bottom of the window like in the depot window?  Will also look at the cycle thing.

Did wernimans use the eighth version of the patch or the binary I uploaded to http://simutrans-germany.com/~patches/download.php?file=sim-wingdi-replacing-r2260.zip (http://simutrans-germany.com/~patches/download.php?file=sim-wingdi-replacing-r2260.zip)?  Please confirm.


@robofish: Thanks for the last comment.  I was starting thinking that there was nothing good about the patch...  Regarding the center question.  At first versions the convoy was at the left, but looked most strange...   In one line would be very difficult with present state of convoy assembler.


EDIT: Below is a screen capture with something towards a more depot-like interface.  Vehicle count and speed of replaced convoy in shown.  Three actions buttons in the middle with explanation tooltips.

@prissi:  I've looked at gui_combobox.cc and cycling is not possible in current implementation.  Even now I remember that there is an "empty state" (-1) I had to manually remove in my component:

      } else if(komp == &action_selector) {
         int selection = p.i;
         if ( selection < 0 ) {
            action_selector.set_selection(0);
            selection=0;
         }
[...]


Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: wernieman on February 18, 2009, 07:27:19 AM
Sorry, but I not make a Version with this patch...
(That mean that I not do in the past, not that I don´t want)

Where do you get a version where somebody say, that he is wernieman??
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: robofish on February 18, 2009, 11:13:43 AM
Quote from: isidoro on February 18, 2009, 12:06:04 AM
@robofish: Thanks for the last comment.  I was starting thinking that there was nothing good about the patch...
Well I don't think prissi would be so critical if he wasn't thinking about including it into the trunk - hence take it as compliment :)

[or am i understanding something wrong? :S]
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 18, 2009, 01:56:06 PM
Quote from: wernieman on February 18, 2009, 07:27:19 AM
Sorry, but I not make a Version with this patch...
(That mean that I not do in the past, not that I don´t want)

Where do you get a version where somebody say, that he is wernieman??

I understood that from this prissi's sentence:
Quote from: prissi on February 17, 2009, 09:17:34 AM
And at least wernimans version never replaced a convoi, since never everybody got off the train.


@robofish: thanks


EDIT:  new version of the patch with:

I hope that the semantics now are clearer and the aspect suits better to ST.  Another z9999's idea (cost) is to be done yet.  Where do you think is the best place to put that?

Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 20, 2009, 12:56:03 AM
Final version of the patch with these additions/corrections:

Although I put a crosscompiled binary in patches.simutrans-germany.com, better check with a windows compiled binary.

Prissi, if this patch is decided to go to trunk, please say and I will merge with latest release and build the help window.

James, if you merge with ST-exp and encounter any problem, please tell.


EDIT: Patch erased to save space.  Please see the next version.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: Trukker on February 20, 2009, 11:21:23 AM
Just a small problem. The replace button is half hidden when you display chart. See image attached.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 21, 2009, 03:10:54 AM
Should be corrected in this version.  Thanks.

EDIT: Patch deleted to save space.  Please see the next version.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on February 21, 2009, 08:35:45 PM
Thank you for the latest version, which I have been integrating into Simutrans-Experimental.

I have found a bug, however: when the "replace cycle" setting is reduced to 0, I get a "runtime check failure" no. 2 ("stack around the variable 'n' was corrupted"), which points to the final closing brace of the void replace_frame_t::update_data() method in the replace_frame.cc file.

Edit: Another glitch is that when the convoys enter the depot and are replaced, the cost of selling the original vehicle and the cost of buying the new vehicle are so rapidly produced that the floating numbers generated by each operation obscure each other.

Edit: I have noticed that the other bug, the vehicles not automatically being sent to the depot, only seems to occur with rail vehicles, not with road vehicles. I have not tried it with other types yet.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: HeinBloed on February 21, 2009, 09:08:45 PM
Quote from: jamespetts on February 21, 2009, 08:35:45 PM
Edit: I have noticed that the other bug, the vehicles not automatically being sent to the depot, only seems to occur with rail vehicles, not with road vehicles. I have not tried it with other types yet.

I hope this isn't misleading, but when I observed that bug it was with a bus. But who knows wether that only occurred in that specific version and not anymore since...
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on February 21, 2009, 09:18:37 PM
Tttron,

interesting - I had it consistently with rail vehicles, but was consistently able to get road vehicles to work. The next thing to work out is whether this is an issue with the original patch or Simutrans-Experimental...

Edit: Incidentally, a further bug: whenever a replacement operation is completed, it gives the message, "all convoys in line [liNE] replaced", even when one has opted to replace only one convoy or all convoys of one type, rather than all in a line.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 21, 2009, 09:38:52 PM
James, nice catch!  The zero bug should be corrected with this version.

The money issue is not printed by the patch.  The depot's buying routine produces it.  It happens the same as when in FF mode.

Regarding the last one, can you be more specific and/or attach a savegame with instructions so that I can reproduce?


EDIT:  Again, the message is not a bug.  Whenever the last replaced vehicle of a line is replaced, the message appear.  If not wanted, a flag can be added to the convoy, but I think that rewriting the sentence can be a better option.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on February 21, 2009, 09:56:17 PM
Isidoro,

thank you for the patch, but I am afraid that it does not fix the problem: the previous behaviour recurs.

Edit: Incidentally, I have modified the text to read, "Replacing vehicles of line (LINE) completed", to avoid confusion.

Edit: Oops - I was getting confused, thinking that you had fixed a different bug. The patch does correctly solve the zero bug. I have uploaded a saved game that reproduces the other two problems here: http://simutrans-germany.com/files/upload/replacing-test.sve . I appreciate that the issue with the text is caused by the depot algorithm, but the reason that the two pieces of text appear so close together is that your replacing algorithm purchases/sells multiple things in quick succession. It would be helpful to modify the code to show the net cost of the entire transaction on a replace cycle.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on February 21, 2009, 11:24:41 PM
I believed the zero bug should have been already corrected...  :police:

Regarding then money messages, that's the way ST shows when buying/selling individual vehicles.  The same effect happens when you click in the "Sell mode" button.  It can be changed, but it is not the objective of the patch.

With this (12+1)th version of the patch, the empty vehicle replacement should have been corrected.  Incidentally it's is not a direct bug of the patch.  Try, for example, to hit the "withdraw" button of a convoy with one locomotive only: the same behavior.  It happens because a convoy with zero capacity is given a 100% loading level.  That's reasonable since those convoys shouldn't stop in a station if the line is marked to wait till fully loaded there.

If the patch finally doesn't come to trunk, this should nevertheless be corrected there, imho.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on February 22, 2009, 12:22:17 AM
Aha, excellent - it works! Thank you.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: gerw on May 06, 2009, 09:12:43 PM
I've updated this patch to the latest revision. I haven't done further checks, but it compiles fine.

Edit: patch removed. New patches attached to later posts.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on May 06, 2009, 11:44:09 PM
Thanks, gerw.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: gerw on May 09, 2009, 04:47:54 PM
New version of the patch. Now convoi_t::goto_depot uses the possibility of route_t::find_route, to find the next depot. It haven't loop over all depots anymore.

Edit: Other new version: Now the sizes of the GUI frames are calculated correctly (and fit on the initial screen size).

Edit: patch removed. New patches attached to later posts.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: prissi on May 09, 2009, 07:52:56 PM
Unfourtunately find_route fails for ships and airplanes ...
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on May 09, 2009, 08:23:46 PM
Prissi,

that's odd - my tests haven't found that. find_route works find for ships. It doesn't work for aircraft that are flying, but does work for aircraft on the taxiway or runway. The aircraft issue is not a problem for the replacing system, since the command to go to the depot is not issued until the aircraft is on the ground in any event.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: prissi on May 09, 2009, 08:28:25 PM
Find_route for ships has often problem with far away depot on large maps, since the tile buffer is full before a depot is reached.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on May 09, 2009, 08:39:10 PM
Ahh, hmm, I didn't use a large map. Perhaps the old iteration system should be used if find_route fails? What's the best way of checking whether find_route has failed?

Edit: Right, I have re-written it such that it falls back to the old system if it doesn't find a route using the new system. This will be in the next release of Simutrans-Experimental.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: gerw on May 10, 2009, 08:00:04 AM
Thank you prissi. Now it uses again the old depot finding algorithm for planes and ships.

Edit: New patch. Now, convoy_assembler isn't a pointer anymore. And if a depot is electrified, a little bolt is presented.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: gerw on May 14, 2009, 01:58:13 PM
Quote from: prissi on May 14, 2009, 12:59:12 PM
THe withdraw all is on my todolist, since the replacing faces imho too many unresolvable problem with different vehicle length and pak sets.
With manual replacement, you will have the same problem with different vehicle length, won't you? And what do you mean with "pak sets"?

One real problem is the electrification...
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: Dwachs on May 14, 2009, 02:41:11 PM
if you switch to another pak set maybe?  ;D
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: Gryphonite on July 04, 2010, 11:00:52 PM
I couldn't get the most recent version to do anything in Tortoise SVN (didn't get the small window), the previous one in this thread patched though. Tortoise threw up some errors as well when I applied the patch, though this is my first time of doing anything like this so I'm probably going wrong somewhere.


More importantly... :)
I know this (or at least something similar) is now in experimental, any chance of it making it into a nightly of standard?
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on July 04, 2010, 11:44:03 PM
Gryphonite,

the reason that it won't merge is because there have been vast changes to the code since this patch was originally written. As you note, there is a version of this feature in Experimental (which is somewhat of an enhancement of this version, as it has additional features). I am not aware of any plans to put this into Standard, but I cannot speak for the Standard developers on the subject.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on July 05, 2010, 12:06:47 AM
Quote from: Gryphonite on July 04, 2010, 11:00:52 PM
I couldn't get the most recent version to do anything in Tortoise SVN (didn't get the small window),
[...]

Patches record changes in the source code of the program.  They include the new lines/changes as well as the context (surrounding lines) where those changes are to be applied.

If the code has been changed a lot since the patch was generated, the patch program will not automatically find the suitable context and hence the errors.

Another possibility you have is to apply the patch to the revision from which it was generated (r2456).  But note that the game will not have the improvements made since then, savegames may not be compatible (opened) by an up-to-date version, etc.

Quote from: Gryphonite on July 04, 2010, 11:00:52 PM
...any chance of it making it into a nightly of standard?

That's not an easy task.  Code may have changed a lot in current version.  Not incorporated patches are very difficult to maintain.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: Gryphonite on July 05, 2010, 05:57:00 PM
Thanks for the reply. I have about zero experience in programming, but see where you are coming from. Would be really useful to have though... perhaps I need to do some learning :)
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: prissi on April 07, 2012, 08:57:15 PM
As most frequently requested feature, I think a modified try in this might be useful. Maybe exclusively as called by the line tool, with indication of total capacity and length of old convois and replacing all of them as only option. Like an line inherent depot actually ...
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on April 07, 2012, 09:59:03 PM
I'd be very glad if an improved version is incorporated in Standard.

Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on April 08, 2012, 01:17:12 PM
If this is to be incorporated in Standard, might I suggest that some changes are made based on experience in Experimental? I am planning to make a few changes to this in any event along these lines, and this might be an opportunity for standardisation (no pun intended) across the two branches. In summary, the suggested changes are as follows:

(1) do not use a series of variables in convoy_t to store the data: add a new replace_datum_t object, and define a pointer to it in convoi_t, which, when NULL, indicates that no replacing is taking place; this will save memory (this is already implemented in Experimental);

(2) allow a current replacing instruction to be cleared before it is executed (this is also already implemented in Experimental);

(3) do not require convoys to go to the depot to be replaced - experience in Experimental shows that this causes large problems with congestion - instead, replace the convoy in situ once it is empty;

(4) allow vehicles in the convoy to be replaced by vehicles (including vehicles now out of production, even when the option to allow buying obsolete vehicles is disabled) from existing depots (this allows for proper cascading); and

(5) connected to the above, make all vehicles available in any depot of any player available from all depots, so that there is in effect a single stock of stored vehicles for each player from which either the replacer or a depot can assemble convoys.

Experimental also plans to implement a secondhand market in vehicles, which will be useful for online play. I am not sure whether this would be something that the Standard developers would like to do, but if so, then it might be worthwhile considering implementing the architecture for that alongside the above suggested changes.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: prissi on April 08, 2012, 06:27:45 PM
It will be probably new code in the line window, like two bottons "Replace all convois" which will set the retire flag at all convois and will assign the new convois to this line from a standard depot dialoge of the depot of most of the convois. The other is add new convois, and will add convois to a diven line using the depot dialogue.

The only difference will be, that a the top of the dialogue the old and new capacity of the line will be shown. That way the depot help an most of the stuff could be reused without making the dialogue much bigger.

It will not work on lineless convois and must be triggered per line.

At least these are my though on this. (Maybe the line information will help even current depots; in that case just a standard depot dialogue is needed.)
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: isidoro on April 08, 2012, 10:48:32 PM
This is a quote from another thread, but I think the answer fits better here:
Quote from: dannyman on April 08, 2012, 07:37:22 PM
[...]
All this is to say: I love the convoy replacing tool, and wish I could use it for adding a copy of a complex convoy to a line, without having to manually send someone to the depot.

The classic Copy to clipboard and Paste from clipboard would be handy here.  The copied convoy would be copied with all its information, including schedule/line...  The Paste from clipboard button would be grayed when there is nothing in the clipboard, as usual.

The present Copy button in the depot dialog should be renamed to Replicate or Duplicate, imho.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: Fabio on April 11, 2012, 11:26:51 AM
Nice! I'm glad this is coming to light after all this time.

A few thoughts:
If some of the convoys in the line already match the new pattern, leave them as they are. An option to only replace obsolete would be handy too.
if the new pattern has some cars in common with the old one (e.g. It only changes the loco), keep those, don't sell and re-buy them.
A second hand market would be nice indeed. all withdrawn cars are sent to a market virtual depot. In player's depot the cars have these numbers:
green: there are n cars in the current depot, you can use them freely.
Yellow: there are n cars in other depots belonging to the same player. you can use them, paying a small transport fee (e.g. 1% of the current value)
Red: there are cars in the market depot. You can buy them at their current value + 1% fee.
No number: no cars, you buy brand new ones.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: neroden on May 13, 2012, 11:29:34 PM
Quote from: jamespetts on April 08, 2012, 01:17:12 PM
(3) do not require convoys to go to the depot to be replaced - experience in Experimental shows that this causes large problems with congestion - instead, replace the convoy in situ once it is empty;

I actually prefer to force them to go to the depot even though it causes problems with congestion; replacing vehicles mid-journey feels way too weird to me.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on May 13, 2012, 11:33:05 PM
Hmm, I think that playability needs to outweigh lack of weirdness on this one...
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: Ters on May 14, 2012, 05:09:56 AM
Congestion when replacing parts of a vehicle is bothersome, but I would hate to have to resort to magic to avoid it.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on May 14, 2012, 12:40:29 PM
I'd be hesitant to say that there is anything fundamentally inconsistent at a gameplay level about replacing without going to a depot: after all, vehicles do not have to go to depots for routine maintenance.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: ӔO on May 14, 2012, 02:04:58 PM
There are a lot of operational details that don't exist in simutrans. I prefer that the convoys just replace themselves when they reach their next stop.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: jamespetts on May 14, 2012, 02:16:04 PM
They'd have to be empty first, however.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: Ters on May 14, 2012, 05:40:38 PM
Then what's the point of depots anyway? Vehicles get maintained without visiting a depot, they can get modified without visiting a depot, they can be sold/scrapped without visiting a depot. Depots would then only be there in order to provide a door for the vehicles to enter the world, but they could just as well appear at the first waypoint. The depot window would become disconnected from any "physical" structures. If one wanted to store a convoy for potential later use rather than sell it or scrap it, the game would just teleport it to the limbo world of the players depot window once emptied. From there it can be brought back just like a new convoy.
Title: Re: [patch] Replacing (aka the long, long crossing of the desert)
Post by: prissi on May 14, 2012, 08:54:29 PM
Depots should be designed in such a way that new convoys can add through them to the world. The replacing patch would set the withdrawal bit on all convois of the line and just add the new ones.