News:

Simutrans Wiki Manual
The official on-line manual for Simutrans. Read and contribute.

Viewing curve speed limits and calculating acceleration time/distance

Started by dannyliux, January 01, 2020, 10:27:56 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

Qayyum

This chart shoes the behaviour of convoy when moving forward constantly from start of journey to one minute after, assuming no junctions.
To RanRan: Let me see...
This chart shows constant acceleration of convoy in one minute after departing. One minute in-game.
My content is Public Domain.

Simutrans - the open source Transport Tycoon Deluxe clone.

Mariculous

QuoteThe real question is whether the extra space taken in the convoy overview window with charts is worth the extra data: I should be interested in players' views on that question.
Generally, I think such a graph (velocity/time graph if I read it correctly) could be worth the space and we could hide the charts, just as the vehicle window does, so people with small resolutions are not too much affected by scrollbars or whatever.

However, what makes me unsure if it's worth is the fixed 1s x-axis, as I suspect that short time is not sufficient to compare characteristics of long-distance trains, which need longer to accelerate, especially for older steam or diesel trains.
Would it require much more work to add meaningful labels to the x-axis and scaling the x-axis by a calculated value?
It would be much more useful to display the graph up to a point where the acceleration becomes reasonably small (hardcoded threshold)

Additionally, I would suggest adding an acceleration/velocity diagram, as it was considered useful in the discussion and we get it nearly for free. The only thing we have to do is turning the values obtained by get_force_summary divided by mass into a graph.

Qayyum

Jamespetts, I think this feature is useful. I would use to determine how many wagons can I have at maximum, referring to the condition of route itself.
My content is Public Domain.

Simutrans - the open source Transport Tycoon Deluxe clone.

Mariculous

QuoteI don't think a(or F)-t graphs are useful;
I fear, I can't follow this. You are talking about an a/t or F/t diagram, while I suggested an a/v diagram.
The implemented f/v diagram is just the same as the suggested a/v diagram, apart from a constant factor, which is effectively a y-scaling, so I'm totally fine with that implementation.
Additionally, adding the running resistance to the graph is a good idea in my opinion.


QuoteHowever, varying the x-axis has some complexities and issues. X-axis scaling, axis labels do not look good etc.
That given, I would stick with the fixed 120s for now and change it later on if we encounter this to be insufficient for some vehicles.

jamespetts

Thank you for your work on this, and my apologies for not having had time to look into this hitherto - I have quite a long queue of things that I am slowly trying to get through as and when I have time.

Thank you for investigating the refunds issue. That does appear to be a bug on the face of it, although I have not had chance to look into that reproduction case at this stage.

One query is whether the acceleration button and refunds button conflict when the refunds button is displayed; perhaps you could look into that? That would be most helpful.
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.

Matthew

I tried to build Ranran's dig-the-acceleration-curve branch to test this new feature. It compiles and runs, but I can't see the Acceleration and Tractive Force buttons in the convoy window. Is this expected? I understand this is work-in-progress and maybe some parts are not on Github yet.  8)

Also, the convoy window had some new bad behaviour. If I followed a convoy and then moved the convoy window, then only the menu bar of the convoy window was visible. The other parts (data, charts, etc.) disappeared.

I can do full steps-to-reproduce for both points if you want.

P.S. The most likely reason for the problem is that I have made a mistake with Git or in Simutrans, of course!
(Signature being tested) If you enjoy playing Simutrans, then you might also enjoy watching Japan Railway Journal
Available in English and simplified Chinese
如果您喜欢玩Simutrans的话,那么说不定就想看《日本铁路之旅》(英语也有简体中文字幕)。

Matthew

Quote from: Ranran on January 19, 2020, 01:10:06 PM
Oh sorry, I think I have fixed it. It's my mistake.  :-[
Thanks for testing and reporting.  :D

Thank you for replying so quickly! I tried compiling your dig-acceleration-curve-chart branch and testing it. But it crashed two times.

Steps to reproduce:
1. Open Simutrans-Extended and make a new map.
2. Build stables (horse depot) and two stops.
3. Open the stables and build a convoy between the stops.
4. Start the convoy and then open the convoy window.
With the normal version, it crashed here. I compiled a debug version and got a little further:
5. Click on 'Tractive Force'.
The debug version crashed here.

The simu.log file from the debug version is here. The Linux terminal had an extra error message:
*** stack smashing detected ***: <unknown> terminated
Aborted (core dumped)


I don't know where the core dump is placed. If it's helpful then I can upload it.

By the way I enjoy playing with the livery patch.  :)
(Signature being tested) If you enjoy playing Simutrans, then you might also enjoy watching Japan Railway Journal
Available in English and simplified Chinese
如果您喜欢玩Simutrans的话,那么说不定就想看《日本铁路之旅》(英语也有简体中文字幕)。

Matthew

Quote from: Ranran on January 19, 2020, 04:32:58 PM
I downloaded it from my repository and compiled it, but I couldn't reproduce the crash.

Ranran, thank you for the quick reply. I checked again that I was up to date with your dig-acceleration-curve-chart, changed back to the normal (not debug) config, deleted my /build folder, and recompiled. I still get a crash after the same steps to reproduce. I have tried pak128.Britain-Ex in 1750 with a post boy like your screenshot. I also tried pak128.Sweden-Ex, in case I had somehow corrupted my Britain-Ex. Every time I got a crash when I tried to open the convoy window.

The build number is #3919923.

QuoteDo other self-built versions crash?

This is a very good question. I am a newbie and it's very possible that I am making a mistake. Thank you for your patience.

Other self-built versions do not crash in this way, though. I compiled James' master branch up to last night (20-01-19 0500 GMT) and my build runs OK. My build had the same version number as last night's Bridgewater-Brunel build (#b4e7900). I am building with G++ on Linux. I have attached my config.default, in case there is a mistake or difference there (I added .txt so that the forum accepts it).
(Signature being tested) If you enjoy playing Simutrans, then you might also enjoy watching Japan Railway Journal
Available in English and simplified Chinese
如果您喜欢玩Simutrans的话,那么说不定就想看《日本铁路之旅》(英语也有简体中文字幕)。

Qayyum

Matthew: Try download this to your computer where you run your game.
My content is Public Domain.

Simutrans - the open source Transport Tycoon Deluxe clone.

Mariculous

Quote from: Ranran on January 20, 2020, 09:30:02 AMI would be grateful if anyone could give advice on this matter.
It will be easyer to have a look at it if you told us your observations. To be preciese: Where in the physics code do you expect it to crash?

jamespetts

Ranran - thank you for your splendid work on this, and apologies for not having been able to look into this earlier. I have now had a chance to test this. I have not noticed any crashes when using this.

Ultimately, this is a useful feature well presented and it would be good to be able to incorporate this. However, there are one or two anomalies at present that need at least some further consideration.

First of all, the acceleration curve seems to be capped at the current way's speed limit, so, for aircraft on a taxiway, for instance, it gives acceleration only up to 32km/h: one has to wait until an aircraft is flying before one can see real acceleration. The same would presumably apply to a train in a depot with a low speed track.

Secondly, the curve seems to change over time for reasons that are not entirely clear, indicating a higher top speed as the vehicle accelerates. This may be an anomaly in the physics engine.

Thirdly, the tractive green rolling resistance (the translation texts should be altered from "running" to "rolling" resistance) is hard to understand and is almost always so small a number that the graph is unintelligible next to the much larger tractive effort figure.

Fourthly, "tractive force" should be renamed "tractive effort" for consistency with other uses.

Finally, for appearance, it would be preferable if the usually hidden refunds button could be moved to the right of the two always on acceleration/tractive effort buttons so as not to leave a gap in normal circumstances. This is a minor enough issue, however, that it can be left if the workload for this would be excessive.

Incidentally, do I recall correctly that you reported a bug relating to the refunds graph not appearing? If so, I should be grateful if you could start a separate thread for that with a reproduction case, or else I will never remember to try to fix it.

Thank you again for all your work on this: this is much appreciated, and apologies again for the delay in testing this.
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.

Mariculous

Quote from: jamespetts on January 21, 2020, 12:30:30 AMThirdly, the tractive green rolling resistance (the translation texts should be altered from "running" to "rolling" resistance) is hard to understand and is almost always so small a number that the graph is unintelligible next to the much larger tractive effort figure.
The also quite common alternative to this would be subtracting the rolling resistance from the force, but to be honest I prefer the two different curves. This will clearly show when a quite fast vehicle is mostly restricted by air resistance rather than tractive force decrease.
In the end this may not be as important to at least most vehicles as I am not aware of any simuvehicle that has a streamlined version as an upgrade. However, maybe there are some.

Matthew

Quote from: Ranran on January 20, 2020, 09:30:02 AM
Matthew - Thank you for the report. Perhaps those files are useless for the cause of the crash in this case, but the information that it does not crash on other versions was a hint. So I came up with one of the possibilities and made a fix for it.
Well I guess it was caused by my lack of knowledge - my apology.

No apology is needed! You have made a patch and have been very helpful in helping me to use it. Thank you!

QuoteI suppose Bernd's acceleration chart code was originally coded to not be used under certain circumstances. So I've changed the code that would have to be skipped if it wasn't supported. (I removed it because I wanted to see the buried chart.  :-[) So I fixed it.
If it's fixed correctly, you won't be able to use this chart instead of causing the crash in your build.

I can see that you have used ifdef to try to avoid the problem. This is a very good idea. Unfortunately, the problem remains. When I open the convoy window, I get a crash. However, I now get a debug message in the terminal:  :)

Build #f33ad74 (today's dig-acceleration-curve-chart branch)
Debug: convoi::dump():
vehicle_count = 4
wait_lock = 0
owner_n = 0
akt_speed = 153
sp_soll = 1704
state = 6
statename = DRIVING
alte_direction = 0
jahresgewinn = 0
name = '(1) Friesian horses (pair)'
line_id = '0'
schedule = '0x5572b4b89920'
Segmentation fault (core dumped)


Not very helpful.  ??? But that enabled me to find this in an old crash log:

Build #3919923 (dig-acceleration-curve-chart branch on 19 January in debug version)
Debug: convoi::dump():
vehicle_count = 2
wait_lock = 0
owner_n = 0
akt_speed = 153
sp_soll = 114
state = 6
statename = DRIVING
alte_direction = 0
jahresgewinn = 0
name = '(1) Friesian horses (pair)'
line_id = '0'
schedule = '0x55612655fcd0'
Message: gui_textarea_t::recalc_size(): reset size to 11,0
Message: event: 0,-16


This might not be related to the crash (it was not the last message in the log). But it is maybe related. Unfortunately, gui_textarea_t::recalc_size() is deep inside the GUI code. There seem to be many calls between this method and your (Ranran's) patch. I think the next step is for me to carefully trace how this method is called in the patch. I think this is too difficult for me now because I am still learning C++. Maybe I will come back to this when I have more knowledge. But I should try some easier problems first.

QuoteI don't know what OS, compiler, lack of libraries, what makes it unavailable on your computer because I am not familiar with it. I would be grateful if anyone could give advice on this matter.

Quote from: jamespetts on January 21, 2020, 12:30:30 AMI have now had a chance to test this. I have not noticed any crashes when using this.

On this point: Ranran and James, are you both using Visual Studio Community on Windows to build this branch? And are you compiling #f33ad74, please? Maybe I am just making a silly mistake like building the wrong patch!

QuoteThirdly, the tractive green rolling resistance (the translation texts should be altered from "running" to "rolling" resistance) is hard to understand and is almost always so small a number that the graph is unintelligible next to the much larger tractive effort figure.

Fourthly, "tractive force" should be renamed "tractive effort" for consistency with other uses.

Ranran already started a thread about proof-reading. Shall we continue that discussion there?

QuoteThank you again for all your work on this: this is much appreciated

It is appreciated here too. I hope my problems don't distract you both from incorporating the patch into James' 'official' build.

Quote from: Qayyum on January 20, 2020, 06:59:40 AM
Matthew: Try download this to your computer where you run your game.

Thank you for your suggestion, Qayyum! As you said earlier, this feature would be a good way to determine how many wagons we can use.
(Signature being tested) If you enjoy playing Simutrans, then you might also enjoy watching Japan Railway Journal
Available in English and simplified Chinese
如果您喜欢玩Simutrans的话,那么说不定就想看《日本铁路之旅》(英语也有简体中文字幕)。

Matthew

Quote from: Ranran on January 30, 2020, 10:30:42 AM
I may have fixed the cause of the crash on linux, so it would be helpful if you could check.

Thank you for your work on this, Ranran.

Unfortunately, I still get a crash at the same point : opening the convoy window, on a new map with Pak128.Britain-Ex. Here is the backtrace information from GDB:

Debug: convoi::dump():
vehicle_count = 3
wait_lock = 0
owner_n = 0
akt_speed = 153
sp_soll = 3661
state = 6
statename = DRIVING
alte_direction = 4
jahresgewinn = 0
name = '(1) Hackney horses (pair)'
line_id = '0'
schedule = '0x5555561e4810'

Thread 1 "simutrans-exten" received signal SIGSEGV, Segmentation fault.
0x00005555556a3398 in button_t::init(button_t::type, char const*, scr_coord, scr_size) ()
(gdb) backtrace
#0  0x00005555556a3398 in button_t::init(button_t::type, char const*, scr_coord, scr_size) ()
#1  0x00005555556cfa95 in convoi_info_t::convoi_info_t(quickstone_tpl<convoi_t>) ()
#2  0x0000555555870df6 in convoi_t::show_info() ()
#3  0x00005555558f3a73 in tool_query_t::work(player_t*, koord3d) ()
#4  0x0000555555932693 in karte_t::call_work(tool_t*, player_t*, koord3d, bool&) ()
#5  0x00005555558c4457 in interaction_t::interactive_event(event_t const&) ()
#6  0x00005555558c4561 in interaction_t::process_event(event_t&) ()
#7  0x00005555558c4c1b in interaction_t::check_events() ()
#8  0x000055555594de54 in karte_t::interactive(unsigned int) ()
#9  0x00005555558d3864 in simu_main(int, char**) ()
#10 0x00005555558e93f2 in sysmain(int, char**) ()
#11 0x00007ffff6748b97 in __libc_start_main (main=0x5555555aaab0 <main>,
    argc=5, argv=0x7fffffffdfd8, init=<optimised out>, fini=<optimised out>,
    rtld_fini=<optimised out>, stack_end=0x7fffffffdfc8)
    at ../csu/libc-start.c:310
#12 0x00005555555aab1a in _start ()


Unfortunately, it seems to be a problem with buttons, your old enemy from the station window.  :-[




(Signature being tested) If you enjoy playing Simutrans, then you might also enjoy watching Japan Railway Journal
Available in English and simplified Chinese
如果您喜欢玩Simutrans的话,那么说不定就想看《日本铁路之旅》(英语也有简体中文字幕)。

Matthew

Quote from: Ranran on January 30, 2020, 09:04:25 PM
Matthew - Thank you for testing and sorry for wasting your time.

You are only wasting my time because you are increasing that chance that I enjoy Simutrans more and play more! Is playing Simutrans wasting time??? Actually, I am worried that I am wasting your time, because it's very possible that these crashes are caused by a mistake on my part. I am still new. Maybe I am doing something wrong.

QuoteI suppose I have identified the cause and fix. Check it out when you have time and it will help.

Thank you for trying again. That fix does not seem to have been successful, but we have some more data. Running a build including #c955a9f04, I still get a crash at the same point. Here is the simu.log:

Debug: interaction_t::process_event: calling interactive_event
Message: interaction_t::interactive_event(event_t &ev): calling a tool
Message: tool_query_t(): checking map square 156,17,0
Message: tool_query_t(): index 2
Debug: convoi::dump():
vehicle_count = 3
wait_lock = 0
owner_n = 0
akt_speed = 153
sp_soll = 1072
state = 6
statename = DRIVING
alte_direction = 2
jahresgewinn = 0
name = '(1) Hackney horses (pair)'
line_id = '0'
schedule = '0x555570eb4400'
Message: gui_textarea_t::recalc_size(): reset size to 11,0


And the GDB backtrace:

Message: gui_textarea_t::recalc_size(): reset size to 11,0

Thread 1 "simutrans-exten" received signal SIGSEGV, Segmentation fault.
0x00005555556bd2b3 in button_t::set_typ (this=0x5555579fbe38, t=button_t::box_state) at gui/components/gui_button.cc:127
127 set_size( scr_size(gui_theme_t::gui_button_size.w, max(D_BUTTON_HEIGHT,LINESPACE)) );
(gdb) backtrace
#0  0x00005555556bd2b3 in button_t::set_typ (this=0x5555579fbe38, t=button_t::box_state) at gui/components/gui_button.cc:127
#1  0x00005555556bcf5e in button_t::init (this=0x5555579fbe38, type_par=button_t::box_state,
    text_par=0x555555a2c720 "Sends the convoi to the last depot it departed from!", pos_par=..., size_par=...)
    at gui/components/gui_button.cc:74
#2  0x00005555556e79b5 in convoi_info_t::convoi_info_t(quickstone_tpl<convoi_t>) ()
#3  0x00005555558bb4c6 in convoi_t::show_info() ()
#4  0x00005555559c5a41 in vehicle_t::show_info (this=0x5555a5463220) at vehicle/simvehicle.cc:2962
#5  0x000055555593d436 in tool_query_t::work (this=0x55555793eea0, player=0x555598aaafc0, pos=...) at simtool.cc:384
#6  0x000055555599668a in karte_t::call_work (this=0x55555a38a580, tool=0x55555793eea0, player=0x555598aaafc0, pos=...,
    suspended=@0x7fffffffb27a: false) at simworld.cc:10155
#7  0x0000555555918785 in interaction_t::interactive_event (this=0x55557fc5bf40, ev=...) at siminteraction.cc:240
#8  0x000055555591918a in interaction_t::process_event (this=0x55557fc5bf40, ev=...) at siminteraction.cc:417
#9  0x00005555559192b1 in interaction_t::check_events (this=0x55557fc5bf40) at siminteraction.cc:439
#10 0x0000555555997eab in karte_t::interactive (this=0x55555a38a580, quit_month=2147483647) at simworld.cc:10464
#11 0x0000555555926d88 in simu_main (argc=5, argv=0x7fffffffdfd8) at simmain.cc:1382
#12 0x000055555593aebc in sysmain (argc=5, argv=0x7fffffffdfd8) at simsys.cc:825
#13 0x0000555555a14f32 in main (argc=5, argv=0x7fffffffdfd8) at simsys_s2.cc:792


Earlier, I ran the debug build of the previous version (not including #c955a9f04) in GDB. It is more successful. I can open the convoy window!  :) I can also see the lovely new acceleration chart. However, I get a crash when I open the tractive effort chart. The terminal report is this:

Debug: main_view_t::display: starting ...
Debug: main_view_t::display: display pointer
*** stack smashing detected ***: <unknown> terminated

Thread 1 "simutrans-exten" received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
51 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.


Here is the backtrace for another crash with that debug build:

Message: interaction_t::interactive_event(event_t &ev): calling a tool
Message: tool_query_t(): checking map square 79,173,0
Message: tool_query_t(): index 3
Message: gui_textarea_t::recalc_size(): reset size to 11,0
Message: event: 0,-16
Message: gui_textarea_t::recalc_size(): reset size to 146,11
*** stack smashing detected ***: <unknown> terminated

Thread 1 "simutrans-exten" received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
51 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) backtrace
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00007ffff6767801 in __GI_abort () at abort.c:79
#2  0x00007ffff67b0897 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7ffff68dd988 "*** %s ***: %s terminated\n")
    at ../sysdeps/posix/libc_fatal.c:181
#3  0x00007ffff685bcd1 in __GI___fortify_fail_abort (need_backtrace=need_backtrace@entry=false,
    msg=msg@entry=0x7ffff68dd966 "stack smashing detected") at fortify_fail.c:33
#4  0x00007ffff685bc92 in __stack_chk_fail () at stack_chk_fail.c:29
#5  0x00005555559b98df in number_to_string (p=0x7fffffffacf6 "", f=0, decimals=180) at utils/simstring.cc:208
#6  0x3030303030303030 in ?? ()
#7  0x3030303030303030 in ?? ()
#8  0x3030303030303030 in ?? ()
#9  0x0000303030303030 in ?? ()
#10 0x0000000100426284 in ?? ()
#11 0x0235023b0243e500 in ?? ()
#12 0x40f4e5e15569000e in ?? ()
#13 0xffffffff0000023b in ?? ()
#14 0xffffffff00000019 in ?? ()
#15 0x0000025000000000 in ?? ()
#16 0x0000004e00000000 in ?? ()
#17 0x0000000000000064 in ?? ()
#18 0x00005555a4a1a308 in ?? ()
#19 0x0000000000000000 in ?? ()


Hopefully I will be able to test the debug build of the new commit at the weekend.
(Signature being tested) If you enjoy playing Simutrans, then you might also enjoy watching Japan Railway Journal
Available in English and simplified Chinese
如果您喜欢玩Simutrans的话,那么说不定就想看《日本铁路之旅》(英语也有简体中文字幕)。

Mariculous

Quote from: Ranran on January 31, 2020, 11:33:30 AMI have added a theoretical high to the acceleration chart. This represents an acceleration graph with no load and unaffected by speed limits and slopes.
That's awesome, however imho the theoretical max load acceleration rather than min load is the most important figure for comparisation.

Quote from: Ranran on January 31, 2020, 11:33:30 AMI changed the "Access charges" chart button to violet because orange has some of duplication.
Maybe we should use the same color for any stat of the same "type", i.e. ones that can technically be displayed at the same time and where it makes sense to do so.
I guess the same principle is currently already used for the minimap options.

In this case it would be some thing like (discussable):
Total Capacity, Transported
Average Speed
Comfort
Revenue, Running Costs, Profit, Access charges
Distance
Acceleration
Tractive Effort

Mariculous

Indeed the color is important to the graph in a different way than it is to the minimap, so we may want to set a border color to any selected button that has the same color as the curve in the graph, whilst the main color of the buttons is the same for any button of the same group of compatible informations.

e.g. main colors
Total Capacity, Transported: Orange
Revenue, Running Costs, Profit, Access charges: Blue
Average Speed, Comfort, Distance, Acceleration, Tractive Effort: unique colors or all grey

Borders (on selection):
Revenue: Green
Running Costs: Red
Profit: Blue
Access charges: Orange

Any other border color can remain the same as the main color as it's unique anyways.

In any event, Buttons should be properly ordered by their type of information, so we should swap the position of "Distance" and "Access charges" and move the "average speed" closer to "Acceleration" and "Tractive effort", as these three are whilst technically incompatible still more or less related.

So I would suggest the following order:
Revenue, Running Costs, Profit, Access charges,
Total Capacity, Transported, Distance, Comfort,
Acceleration, Tractive Effort, Average Speed

I know the number of buttons per line is size dependant but I guess allignung the same type of information to the same line in default window scale is not a bad thing.

Mariculous

Quote from: Ranran on January 31, 2020, 01:37:53 PMPerhaps (2) is wise?
A mapping is quite straight forward, simple, efficient and does not break compatibility, so i guess yes, that is wise.

Quote from: Ranran on January 31, 2020, 01:37:53 PMAbout color uniformity,
Does it make sense to swap the colors of "Cash flow" and "Operating profit" in the Finance window?
That is, match the color of "profit" in the convoy info dialog.
Absolutely! The same type of data should have the same color in any aspect where it is listed, thus e.g. Operating Profit should have the same color in vehicle window, line window and finance window (and any further if I forgot one)

jamespetts

Thank you for this.

I have now had an opportunity to test this a little further. I notice one anomaly at this stage: I am not sure how easy that it is to fix. This relates to the tractive effort graph: both numbers (the tractive effort and rolling resistance) seem to reduce to zero when the vehicle reaches its maximum speed. However, the x axis on the graph is not truncated to this point, but shows a large amount of useless data to the right of this point. One particularly bad example is convoy no. 22 on demo.sve, where about a quarter of the graph is occupied with zero data. Ideally, the graph would end at the highest speed attainable and before the values drop to zero. In the current state, it is very difficult to read the resistance graph, since the value on the right hand side is always given as zero, so that the only way to read any values on the graph is to mouse-over them.

Also, I notice that there are no tooltips for the buttons in the convoy window, but this is equally the case in the current master branch. Had it been intended to implement these?

Thank you again for your work on this.
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.

Mariculous

Quote from: Ranran on August 02, 2020, 02:58:06 PMI think that was the cause of the crash on Linux.
lalala std::array is nice, we all should like the stdlib.

jamespetts

Quote from: Freahk on August 02, 2020, 03:25:37 PM
lalala std::array is nice, we all should like the stdlib.

The problem with the std collection classes is that they are often slower than the Simutrans equivalents (this is definitely true of the hashtable), and this can make a big difference to performance whenever they are used in performance critical parts of the code.

The Simutrans vector_tpl is sufficient for most cases where an array is needed.
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.

Mariculous

This is definitely not true for std::array. It is equally fast as plain C arrays, though the operator[] doesn't do boundary checks by default for performance reasons.
It can however be compiled to do these checks, which plain arrays (e.g. something like int[256]) cannot do on its own.

Whenevever an array is needed, you don't want to use vector_tpl, as it's not an array but a vector.
The standard equivalent to vector_tpl is simply called std::vector and it's pretty fast either. I would not expect any performance penalty compared to vector_tpl here either.

About std::map, it's still much more complicated here. In the first place, std::map is not the equivalent of hashtable_tpl, because std::map is ordered, hashtable_tpl is not (within a single bucket it is, but generally it's not.)
The actual equivalent is std::unordered_map, which is usually roughly 3-times faster than std::map, though it cannot be said that generally either.
On top of this, you definitely can not even generally say "simutrans implementation is definitely faster than the standard one"

Hashtables are always a tradeoff in between memory consumption and performance.
Simutrans implementation makes use of a fixed size of 101 buckets, which semselves are linked lists.
So with a small number of elements, you will most likely never iterate any of these lists. The has will tell you in which bucket you can find the data and the data will most often be the very first element of the list.
Though, even if there is only a single element stored in the hashtable, there will be 101 lists, of which 100 are empty and one contains exactly on element. This waste of memory is obviously very fast when it comes to any kind of operation.

On the other hand, if you insert 101 million elments into a single simutrans hashtable, there will be on average 1 million elements per list, so your performance will be simmilar to accessing from or inserting to a 1 million element long linked list.
Even std::map will be much faster here, not to mention std::unordered_map will be even faster.

jamespetts

That is interesting, although I should note that my performance comparisons were with std::unordered_map, not std::map.
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.

jamespetts

Ranran - thank you for this. I have now had a chance to test the latest version. What I notice is that the graph is truncated but that the scale of the chart is not updated to match that truncation such that there is still a considerable amount of wasted space on the right of the chart. Would updating the scale of the chart dynamically so that the truncated lines end up on the far right of the chart no matter where the truncation takes place be challenging or is it something that can readily be achieved?

Thank you again for your work on this.
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.

Mariculous

I don't know. This was suggested before, but Ranran said it would be very difficult.
I assume the idea behind this was to have fixed speed steps in between two data points but a variable number of data points. I have no idea of the involved graphics code, so I can't say anything about this nor work out an idea to solve this.

The other way round might be easier. Have a fixed number of data points but variable speed steps in between these.
You could simply get the maximum speed and display data of every vMax/datapoints increment of speed.

I am quite sure I have seen something like this before in one of the graphs you presented.
Why was this replaced by the current figure with a lot of empty space?
If it was because of odd speed steps, like 225/24=9.375 km/h, this could be solved by rounding these steps up in 5 km/h steps or something, so at least there will be less wasted space.

jamespetts

I notice that Ranran has pushed some changes that appear to be making progress towards truncating the graphs, but the trunction seems to be off by one so far. I shall be interested to learn of progress in relation to this.
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.

freddyhayward

This branch still crashes on linux due to the use of char *.

freddyhayward

Quote from: Ranran on August 10, 2020, 10:28:55 AM
Thank you for testing.
The only new char variable I added is the button label, so I wonder if it was because I didn't enclose the added macro replacement list in parentheses. If not, I can't think of any other cause... (´・ω・`)
I could not properly trace where the error originated, because the memory was corrupted by some kind of overflow. But it did show that it came from number_to_string().
EDIT: I tried with your latest commit, "align buffer size", but it still crashes,

freddyhayward

I found the cause of the crash - to calculate the precision of the curves, you used the lookup-table array that stores whether a given button index ought to display curves, and multiplied it by two. The intended outcome was 0 (false * 2), but instead accessed garbage data beyond the array's capacity, so the actual outcome was 180 digits after the decimal place, too large for the maximum size of 128 allowed by number_to_string. How was the button index and the lookup array capacity determined? I don't know, but it seemed to be some ancient ritual using macros. I can't blame you for following tradition - you did not introduce any bad code yourself, but relied on it. I think that this episode has provided a valuable lesson in what to avoid: avoid macros, avoid c-style arrays, and avoid c-style strings/buffers. The first two should be replaced by an enum containing the names of all the buttons and a function that maps each button index to the corresponding number of decimal places using a switch statement.

EDIT: The crash still occurs upon click on the convoy in release builds. Only the debug builds work.

Separately, I have a few issues with the current implementation of this feature.
1. I don't think that these graphs belong with the history charts - they use a different x-axis and cannot be overlaid with them. If you have 3 history curves enabled, but then enable the acceleration curve, the other 3 disappear. Disable the acceleration curve and the 3 curves are still missing, but the different x-axis from the acceleration curve remains.
2. The curve flaps up and down when the convoy is moving. This ought to be constant information.
3. it is unclear what the x-axis means.
4. it is unclear what the secondary line means, and it is impossible to see it clearly.

jamespetts

I think that, if the crash can be fixed, this patch in its current form is better than not having an acceleration graph even if it would be ideal to have the graphs in the depot one day.

Ranran - can I check whether the crash has been fixed? If so, I can re-test and consider any more minor amendments that may be needed.
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.

freddyhayward

Quote from: jamespetts on August 18, 2020, 10:58:40 PM
I think that, if the crash can be fixed, this patch in its current form is better than not having an acceleration graph even if it would be ideal to have the graphs in the depot one day.

Ranran - can I check whether the crash has been fixed? If so, I can re-test and consider any more minor amendments that may be needed.
I should just note that these crashes tend to occur on Linux rather than Windows, so it must be tested on Linux (which I will gladly do) before merging.

jamespetts

Quote from: freddyhayward on August 19, 2020, 01:39:40 AM
I should just note that these crashes tend to occur on Linux rather than Windows, so it must be tested on Linux (which I will gladly do) before merging.

That is very helpful, thank you. If Ranran could let Freddy know when this is ready to be tested and if Freddy could let me know when he has tested, that would be very helpful.

Thank you both.
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.

jamespetts

I have seen some (possibly conflicting) activity on Github relating to this, but I do not think that I have seen any confirmation that this is ready; can I check what the status of this is to make sure that I am not missing this in error?
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.

jamespetts

Thank you for this - that is very helpful. I am sorry that you have to do so much work to redesign what was virtually finished and very useful.

In relation to a GUI overhaul, I should be interested in your views on how that should interact with the much delayed work on convoy recombination and vehicle maintenance; the UI on this has been started by Ves some time ago, I think, but I am not sure how much work has been done in relation to it.

At present, my priority needs to be to try to find the cause of the loss of synchronisation, so there will be some time before I can return to consider working on the balance critical features, in which time some consideration might be given to whether the GUI overhaul should be done first so that the GUI for this ends up being written with the new UI code.
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.

jamespetts

This is extremely helpful - thank you for all your excellent work on this. I will hopefully have a chance to test progress so far once the loss of synchronisation issue has been fixed.
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.