News:

Simutrans Tools
Know our tools that can help you to create add-ons, install and customize Simutrans.

[patch] Replacing (aka the long, long crossing of the desert)

Started by isidoro, February 04, 2009, 02:07:55 AM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

jamespetts

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.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

VS

Umm, apart from delete, escape works there, too.

Escape, delete - close topmost window. Backspace - close all windows.

My projects... Tools for messing with Simutrans graphics. Graphic archive - templates and some other stuff for painters. Development logs for most recent information on what is going on. And of course pak128!

Fabio

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...

gerw

For me, the buttons "Show all" and "Show obsolete too" won't work either in depot nor in replacing frame.

isidoro

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:

  • jamespetts: the message telling the convoy has reached a depot is not shown if autorestart
  • z9999 and fabio: when last convoy to be replaced in a line is replaced, a message is shown
  • gerw: now show obsolete and show all hopefully work again
  • prissi: calcel button has vanished
  • isidoro: if convoy is already empty, no need to go to next station.  Go directly to depot

prissi

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.

robofish

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!

isidoro

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?  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;
         }
[...]



wernieman

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 hope you understand my English

robofish

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]

isidoro

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:

  • prissi: number of vehicles and station tiles in replaced convoy
  • new distribution of the dialog (more depot-like): see images
  • prissi: better resize
  • replace cycle is gray if not to be used (not in all or in line modes)
  • z9999: number of replacements to be done on each category (on the right of replace, sell, skip input elements)

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?


isidoro

Final version of the patch with these additions/corrections:

  • z9999: cost of replacement now shown
  • z9999: replacing convoy void no longer activates no load

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.

Trukker

Just a small problem. The replace button is half hidden when you display chart. See image attached.

isidoro

Should be corrected in this version.  Thanks.

EDIT: Patch deleted to save space.  Please see the next version.

jamespetts

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.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

HeinBloed

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...
(previously known as "tttron")

jamespetts

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.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

isidoro

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.

jamespetts

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.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

isidoro

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.

jamespetts

Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

gerw

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.


gerw

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.

prissi

Unfourtunately find_route fails for ships and airplanes ...

jamespetts

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.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

prissi

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.

jamespetts

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.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

gerw

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.

gerw

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...

Dwachs

Parsley, sage, rosemary, and maggikraut.

Gryphonite

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?

jamespetts

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.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

isidoro

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.

Gryphonite

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 :)