Alberth wrote:

Here is a .png file with the new advanced setting pages, so you can look at the picture instead of read about them.
You can clearly see the groups of options which imho makes navigating easier.

Attachments

• configpatches3.png (76.45 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/2322#comment4792

Rubidium wrote:

Constants like introduced in 03-05 should be capitalized like all other constants.

Further I'm wondering whether the current 'design' (GUI wise) is the best way to show the configuration settings; it scales fairly badly for small windows (this patch seems to make it even worse). So I guess there needs to be a scrollbar when the settings window is too big for the resolution.

This comment was imported from FlySpray: https://bugs.openttd.org/task/2322#comment4941

Alberth wrote:

Second try, against r14542.

I really liked your suggestion. I was also not entirely happy with my solution, as I wanted to add more 'section-like' headings in the window instead of the small amount of white space, but that would blow up the Vehicles tab even more and I could not find a better way.
Your suggestion triggered me to come up with the solution, merging all tabs into a collapsable treeview and a scrollbar for easy moving around!!

As always, we start simple:
- 01_doxygen.patch: 4 simple doxygen additions
- 02_codechange_settingheight.patch: Introduction of SETTING_HEIGHT constant (moved this outside the window since TreeField struct will need it)
- 03_codechange_replace_xconstant.patch: Replacement of a constant with an x+offset, so the patch text will move if the base x changes
- 04_codechange_left_offset.patch: Introduction of SETTINGTREE_LEFT_OFFSET constant
- 05_codechange_top_offset.patch: Introduction of SETTINGTREE_TOP_OFFSET constant
- 06_codestyle.patch: Moved 'page' variable down to just above where it is needed first, moved declarations to first use.

Next two patches introduce DrawPatch() method. First one creates the function, second one fixes the indentation.
Note that this is not the final place of the method, it will be moved again in patch 10b.
- 07a_codechange_drawpatch.patch
- 07b_codechange_drawpatch.patch

The collapsable treeview needs TreeSection and TreeField structs.
- 10a_treeview_code.patch: All TreeSection and TreeField code, except the TreeField::DrawPatch() method.
- 10b_copy_drawpatch.patch: An non-modified copy of the DrawPatch() code to TreeField::DrawPatch(), also shifted 1 indent level (can be done here as it doesn't increase the patch-size)
- 10c_codechange_patch_offset.patch: Replaced patch setting offset numberic constant by TREE_PATCH_TEXT_OFFSET

Next step is a hostile take-over of the current advanced settings window code. Not everything works in-between patches. After 20a and 20b even compilation fails.
- 20a_treeview_global_var.patch: Modify current PatchPages entries to TreeField nodes, resulting a global _treeview var. This one only modifies the declarations, and adds a few TreeSection's (one for each tab, and a root section). Next one does a big change of the contents of all arrays. Code will not compile after applying this patch.
- 20b_treefields_fix.patch: TreeField's need a constructor call. Added around all string constants. After applying this patch, code will not compile, since we have destroyed the PatchPages.
- 20c_display_treeview.patch: To make it compile again, deleting all PatchPage stuff, and add some _treeview calls to get the tree displayed. Removed the DrawPatch() method, and commented click handling and editbox handling out temporarily.

- 21a_clickhandling.patch: Fixing click handling and editbox handling, now using the TreeField's instead of PatchEntry's. Some code is not shifted here, but is done in the next patch.
- 21b_shift_code.patch: Fixing indentation. Note that I didn't shift the code inside the switch-statement.

Everything is quite funcional again, except that the window is not wide enough anymore due to addition of the treeview indenting, and you cannot scroll up and down. This is fixed in the next patch.
- 22_widgets.patch: Change widgets + widget layout so everything fits again and is working properly.

Attachments

• 01_doxygen.patch (1.68 KiB)
• 02_codechange_settingheight.patch (1.49 KiB)
• 03_codechange_replace_xconstant.patch (0.37 KiB)
• 04_codechange_left_offset.patch (1.88 KiB)
• 05_codechange_top_offset.patch (2.16 KiB)
• 06_codestyle.patch (1.83 KiB)
• 07a_codechange_drawpatch.patch (1.25 KiB)
• 07b_codechange_drawpatch.patch (3.45 KiB)
• 10a_treeview_code.patch (15.39 KiB)
• 10b_copy_drawpatch.patch (1.88 KiB)
• 10c_codechange_patch_offset.patch (0.39 KiB)
• 20a_treeview_global_var.patch (4.79 KiB)
• 20b_treefields_fix.patch (8.54 KiB)
• 20c_display_treeview.patch (8.01 KiB)
• 21a_clickhandling.patch (6.40 KiB)
• 21b_shift_code.patch (3.93 KiB)
• 22_widgets.patch (7.04 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/2322#comment4978

Alberth wrote:

The patch shuffling done in the first attempt (patches 12-16) has not been done for the second attempt. I will add them later.
Also, could you please decide about patch 17 of the first attempt?

This comment was imported from FlySpray: https://bugs.openttd.org/task/2322#comment4979

Alberth wrote:

After a good night sleep, I think patch 20a and further are wrong in the sense that the path taken to introduce the new functionality is too abrupt. I'll try to find another path instead.

This comment was imported from FlySpray: https://bugs.openttd.org/task/2322#comment4991

Alberth wrote:

With the retirement of patches 20+, patches 10[abc] are also obsolete.

This comment was imported from FlySpray: https://bugs.openttd.org/task/2322#comment4994

Alberth wrote:

New path would be by extending PatchPage and PatchEntry.
Before doing that however, first extension of the advanced settings window with a scrollbar, a resize button, and some declarations cleanup.
These changes are after 07b, although they don't directly use that change.

Attachments

• 30_added_scrollbar.patch (6.29 KiB)
• 31_add_resizer.patch (2.99 KiB)
• 32_codestyle.patch (1.25 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/2322#comment4997

Alberth wrote:

A rather long sequence of patches to get collapsable nested sub-categories in the advanced setting sindow. Builds on top of 32 above (which bulds on top of 07b of second attempt)

First objective is to construct an interface between the settings-windoW and
the patch-page, so the former doesn't need to know how the latter works.

Moving PatchEntry & PatchPage data struct up in the file without modifications
- 33_moved_datastructs.patch

Name of the patch entry is moved from a seperate array into the PatchEntry
structure. Also, the PatchEntries become statically allocated instead of being
dynamically allocated. This is a patch in 2 parts to make it easier to understand:
- 34a_move_names.patch: Code modifications, except for the constructor call. You need to apply 34b too to make it work again
- 34b_move_names.patch: Adding of explicit TreePatch() call around all element of the (now) PatchEntry arrays.

Moving the initialization of the PatchEntries to a method call. Used this solution rather than in the constructor to prevent dependencies of initialization between load/save part, and PatchEntry arrays initialization at load time (just before main() is called).
- 35_move_init.patch

Code move of window DrawPatch() to PatchEntry::DrawPatch(). Code is not changed otherwise.
- 36_move_drawpatch.patch

PatchEntries should draw themselves
- 37_add_draw.patch

'click' window variable replaced by a pointer + some bits inside the
PatchEntry. Reason is that row numbers in a window will not mean anything any
more with collapsable sub-pages.
- 38_move_depressed.patch

'entry' variable replaced too for the same reason.
- 39_modify_entry.patch

Patch page should draw itself
- 40_move_pagedraw.patch

With recursive collapsable sub-page, an index in the main-page is meaningless. Instead we search for the right entry. It is a bit overkill now, but at 46_subpages patch, we must do this recursively for the correct result.
- 41_abstract_entries.patch

Query the page for its length
- 42_add_length.patch

Create a patch-entry kind field, and move the data of the currently existing kind into d.entry struct
- 43_entrykind.patch

Count rows before and while drawing rows
- 44_draw_row_counting.patch

Count rows to find an entry
- 45_find_entry_counting.patch

Count rows when computing length
- 46_length_counting.patch

Add 2nd kind to PatchEntry (namely a collapsable sub-page), and extend all functions with a 2nd case. Also, handle folding and unfolding in the OnClick() window code
This patch has a 'Signals' sub-page in the Construction page.
- 47_subpages.patch

At this point, everything is working, except
a) unfolded sub-pages are not drawn at a deeper nesting level, so it may be tricky to understand for the user,
b) A lot more sub-pages should be introduced, and current settings should be shuffled a bit (like in the first attempt)

Attachments

• 33_moved_datastructs.patch (2.14 KiB)
• 34a_move_names.patch (3.99 KiB)
• 34b_move_names.patch (7.63 KiB)
• 35_move_init.patch (1.67 KiB)
• 36_move_drawpatch.patch (4.28 KiB)
• 37_add_draw.patch (1.89 KiB)
• 38_move_depressed.patch (4.74 KiB)
• 39_modify_entry.patch (1.77 KiB)
• 40_move_pagedraw.patch (1.80 KiB)
• 41_abstract_entries.patch (2.39 KiB)
• 42_add_length.patch (1.46 KiB)
• 43_entrykind.patch (3.54 KiB)
• 44_draw_row_counting.patch (3.14 KiB)
• 45_find_entry_counting.patch (2.32 KiB)
• 46_length_counting.patch (0.97 KiB)
• 47_subpages.patch (5.49 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/2322#comment5023

Alberth wrote:

Two patches to fix the drawing problem mentioned above:

Adding a 'level' byte and 'last_field' bit + initialization
- 48_leveldata.patch

Drawing + handling of indented section levels is a bit more involved.
- Drawing is somewhat complicated by the need to know of all parent levels whether more fields follow at that level (stored in the parent_last integer).
- Click handling has to take the indentation into account
- Window has to be widened by 2 indent levels (2*15 = 30 pixels)
- 49_draw_indented.patch

And as a dessert, a screen-shot of the construction page with an unfolded 'Signals' sub-page.

Attachments

• 48_leveldata.patch (3.01 KiB)
• 49_draw_indented.patch (11.41 KiB)
• unfolded_signals.png (10.45 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/2322#comment5032

Alberth wrote:

Length calculation changed in 47_subpages.patch was not correct, folded sub-pages got always counted as unfolded (causing the window to think there were more rows than in reality). Fix is a two-line patch attached below.

Attachments

• 50_length_fix.patch (0.34 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/2322#comment5033

Alberth wrote:

First step, removing all tabs, and merging them all in a single page (ie an
additional top-level sub-list). The patch is split in 3 parts. The code
compiles and runs between every patch, except that the code looks unfinished.

Remove the buttons, the top-panel where the buttons are displayed in, and their 'case' in the OnClick() method.
- 51a_remove_tabs.patch

Create new sub-list titles, add PatchPage for each tab, add some comments
between the PatchEntry/PatchPage variables for easier navigating, Construct a
new _patches_main main list, and replace the old list of tabs with just the new
one.
The OnClick() code for the PATCHSEL_OPTIONSPANEL is now the only case left, so
the switch is replaced by an if. All code is dedented one TAB (and not further
modified), except for the cases of the inner switch statement which are not
moved (thus fixing the code style of this switch/case).
Obviously, the 'break' and a curly-bracket at the end is also removed.
Finally, the window needs an extra 15 pixels (TREE_WIDTH iirc) for the additional sub-list level.
- 51b_remove_tabs.patch

A list of tabs with 1 entry is not particular useful. Also the 'page' variable in the Window is not needed any more.
- 51c_remove_tabs.patch

The two PatchEntry sub-structures are moved to top-level
- 52_extract_substructs.patch

The 'Signals' sub-page was not according to name style + position of the sub-pages to come.
- 53_move_rename_signals.patch

Second step:
A sequence of option shuffling, and sub-pages creation.
- 54_displayopts_autorenew_servicing.patch
- 55_interaction_airport_town_ai.patch
- 56_cargo_industries.patch
- 57_routing_gamesettings.patch
- 58_trains_moves.patch
- 59_orders_timetables.patch

Afterwards, I checked over the entire patch-sequence (51a-59) that I didn't lose a PatchEntry with a setting in it (I didn't).

Attachments

• 51a_remove_tabs.patch (6.07 KiB)
• 51b_remove_tabs.patch (10.74 KiB)
• 51c_remove_tabs.patch (3.39 KiB)
• 52_extract_substructs.patch (1.69 KiB)
• 53_move_rename_signals.patch (1.80 KiB)
• 54_displayopts_autorenew_servicing.patch (5.16 KiB)
• 55_interaction_airport_town_ai.patch (6.88 KiB)
• 56_cargo_industries.patch (3.80 KiB)
• 57_routing_gamesettings.patch (4.00 KiB)
• 58_trains_moves.patch (3.98 KiB)
• 59_orders_timetables.patch (3.45 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/2322#comment5061

Alberth wrote:

A composed overview of all pages and sub-pages unfolded.

Attachments

• all_unfolded.png (72.11 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/2322#comment5062

Rubidium wrote:

Is there some git/hg repository so I don't need to manually download each of the patches and apply that?

This comment was imported from FlySpray: https://bugs.openttd.org/task/2322#comment5180

Rubidium closed the ticket.

Reason for closing: Implemented

Around r14975. Nice job Albert

This comment was imported from FlySpray: https://bugs.openttd.org/task/2322