News:

Simutrans Sites
Know our official sites. Find tools and resources for Simutrans.

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 2 Guests are viewing this topic.

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的话,那么说不定就想看《日本铁路之旅》(英语也有简体中文字幕)。

Ranran(retired)

Matthew - Thank you for testing. The part related to the button appears to have a problem, but currently has no idea...



I have added a theoretical high to the acceleration chart. This represents an acceleration graph with no load and unaffected by speed limits and slopes.




I changed the "Access charges" chart button to violet because orange has some of duplication.

In the finance dialogue, the expression "Access charges" is used. Is the expression "Access charge" correct in the convoy info dialog?
As you can see in the image above, "Access charges" is caught by the character limit, but I think "Access charge" will fit.

Finance originally modified Road toll with translation files. Does it change "Way toll" to "Access charge" via tab file in that case?
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

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

Ranran(retired)

Quote from: Freahk on January 31, 2020, 12:29:21 PMhowever imho the theoretical max load acceleration rather than min load is the most important figure for comparisation.
I agree, it's make sense. I would change that way.

Quote from: Freahk on January 31, 2020, 12:29:21 PMIn 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
I changed the color of some charts in this way when I changed the station chart. However, multiple charts are complicatedly involved, and it is troublesome to avoid conflicts. This time, I think the same can be said for convoy and finance. The finance window has many red-based colors. Which color do you think you should change?
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

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.

Ranran(retired)

I agree that similar kind information should be adjacent and more important information should be placed ahead. I was thinking a little about it.

Basically, the button layout code is from standard, which is arranged in numerical order of the record. To change this,
(1) Specify and place each one
(2) Create and assign an array that registered the location of each button
(3) Replace data when reading from old save depending on save version

Perhaps (2) is wise?

About 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.
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

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)

Ranran(retired)

I wondered if some work would be needed for this repair work, but it was completed soon. So finally I post it. (Pull request #223)
This patch was frozen because I was unable to address a crashing issue on linux last time.
But then, with the clue as to the problem caused by another patch, it turned out that it was most likely caused by the tooltip character limit.
I just fixed it and resolved the conflict with the master branch. (Please note that those tooltips were added by this patch and were not originally there. And I recommend that you revise the help text to explain this to players.)
I would appreciate if someone could check this on linux to see if it caused a crash.
Adieu (´・ω・`)
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

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.

Ranran(retired)

QuoteI am not sure how easy that it is to fix.
I'm worried that the code for it creates some complexity, as it hacks and displays an existing chart code.

QuoteIdeally, the graph would end at the highest speed attainable and before the values drop to zero.
I made that change.

QuoteAlso, 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?
Yes, I tried to add it while I was making this patch before. But I made the array size too small. I think that was the cause of the crash on Linux.
I didn't add it this time because it didn't originally exist. If it doesn't crash in Linux, it's clear that it was the cause.
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

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.

Ranran(retired)

Label must be an integer. Otherwise it will be difficult to set the x-axis label and see the graph. So I set the proper axis and then quit the process.

As already mentioned, it hacks and displays existing systems like an overtaking patch, so it inserts a lot of code to resolve inconsistencies in order to be consistent with existing systems. I'm afraid that this will compromise readability and maintainability.
For example, it doesn't depend on this graph orientation option.
extended has a different orientation than standard from the default orientation of the graph.
So this means the graph of maximum speed on the left. It is the exact opposite of a general f-v/v-t graph.
If this is cut off in the middle, the label value and position and the drawing position of the graph must be adjusted.
If you want to fix this in the future, you need to be careful if the left_to_right_graphs option is changed and it still works.
That way many cat and mouse games are executed.
QuoteI notice that Ranran has pushed some changes
Anyway i implemented it. I apologize for the late report.

And unfortunate news. I made a mistake in the judgment formula that aborting the chart display last time. That is, the chart was aborted one step before the actual display.

Quotethe trunction seems to be off by one so far.
Therefore, it is a correct specification that the chart should be 0 at the end. This is because the acceleration force of a vehicle with a maximum speed of 120 km/h returns 0 when the maximum speed is 120 km/h. That is the current physics code. If not, it will accelerate to 121 km/h.

EDIT:
There is currently a bug with incorrect x-axis labels. (One shift)
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

freddyhayward

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

Ranran(retired)

Quote from: freddyhayward on August 08, 2020, 12:42:22 PMThis branch still crashes on linux due to the use of char *.
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... (´・ω・`)
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

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.

Ranran(retired)

Thank you very much for testing and elucidating the cause. That is very helpful.
I almost agree with freddyhayward's opinion, but after thinking about the solution, unfortunately this patch seems to have to be moved to the end of the long queue again. (´・ω・`)

Originally this patch was a fairly low priority for me. But I happened to find the Bernd Gabriel's chart in the code and thought I could implement it with a little effort, so I worked on it.

Quote1. 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.
This was implemented here only because Bernd Gabriel made it here. (It may have been just for debugging purposes.) It's better to check in advance in the depot than to check it when convoy is running. Adding charts to the depot dialog also seems like a big task. The overhaul of the depot dialog may also be something to go with the addition of the overhaul feature or the re-combination feature. (Accompanied by changes in the replace dialog.)

Originally there was only one graph and only 12 x-axes. I felt that the x-axis was missing, so I expanded it. Added more items.
I hacked and expanded the chart like that, but this was not a good technique. I think we should have separated the charts with tabs.
The tabbing of convoy info is done in standard, and standard has integrated the convoy detail tab into 1 dialog. But in my opinion, extended should not go the same way.
Convoy detail is already tabbed and has some tabs. In addition, it is necessary to tabbed features that are not in standard such as class manager and times history.
Tabbing them isn't a high priority due to their small benefits, and it's better to wait for the standard GUI overhaul to be incorporated. (At least for the moment, tabbing them seems like a lot of work.)
Macros will no longer be needed.

Quote2. The curve flaps up and down when the convoy is moving. This ought to be constant information.
This is almost exactly the specification made by Bernd Gabriel. It needs to change the design because it picks up the current data of convoy and needs to eliminate those effects. Some also wanted to know the current acceleration and acceleration time information.

Quote3. it is unclear what the x-axis means.
I think standard has the function to add units but extended has not.

Quote4. it is unclear what the secondary line means, and it is impossible to see it clearly.
Separating the buttons suggests that we should have three tabs for charts.


These are the reasons why this patch is postponed to a distant future. (´・ω・`)
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

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.

Ranran(retired)

Thank you for your opinion.
Unfortunately, I think the effort to make my patch sanity is huge and the outlook for the future is bleak. (´・ω・`)
I just tampered with the chart code by incorporating from the standard. It can't avoid conflicts with them, and requires more effort. It is useful in its own right as it includes the ability to display units on charts, and is also useful for accelerated charts. Rather, it's a necessary feature as pointed out by freddy. As such, the code in this patch needs a major overhaul as it needs to be redesigned.

Quote from: jamespetts on August 25, 2020, 08:59:29 PMI have seen some (possibly conflicting) activity on Github relating to this
I'm aware of a bug in the code on the master branch and I've fixed some with my unfinished patch. Many of the variables used for the maximum value of the chart's for statement are incorrect. (save and loading thing too)
I think others fixes that are relevant are probably those, but those fixes are welcome. Don't worry about the progress of this patch.  ;)

Now I'm thinking it's better to wait for a GUI overhaul as recommended by Prissi and A.Carlotti. Because I feel it is relatively close. It can be a daunting task in itself, but it's definitely beneficial.
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

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.

Ranran(retired)

I think ceeac was working on restoring the acceleration chart. It should look something like this, as I introduced before.
Quote from: Ranran on January 06, 2020, 06:02:29 AMIt was something like this.  8)


Quote from: jamespetts on August 29, 2020, 11:15:47 AMIn 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.
I'm currently focusing on commits related to GUI and drawing. Because I'm looking at the GUI code to some extent, conflict resolution doesn't take much time. In addition, it is unlikely that incorporating about the GUI will affect desync, so I think it will not be so far to proceed to just before the GUI overhaul.


There will be three changes in the GUI coding in the near future.

(1) r8134(120.2.2) - The color definition is extended from 8bit to 16bit.
Therefore, it is necessary to deal with the change of variables and functions. This is what I am working on now and is almost complete. Currently stumbling on reading the old save.

(2) r8434(120.3) - Support for truetype fonts and large fonts.
Players will be able to select their favorite font in settings. You can easily change the font from the setting dialog.
This will require revising the layout and variables. I'm now ready to start this incorporating work and I can start it anytime.
Now that the basic import is complete, I'll pick up the related commits.

(3) r8653 - GUI overhaul
This requires a rewrite of the all GUI code. Create a GUI to place components in tables and cells. This requires a lot of work and an extended-specific GUI can require a lot of work. This is why some people advised to wait until this stage for making a new GUI.


If you create a new GUI at the moment, that GUI will have to deal with all the conflicts for these three changes. (´・ω・`)
If we don't merge those from standards, we don't need to. But with phystam progressing it to some extent, I realized it was imminent.
So when it comes to the GUI, I thought I could speed it up, so I decided to go ahead with it rather than make new changes.
If Ves or someone else doesn't handle the three changes, creating a GUI after the introduction of the overhaul will save you a lot of work.
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

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.

Ranran(retired)

I ported the accel chart to convoy detail so that I can see the accel chart from the depot as reported in another thread.
However, it is currently not possible to open it from the depot. (However, it has been changed so that the dialog does not close when entering the depot.)
It would be helpful at this stage to see if it would cause a crash on linux as before.

The branch is accel-curve-chart-v2.

Acceleration and force information has been separated into separate tabs based on freddy's suggestions.
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

freddyhayward

Quote from: Ranran on November 01, 2020, 05:13:28 AMIt would be helpful at this stage to see if it would cause a crash on linux as before.
I cannot compile that branch. When I try to checkout, it deletes cmakelists.txt and breaks my IDE. I can't remember what the error was, something with type_traits and static_assert. I can't try it again because I don't want to break my configuration again. According to github there are over 700 commits in that branch - it can't have been a good idea to expand the branch with additional changes and features when the most important feature is broken.

jamespetts

Can I check whether this branch is intended to work alongside the master branch or one of the merge from Standard branches?
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 November 01, 2020, 02:11:09 PM
Can I check whether this branch is intended to work alongside the master branch or one of the merge from Standard branches?
I understand this is addressed to ranran, but I should add that the other merge-from-standard branches that I have tested are able to compile and run successfully.