[PATCH] Add a function to manipulate selected_column_order and selected_column of COList

[Imarok]

the parameters would be name and selected_column_order or selected_column. getting the specific object could maybe done by IGUIObject* guiObj = g_GUI->FindObjectByName(name); An alternative would be to add a function that refreshes the COList This is needed for #3742

[elexis]

Problem: When setting `selected_column_order" via JS, the GUI arrows aren't updated:

The COList.cpp should be notified implicitly and update the graphics without a new function ideally.

[elexis]

About the existing code:

• The name m_SelectedDef is terrible and should be renamed to m_SelectedColumn.
• Someone write a simple ticket about that default style TODO that is copied to most GUI objects
• It seems those COList bugs come from the COList implementation not being cleanly / completely being implemented when the features were added. This should be compensated by cleaning up the mess instead of supplementing checks for some edge cases.

About the patch:

• Should become consistent, either substr(5) or "list_" + ... everywhere
• Messy code appearance: Use m_SelectedDef and m_SelectedColumnOrder instead of introducing selected column and selectedColumnOrder . The check selectedColumn.size() != 0 seems inconsistent, as the other code checks for m_SelectedDef != (size_t)-1 It were cleaner if the member variables would just be updated on GUIM_SETTINGS_UPDATED, so it doesn't have to fetch the setting again whenever it wants to use it.
• If the code won't become more readable, it should have a comment about what the code is doing there

[Imarok]

Rename m_SelectedDef to m_SelectedColumn. Update on GUIM_SETTINGS_UPDATED

[Imarok]

renaming ObjectDef and m_ObjectsDefs. Change one if to an else if

[Imarok]

Remove every "def". Store the selected column string instead of the selected column index. Some cleanup

[elexis]

• TODO 1: Do we need default_column and default_column_order at all if we can just use selected_column and selected_column_order instead from both JS and XML?

Implementing some but not all settings on GUIM_SETTINGS_UPDATED seems incomplete. However it is complete, as DrawList gets the current setting value right away by using GetSettingPointer and sortable also using GetSetting on GUIM_MOUSE_PRESS_LEFT. Only m_SelectedColumn and m_SelectedColumnOrder are bugging. Since all the other code calls GetSetting again and again and thus avoids the described bug,

• TODO 2: it would be cleaner to remove these member variables altogether. (single source of truth)

The code from L88 to L99 could be removed entirely probably.

• Independent cleanup: The repetition of GetScrollBar(0) seems ugly. Instead m_ScrollBars[index] should become m_VerticalSrollBar (and m_HorizontalScrollbar in the future), since there likely won't be more than 2 scrollbars. The CRect rect assignment could be moved some lines up to avoid that other GetListRect call.

• Independent cleanup: Having m_HeadingHeight hardcoded as 30 seems bad, should become a Setting just like the other settings. The member variable should be nuked then as well.

[Imarok]

Made all changes proposed in comment:7 besides the GetScrollBar(0) thing. COList don't requires a column "name" anymore. Some cleanup.

[Imarok]

Through a warning when no selected_column_order is set. Fix some oversights in lobby.xml

[elexis]

In 18652:

Major ordered list GUI cleanup. Patch by Imarok, fixes #3905.

Remove m_SelectedDef, m_SelectedColumnOrder and m_HeadingHeight. Use GetSetting and SetSetting instead. Thus
implement selecting a column / column order sprite from JS correctly.
Since the actual list sorting is done in JS, only the column header sprites were inaccurate.

Remove "selected_def" since that is redundant with "selected_column".
Merge "selected_column" with "default_column" (and "selected_column_order" with "default_column_order") since
all properties provided in XML pages are defaults and to remove the copying on init.

Rename "def" to "column" and "ObjectDef" to "COListColumn".
Replace 30px header height hardcoded in the C++ with an option set by the style.
Remove the unneeded requirement to specify a "list_name".
Thus rename "list_name" to "list_month" in the replay menu.
Remove unneeded "list_" prefix and substr(5) in column checks.
Use ranged loops.
Fix typo "Avalible".
Add an error message if the GUI style doesn't specify the selected column order.

[elexis]

Thanks for taking care of this bug that was deeply buried in weird code!

[Imarok]

Do not show the sort sprites (little arrows) when list is not sortable.

[elexis]

In 18694:

Supress an error warning that isn't valid for unsortable ordered lists and hide sorting sprites if sorting is disabled. Based on patch by Imarok, refs #3905.

[elexis]

• Thanks user1 for reporting the error.
• Nice cleanup Imarok, we only need one sprite variable indeed!
• Therefore nuking sprite_not_sorted too
• Thus being able to flip the selected_column_order and selected_column call nicely
• Moving the // Draw column headers comment a bit below, so that those 4 setting calls are grouped
• (Not sure if most compilers were smart enough to optimize out a string variable, if one were to remove the duplicate GetSettingPointer calls)
• Rephrasing comment If the list sorted by current column -> adding an "is" and "the"
• trailing whitespace
• removing unneeded comment
• moved the drawsprites comment
• adding few newline