Game Settings Overview Window

Got an idea for OpenTTD? Post it here!

Moderator: OpenTTD Developers

Eagle_rainbow
Engineer
Engineer
Posts: 59
Joined: 13 Oct 2012 20:27
Location: Germany

Game Settings Overview Window

Post by Eagle_rainbow »

Hi everyone,

Playing OTTD now for a while, I start participating in multiplayer sessions on the internet. Though slowly "becoming domestic" on one server, what I like most is, that you can play on one server today, while you will be playing on another tomorrow. Doing so, you will notice that the server settings may be quite deviating from each other, which means that you have to adjust your building style. However, I find it nerve-wracking to browse through all the three (well, in most cases it's only two) setting screens to find out what's the current settings of the server you are logged on to not spoil your game with a wrong focus right from the beginning. That's why I was searching for a "brief settings overview" window somewhere that was telling me the most important settings of the server currently logged on at a single spot. However, I was not able to find anything that came close to my needs.
Trying to get familiar with the development style in OTTD, I started to implement a first prototype for the idea above. Here's a screenshot where I currently landed after some iterations:

Image

Note that:
  • This overview window is read-only and does not show anything else that you couldn't get out of the rest of the screens already.
  • The collection of settings to be shown is my perferred list only and is kept short by concious decision
  • The window is reachable via a dedicated setting in the Settings Dropdown menu.
What's available there currently is entirely satisfying my needs. Yet, I'd like to receive feedback from you, if you find that useful as well or where things can still be improved, for example:
  • Layout Design
  • Labelling/Language consistency
  • Which settings to show (and which to leave out and why)
  • Coding Style used and where I still have glitches
One feature that looks interesting to me is that the window should open up automatically once you have logged on to a server successfully (to me only network games make sense, as with a local game, you have chosen those values yourself a couple of minutes before anyway...). However, I am still searching for the right coding location yet... :roll:

For reference to you, developers, please find attached a patch file against current trunk revision 24586. Furthermore, there is also a patch version against branches/v1.2.

Thanks for your feedback and having mercy with me for my "first patch" :)

Regards,
Eagle_Rainbow
gso-r24586.diff
game settings overview window patch against trunk (as of writing)
(12.68 KiB) Downloaded 151 times
gso-branch-1.2.diff
game settings overview window patch against branch/1.2
(12.8 KiB) Downloaded 139 times
Attachments
Settings Overview Window Screenshot
Settings Overview Window Screenshot
gamesettingsoverview.png (88.45 KiB) Viewed 5683 times
minecraftfan
Engineer
Engineer
Posts: 9
Joined: 13 Oct 2012 13:08

Re: Game Settings Overview Window

Post by minecraftfan »

I would love this to be implemented. The number of times I have logged onto a server and wasted 30 seconds trying to find the useful options means something like this would be brilliant in trunk. What do yuo say Owen?
frosch
OpenTTD Developer
OpenTTD Developer
Posts: 988
Joined: 20 Dec 2006 13:31
Location: Aschaffenburg

Re: Game Settings Overview Window

Post by frosch »

The selection of settings to display in such a window is quite arbitrary. I mean, imagine a server disabling "build on slopes". You most certainly want to see that, but then the window would be huge.

So, how about a different approach: Display all non-client settings which deviate from the local client's new-game settings?
That would also work for single player and scenarios to see the settings deviating from the values the player is used to.

Also you can generate that window from the settings descriptions, it would not require any decision on what settings are considered "important" (which is quite subjective).
⢇⡸⢸⠢⡇⡇⢎⡁⢎⡱⢸⡱⢸⣭⠀⢸⢜⢸⢸⣀⢸⣀⢸⣭⢸⡱⠀⢰⠭⡆⣫⠰⣉⢸⢸⠀⢰⠭⡆⡯⡆⢹⠁⠀⢐⠰⡁
Eagle_rainbow
Engineer
Engineer
Posts: 59
Joined: 13 Oct 2012 20:27
Location: Germany

Re: Game Settings Overview Window

Post by Eagle_rainbow »

I see your point. During implementation I also had my doubts on the fact that my selection might be very personal - and thus arbitrary for others. My initial thought on that was to allow making the list of settings user-configurable (for example, via .ini entries or something like that). Already there I felt a bit uneasy and that's why I did not implement it, but first wanted to have some feedback from you here (there were other reasons as well to make this suggestion, though). And as we see, your valid point with "building on slopes" would also not be covered by my .ini-approach (as the user most likely won't have had added it to the list).

On the other hand, your approach also has a weakness, which I currently don't feel comfortable with: Assume that you have set your local settings to something totally odd, because you wanted to check out a variant how this feels like playing with. Joining a server which you really know well thereafter, you will then get a huge strange list of about "trivial and very well-accepted" setting values. The list then will be filled with a lot of clutter. Having joined there before, you remember that the list was really short... This behaviour can also appear to be quite erratic to the end user and could provoke bug reports with something like "Settings Overview screen provides random selection".

What about the idea to not make it a "diff against your local settings" but "against the default settings" (as shipped via release packages)? This is a globally defined fixed set and - normally - every player will become familiar with them at least once in a lifetime :)
Last edited by Eagle_rainbow on 14 Oct 2012 19:59, edited 1 time in total.
Cheers,
Eagle_Rainbow

My Wiki Home (incl. overview of patches)
frosch
OpenTTD Developer
OpenTTD Developer
Posts: 988
Joined: 20 Dec 2006 13:31
Location: Aschaffenburg

Re: Game Settings Overview Window

Post by frosch »

I wondered whether something like this could be integrated into the existing adv. setting dialog.

Something like:
  • A dropdown to select between "show all settings", "show deviations from new game setting" (only in game), "show deviations from default settings", "show deviations from default settings, not considering client/gui settings".
  • Collapse/expand all
  • Make these settings the new game settings. (only in game)
  • Revert settings to default. (only in main menu)
This would cover your GUI when triggering the "expand all" upon selection of the "show derivations from xxx settings", and it would also cover the often-suggested "revert to default" thing.

I am not sure about the "not considering client/gui settings". You do not want to see them when checking a server because they will be your settings. But you want to see them when generally checking your settings. So I made it two options for now :s
⢇⡸⢸⠢⡇⡇⢎⡁⢎⡱⢸⡱⢸⣭⠀⢸⢜⢸⢸⣀⢸⣀⢸⣭⢸⡱⠀⢰⠭⡆⣫⠰⣉⢸⢸⠀⢰⠭⡆⡯⡆⢹⠁⠀⢐⠰⡁
Yexo
Tycoon
Tycoon
Posts: 3663
Joined: 20 Dec 2007 12:49

Re: Game Settings Overview Window

Post by Yexo »

frosch wrote:...
I like that idea :)
Eagle_rainbow
Engineer
Engineer
Posts: 59
Joined: 13 Oct 2012 20:27
Location: Germany

Re: Game Settings Overview Window

Post by Eagle_rainbow »

I wondered whether something like this could be integrated into the existing adv. setting dialog [...]
Could also be an option.
However, to be honest, already from the beginning I had my troubles with all these settings screens. First of all, I never really got the point why there are three of them (game options, difficulty and adv. settings). And you are right in that sense, that a fourth one (which then is r/o) doesn't make the world better either. Especially the advanced settings window (affecting both visualization and difficulty) is hard to chew. The tree there tries to make this long list more digestable. However, what is causing me headache, is that the grouping is not really orthogonal and distinct. Let's look at an example for what I mean:

The option "Show timetable in ticks rather than days" are below "Interface" only. On the other hand this is just a "display thingy" which - to my mind - would qualify it to the settings page "interface -> Display Options". However, you also could argue that it should go somewhere to the "vehicles", for instance, next to the "warn if a vehicle's income is negative" (which is just another thing, that only affects if/how something is shown). Moreover, you could think of putting it to another setting page after all ("debugging" or "technical"), as I would assume that most standard users will not like to see rather technical game ticks, but would prefer having something tangable on the screen.

To make a long story short: though I appreciate the idea of having a tree there, the logic behind the grouping for this tree never became obvious to me (if there is a certain hidden logic behind it, then I am eager to listen to it!). Taking the example above, I also have my doubts that it will be easy to find one that fits all. Adding buttons and features to the list, will not make this thing more logical, but for sure by providing a mechanism to filter the list of rows would reduce the complexity for the user.
However, the most important part IMHO would be to group all those settings better: Today you always need to look very closely if the buttons are disabled if you are on a network game opening the adv. settings window and try to see what you can adjust "locally" (because it does not affect the other players on the server) and what has been set by the server, because it affects gameplay. Only those will have an impact on the "settings overview" at all.
Going for an integration into the advanced settings window we need to consider this - otherwise it could become risky that we overload an already tricky screen. And that is exactly what I wanted to prohibit with my suggestion at the first place...
Cheers,
Eagle_Rainbow

My Wiki Home (incl. overview of patches)
User avatar
planetmaker
OpenTTD Developer
OpenTTD Developer
Posts: 9432
Joined: 07 Nov 2007 22:44
Location: Sol d

Re: Game Settings Overview Window

Post by planetmaker »

I do agree with your analysis that the three different settings windows (options, difficulty, adv. settings) is rather sub-optimal - a historically grown jungle of settings in various places.

Personally I'd prefer a split in user-settings and in game settings, maybe a 3rd network setting for cases where you start a (non-dedicated) server.

Wrt grouping: that's a difficult thing IMHO - any grouping is somewhat subjective, I fear. I once tried to (re-)group them... but always found that some settings belong in at least two categories. But doing so certainly won't help conciseness of the already long list - even when it's split into the above mentioned user, game and network parts.

That said, I also like frosch's suggestion on how the important changes of a network game could be visualized. Basically I think that that change can be done independent of a re-grouping of the settings windows, though; it's two related tasks, but can be done independent (and probably should - or neither is done ever)
Eagle_rainbow
Engineer
Engineer
Posts: 59
Joined: 13 Oct 2012 20:27
Location: Germany

Re: Game Settings Overview Window

Post by Eagle_rainbow »

I just saw that with rev. 24631-24633 the filter capability (by search text) in the advanced options screen was added. I just tested it and think, that it solves the search issue I mentioned above.
Currently, I am thinking/trying to implement on the other idea frosch mentioned, namely, to filter on the entries which deviate from other settings (vs. new game/default)..
Cheers,
Eagle_Rainbow

My Wiki Home (incl. overview of patches)
Eagle_rainbow
Engineer
Engineer
Posts: 59
Joined: 13 Oct 2012 20:27
Location: Germany

Re: Game Settings Overview Window

Post by Eagle_rainbow »

Hi everyone,
Having found some hours to read up to the advanced options screen, I also managed to get something running as well. I have attached a patch file plus some screenshots based on the current state where I am. The patch is against rev. 24634.
asrestriction_ddb-against-24634.patch
Advanced Setting GUI with restriction on default values etc.
(17.05 KiB) Downloaded 153 times
I also got it working in conjunction with frosch's latest enhancement, the search edit box. Also, I could test that this enhancement works very well in a local game; yet, I was not able to check it in a network game (simply didn't find the time yet).
The only little glitch that does not pleasure me yet, is that the drop down arrow of the drop down box is not being reset properly after you have chosen a value yet (see also second screenshot below). However, I think that this is something really tiny which I just don't find to fix easily as I am still quite a greenhorn... Can someone of the more experienced developers assist here from where this is coming?

Any feedback on this already?
I think it would also solve my initial request described in this thread.
Attachments
gsrm-1.png
gsrm-1.png (8.83 KiB) Viewed 5251 times
gsrm-1b.png
gsrm-1b.png (9.14 KiB) Viewed 5251 times
Cheers,
Eagle_Rainbow

My Wiki Home (incl. overview of patches)
Eagle_rainbow
Engineer
Engineer
Posts: 59
Joined: 13 Oct 2012 20:27
Location: Germany

Re: Game Settings Overview Window

Post by Eagle_rainbow »

Update: Now, I was also able to do a test in network mode - and it works like a charm.
Yet, I found the labels in the dropdown box a bit odd and changed them; here's the updated patch.
Attachments
asrestriction_ddb-v2-against-24634.patch
(17.07 KiB) Downloaded 135 times
Cheers,
Eagle_Rainbow

My Wiki Home (incl. overview of patches)
Yexo
Tycoon
Tycoon
Posts: 3663
Joined: 20 Dec 2007 12:49

Re: Game Settings Overview Window

Post by Yexo »

Nice work!

I do have some comments on the code though, most of them are minor and should be very easy to fix.

- I see no reason to move the complete ResolveVariableAddress function, just declared it near the top of the .cpp file like this:

Code: Select all

static const void *ResolveVariableAddress(const GameSettings *settings_ptr, const SettingDesc *sd);

Code: Select all

StringID restrictionMode
-While I understand your reason for doing this, I dislike it. Please create a new enum with the restriction mode and an easy way to map that enum to/from a string. It makes it a lot easier to check those functions if they implement all possible values from that enum, instead of having to check the language file with the risk of overlooking one.

Code: Select all

if ( (restrictionMode == STR_CONFIG_SETTING_RESTRICT_CHANGED_AGAINST_DEFAULT)
- The coding style says to remove the space after the first (, so

Code: Select all

if ((restrictionMode == STR_CONFIG_SETTING_RESTRICT_CHANGED_AGAINST_DEFAULT)

Code: Select all

// this entry shall only be visible, if the value deviates from its default value
- The coding styles says use /* */-style comments for comments not on the same line as code.

Code: Select all

		assert ( (this->flags & SEF_KIND_MASK) == SEF_SETTING_KIND );
- Again, space. This should be:

Code: Select all

		assert((this->flags & SEF_KIND_MASK) == SEF_SETTING_KIND);

Code: Select all

+		if (settings_ptr == &_settings_newgame)
+			// we are changing the new game's settings and shall compare against the new game's settings
+			// ==> we never will find any difference there
+			return false; // situation should not occur anyway, but we can recover from here without causing damage
- Better make sure the code really never comes there by disabling this filter in the main menu. Place an assert at this location to make sure it never happens.

Code: Select all

+			if (force_visible) {
+				visible = true;
+			} else if (!filter.IsEmpty()) {
Could be this since force_visible is already initialized to true above.

Code: Select all

			if (!force_visible && !filter.IsEmpty()) {

If nobody else objects and you fix the above issues I think this can go in trunk very soon.
Eagle_rainbow
Engineer
Engineer
Posts: 59
Joined: 13 Oct 2012 20:27
Location: Germany

Re: Game Settings Overview Window

Post by Eagle_rainbow »

Hi Yexo,
Yexo wrote:Nice work!
Thanks, especially as this is the first patch which I've submitted :)
Yexo wrote:- I see no reason to move the complete ResolveVariableAddress function, ...
done.
Yexo wrote:

Code: Select all

StringID restrictionMode
-While I understand your reason for doing this, I dislike it. Please create a new enum with the restriction mode and an easy way to map that enum to/from a string. It makes it a lot easier to check those functions if they implement all possible values from that enum, instead of having to check the language file with the risk of overlooking one.
Funny. When I came to that part, I first had implemented it with an enum, but finally refrained from doing so before uploading the patch, as I did not consider this entity to be strong enough. It was just a 1:1 mapping without additional value to me. But as you requested it, here we go :)
Please note that I have skipped implementing a RestrictionMode->StringID routine, as it would not be used actively in this coding. Thus, I did not want to introduce any unused code lines which just can date out... Moreover, this inverse direction is also really a low-brainer anyway, so that anyone may write it, if it should be become necessary at some later point in time...
Yexo wrote:

Code: Select all

if ( (restrictionMode == STR_CONFIG_SETTING_RESTRICT_CHANGED_AGAINST_DEFAULT)
... 
// this entry shall only be visible, if the value deviates from its default value
...
assert ( (this->flags & SEF_KIND_MASK) == SEF_SETTING_KIND );
:oops: sorry, on the code style thingy... In the last three weeks I have implemented stuff in three different languages (ABAP, Java and now C++) and everyone of them has a slightly different style guide... It's just too easy to mix them up.
of course, fixed as well.
Yexo wrote: - Better make sure the code really never comes there by disabling this filter in the main menu. Place an assert at this location to make sure it never happens.
Could be this since force_visible is already initialized to true above.
done those two things as well
Yexo wrote: If nobody else objects and you fix the above issues I think this can go in trunk very soon.
Both I would be glad and it would be an honor for me :)
Here's the revised version of the patch.
asrestriction_ddb-v3-against-24634.patch
revised version of patch for restricting by default values etc. in the Advanced Setting GUI
(17.77 KiB) Downloaded 150 times
Now, the only pending thing is about the hanging drop down box arrow to the right. Do you have an idea what I missed there?
Cheers,
Eagle_Rainbow

My Wiki Home (incl. overview of patches)
Yexo
Tycoon
Tycoon
Posts: 3663
Joined: 20 Dec 2007 12:49

Re: Game Settings Overview Window

Post by Yexo »

I've fixed the bug with the dropdown box. It was not really something you forget, the advanced settings gui just is a special window in that it handles the dropdown boxes itself instead of leaving it to the more generalized code in the Window class. My fix was to call the general function for your new dropdown box.

More in general I've also made some changes to the rest of your patch. The StringID->RestrictionMode function was not required, you were doing the conversion twice without realizing it. I've also done a few more changes to make the code suit the style of OpenTTDs codebase more.

You can find my changed version of your patch here: http://devs.openttd.org/~yexo/filter_changes.diff
The strings in english.txt will still have to be changed, they're not consistent with capitalization/punctuation and I think they can be written in a more clear way. This patch is now up for review by other devs. Depending on review I'll commit it later this week.
Michi_cc
OpenTTD Developer
OpenTTD Developer
Posts: 619
Joined: 14 Jun 2004 23:27
Location: Berlin, Germany
Contact:

Re: Game Settings Overview Window

Post by Michi_cc »

Yexo wrote:This patch is now up for review by other devs.
There are a couple if's with the body on a new line without braces. And I'd include a full stop in the function doxygen comments wherever there is a real sentence.

-- Michael Lutz
Yexo
Tycoon
Tycoon
Posts: 3663
Joined: 20 Dec 2007 12:49

Re: Game Settings Overview Window

Post by Yexo »

Thanks, I hope I got them all this time. Same url, new diff.
Alberth
OpenTTD Developer
OpenTTD Developer
Posts: 4763
Joined: 09 Sep 2007 05:03
Location: home

Re: Game Settings Overview Window

Post by Alberth »

Looks fine to me
Yexo
Tycoon
Tycoon
Posts: 3663
Joined: 20 Dec 2007 12:49

Re: Game Settings Overview Window

Post by Yexo »

And in trunk since r24647. Please have a look at the final commit to see what was changed since your last diff. I think the most important points are:
- if() xyz; needs to be on a single line. If split over two lines, add braces.
- Comments need capitalization and punctuation.
- Strings in english.txt were rewritten.
Eagle_rainbow
Engineer
Engineer
Posts: 59
Joined: 13 Oct 2012 20:27
Location: Germany

Re: Game Settings Overview Window

Post by Eagle_rainbow »

Thanks! I'll try putting your comments in my pipe and smoke it...
Moreover, I will check my other patch posted in the other thread right away on these learnings.
Cheers,
Eagle_Rainbow

My Wiki Home (incl. overview of patches)
User avatar
ChillCore
Tycoon
Tycoon
Posts: 2822
Joined: 04 Oct 2008 23:05
Location: Lost in spaces

Re: Game Settings Overview Window

Post by ChillCore »

After screwing up my config file, (*), I have found that it would be nice to have two more options in the dropdown menu:
- Show all settings that have a default value.
- Show all client setting whatever their value is.

The first on might perhaps be tricky though.
We do not want the setting to dissappear from the gui as soon as the value is changed, perhaps someone wanted to click an button twice or maybe selected the wrong dropdown option.


(*) calling SaveToConfig() from a button in scenario editor mode was a bad, bad idea. :mrgreen:
-- .- -.-- / - .... . / ..-. --- .-. -.-. . / -... . / .-- .. - .... / -.-- --- ..- .-.-.-
--- .... / -.-- . .- .... --..-- / .- -. -.. / .--. .-. .- .. ... . / - .... . / .-.. --- .-. -.. / ..-. --- .-. / .... . / --. .- ...- . / ..- ... / -.-. .... --- --- -.-. .... --- --- ... .-.-.- / ---... .--.

Playing with my patchpack? Ask questions on usage and report bugs in the correct thread first, please.
All included patches have been modified and are no longer 100% original.
Post Reply

Return to “OpenTTD Suggestions”

Who is online

Users browsing this forum: No registered users and 7 guests