[patch] Change the graphs so they go from old-to-new instead of new-to-old

Topic: [patch] Change the graphs so they go from old-to-new instead of new-to-old (Read 11205 times) previous topic - next topic

[patch] Change the graphs so they go from old-to-new instead of new-to-old

July 02, 2009, 08:50:50 pm

Title says it all.

Since I ****ume that some people (or actually most people) like the current behaviour I made it so that it is easy to make it configurable (but since I don't know the config system that is not yet finished)

So for now the patch changes the direction of the graph in the finance window (as a proof of concept)

Please comments on this patch (and help on how to make it configurable)

Download patch

Note: this is a patch based on simutrans-experimental (git), but I hope it also applies on standard-svn

Note: I saw a lot of strange things in this file: (this was my first vision of the simutrans source code)

* No code-style consistency (But since this project is touched by many people is forgivable, but are their any guidelines for newly written code?)
* Usage of pscale where references could be used or just &scale in the function call of calc_gui_chart_values (same for pbaseline)
* The implementation of display_direct_line() (and maybe others) => why not using the drawing functions of the backend (SDL, GDI, ...) since the in-house implementation is probably slower.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #1 – July 02, 2009, 09:07:16 pm

Coding rules:
In the folder documentation there are code guidelines. Also on SF wiki are guidelines.

display_direct_line:
SDL does not have a function to draw on all offscreen bitmaps, same goes for allegro (albeit the latter does not use offscreen bitmaps). And GDI does not like 15 bit modes when the screen is on 32bits sometimes. Also the slowness does not really matter for graphs.

I am curious however, what is the motivation fror that patch?

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #2 – July 02, 2009, 09:30:05 pm

Most (if not all) graphs (with on the x-as time) I've seen (outside simutrans) have the oldest dates left, and the newest dates right

Since I like this behaviour (and simutrans does just the oppositse) I changed it...

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #3 – July 02, 2009, 11:31:50 pm

I have to say, when I first started using Simutrans, I found the reverse direction of the graphs very confusing, too. However, I suggest that any patch to change something like this that has been in Simutrans since time immorial be made optional, so as not to confuse seasoned players.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #4 – July 02, 2009, 11:46:36 pm

Quote from: jamespetts – on July 02, 2009, 11:31:50 pm

I have to say, when I first started using Simutrans, I found the reverse direction of the graphs very confusing, too. However, I suggest that any patch to change something like this that has been in Simutrans since time immorial be made optional, so as not to confuse seasoned players.

That was the idea, but I don't know how to hook it (The graph cl**** has a new function set_ltr(bool) which when set to true uses this behaviour, but the default is still the old behaviour)

So everywhere where a graph is made the function graph->set_ltr(#value in config file) should be called. But I don't have the knowledge to do that...

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #5 – July 02, 2009, 11:55:44 pm

Look in the cl**** umgebung_t for the various options. You will also need to add a button to the "display" menu (color.cc) so that the option can be toggled in-game.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #6 – July 04, 2009, 03:42:44 pm

I pushed a git repo to:

http://github.com/nathansamson/simutrans/

It now reads the option from the config file (finance_ltr_graphs for finance windown, ltr_graphs for all other graphs) (0 = default = old behaviour)

I still have to add an option to the display menu (although maybe this is not wanted, not all other options are there...)

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #7 – July 04, 2009, 03:57:02 pm

Nathan,

this looks interesting - I shall have to try it out! I really think that an option in the display menu is needed for this one, though, as it is something that makes a big difference to how things appear, and could be quite confusing to users if it appears different to how they normally have it. The simuconf.tab file is a cumbersome and awkward way of changing things, and should not really be necessary for mere user preferences, but should rather be used for gameplay balance settings that are generally set by a pakset author, rather than by a player. Indeed, one may wonder whether a simuconf.tab setting is appropriate at all for this function, rather than a settings.xml setting.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #8 – July 04, 2009, 04:16:07 pm

I didn't even know the existance of these settings.xml

and maybe you are right: user settings should go in settings.xml and other configurations (that are more gameplay specific) should go in simuconf.tab

You are also right in the fact that simuconf.tab is not the most readable format but still I believe that config files should be more or less human readable, which settings.xml is defintely not. (I don't know why, but XML is abused here. It should not matter in what order the tags are, but in this case it does)

Btw I did not use umgebung because it seemed that it was version specific so I was thinking it had more to do with the gamesave format or so (I had no idea why a config file would have needed a version number), it seems that I was completely wrong (and it is completely clear why a version is needed for this game format)

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #9 – July 04, 2009, 04:20:15 pm

YAY! This patch addresses one of the most brain-bending aspects of the UI... nowhere else in the world will you see a "reverse time" graph.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #10 – July 04, 2009, 04:24:44 pm

Nathan,

umgebung is used for things that do not need to be saved with the game, and einstellungen is used for things that do need to be saved with the game. The former deals with superficial user preferences (such as cycling between day and night views, etc.), and the latter with things that affect gameplay. The former settings are saved in settings.xml and are specific to each user: the latter are saved with each saved game and are specific to that saved game. The distinction will become even more important when multiplayer gaming via a network/the internet is introduced.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #11 – July 04, 2009, 04:29:29 pm

Quote from: jamespetts – on July 04, 2009, 04:24:44 pm

Nathan,

umgebung is used for things that do not need to be saved with the game, and einstellungen is used for things that do need to be saved with the game. The former deals with superficial user preferences (such as cycling between day and night views, etc.), and the latter with things that affect gameplay. The former settings are saved in settings.xml and are specific to each user: the latter are saved with each saved game and are specific to that saved game. The distinction will become even more important when multiplayer gaming via a network/the internet is introduced.

I used einstellungen, but still I can switch an old savegame from old to new behaviour (and the other way arround)

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #12 – July 04, 2009, 04:30:22 pm

It's best to be consistent, and use umgebung for user preferences that do not affect gameplay.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #13 – July 04, 2009, 07:12:54 pm

Ok it uses now umgebung.

This was the only patch for simutrans I was planning, but now I seriously think to do some others:

overall cleanup (unused variables, etc) => no warnings is my goal (but maybe impossible)
configuration mess cleanup (but I'm afraid part of the messup is to be compatible with older configurations, so this maybe hard to achieve)

I'll will start a thread for this last one to have some communication with the simutrans-devs (if I really start doing this)

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #14 – July 04, 2009, 07:26:01 pm

Before clean ing just a remark that this only makes sense for a stable version, when bugs are considered mostly harmless. And to have you patch included, please post it as a diff with a version number that patch can understand.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #15 – July 04, 2009, 07:43:28 pm

Quote from: prissi – on July 04, 2009, 07:26:01 pm

Before clean ing just a remark that this only makes sense for a stable version, when bugs are considered mostly harmless.

(Emphasis mine)

I never knew that you were a Douglas Adams fan ;-)

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #16 – July 04, 2009, 09:24:14 pm

[very far offtopic]
Any programming entity born 1970 might be strongly influenced by Douglas Adams, I presume.
[/very far offtopic]

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #17 – July 04, 2009, 09:36:31 pm

True ;-)

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #18 – July 06, 2009, 02:26:31 pm

Final patch (with configuration options in display) (only final if no problems are found...)

Download patch

Apply with "patch -p1 -i graphs_final_r2569.patch" it applies to r2596 and r2570 (at least, probably earlier versions too, but that doesn't matter)

As you can see I also did some "refactorings" in colors.cc.
In the earlier versions it was hard do insert a widget (all constants for the widgets after the inserted one had to be adapted), with my "new system" only one constant has to be changed...
I also moved the MAX_BUTTONS constant to colors.h (since the same number has to be used there) and is renamed to COLORS_MAX_BUTTONS (if other widget frames are doing the same I can imagine MAX_BUTTONS will conflict...)

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #19 – July 07, 2009, 06:34:55 am

I think this doesn't need GUI settings.
This is the same as "window_buttons_right", we don't need to change this in the game. simuconf.tab is enough.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #20 – July 07, 2009, 08:05:40 am

I think that it's rather easier for users to be able to change options via a GUI interface. Ideally, all options would be changed via a GUI interface, and there would be no simuconf.tab at all.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #21 – July 07, 2009, 10:22:51 am

Quote from: z9999 – on July 07, 2009, 06:34:55 am

I think this doesn't need GUI settings.
This is the same as "window_buttons_right", we don't need to change this in the game. simuconf.tab is enough.

simuconf.tab is not for user settings, they should be in settings.xml, and since that file is not ought to be read by humans...

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #22 – July 07, 2009, 01:26:03 pm

I don't agree with you.
Settings file should be readable and be editable, which should contain both "key" and "value".

settings.xml is a temporary value. Only the way to repair to default value is to delete file. If only the way to do this is GUI, we need to waste too many time to customize it. It's very bad.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #23 – July 07, 2009, 06:41:20 pm

Umm... shouldn't we keep the (heated) config debate at one place?

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #24 – July 07, 2009, 09:14:25 pm

Nathan,

I have just had a chance to test out your latest code, and, I must say, it makes the graphs much easier to read. I have changed your code a little so that it is enabled by default from the next build of Simutrans-Experimental onwards (but veteran users can disable it, and the settings will be saved in settings.xml). This should make things a great deal easier for newer users - thank you very much for the work :-)

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #25 – July 08, 2009, 07:03:56 am

In my opinion, the old unintuitive graphs with reversed time axis shouldn't be kept just for historical reasons. Is there any benefit from this?

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #26 – July 08, 2009, 11:11:20 am

You can more easily see the number on the left axis of the current value. Moving it to the right would in principle need also some resizing; however as the axis do only have max and min and no nice scaling this is anyway something for improvement.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #27 – July 08, 2009, 11:23:57 am

Maybe the whole numbers on the y-axis should be moved to the right? Any better suggestions for designing the graphs?

Imho, the reverted time axis isn't worth a configuration option. One thing I didn't liked in OTTD was, that you have 5+ pages full of unclear options, I think we should avoid this.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #28 – July 08, 2009, 07:54:18 pm

Ahh, I had specified an option for Simutrans-Experimental because regular users might prefer to use existing settings. If the change is to be adopted in Simutrans-Standard, too, then there might be less need for there to be such an option.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #29 – September 05, 2009, 07:59:44 pm

Incorporated (with slight modifications) in 2630.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #30 – September 06, 2009, 01:24:43 pm

After patch inclusion, max and min values (in white) are still shown to the left of the graph, causing the current value to appear _outside_ the main dialog. Screenshot included.

Re: [patch] Change the graphs so they go from old-to-new instead of new-to-old

Reply #31 – September 06, 2009, 01:33:38 pm

Quote from: sanna – on September 06, 2009, 01:24:43 pm

After patch inclusion, max and min values (in white) are still shown to the left of the graph, causing the current value to appear _outside_ the main dialog. Screenshot included.

It's great. I like this.

But unfortunately, some of text color are not readable.