Opened 3 years ago

Closed 22 months ago

Last modified 22 months ago

#3737 closed enhancement (fixed)

Improvements to options menu

Reported by: fabio Owned by:
Priority: Should Have Milestone: Alpha 20
Component: UI & Simulation Keywords:
Cc: mimo Patch:

Description (last modified by elexis)

Some possible improvements to options menu. This is a follow-up of problems discussed in #3511.

  • DONE in r17739 it would be nice to have a button to "Restore default game settings. REQUIRES GAME RESTART" that will remove any settings in user.cfg (excluding the lobby login and password to avoid users losing them, userreport.id and possibly some other setting), this way users that tampered their settings have a quick way to restore the default;
  • sound settings value should probably be remapped to a 0-10 (or 11 for some fun :) scale both in options menu and all config files; the engine should then remap this values to its internal values (it may be that current values already have meaning in the scale 0-10? or are percentage with a maximum of 1.0 as their initial value between 0.2 and 0.9 suggests?);
  • refs #4039: sound settings should eventually use a slider or a combo box rather than text field (currently it is not clear what is the range);
  • some options as such Graphics Quality and Windowed Mode could support on the fly change without restart.

DONE in r17657 Also related to options menu (but already filed as #2596) materialmgr.quality setting is missing in option menu (this setting is useful to enable the best quality graphics option and actually it's the only useful setting that cannot managed with options menu but must be manually enabled editing the file), possibly to be implemented using a slider/combo box.

DONE in r17729 Finally some options are only effective if other are already enabled. E.g. "Shadow filtering" is effective only if "Shadows" is already enabled. Eventually those options should be "greyed out" or only appears if the required option is not set. But probably this is not so useful and also requires some more logic to make sure everything works OK even when manually editing options in the config files.

Some settings inside the options menu should be better organized. For example:

  • the "Lobby Settings" section currently has only "Chat Backlog" option. This section could be maybe renamed as "Multiplayer Settings" and move here "Playername (multiplayer)", "Network Warnings", "Late Observer Joins", "Observer Limit" and "Chat Timestamps" (possibly also "Nick Notification");
  • also moving "FPS Overlay" under "Graphics Settings"?

Attachments (8)

optionsReset.patch (14.1 KB) - added by mimo 3 years ago.
optionsReset-v2.patch (15.0 KB) - added by mimo 3 years ago.
rebased patch after r17657
optionsDependances.patch (9.1 KB) - added by mimo 3 years ago.
I've attached a patch which reorganizes the options, reordering them a bit as proposed in fabio's patch in #2596, and adding some dependences between them (options are now disabled when dependent on an other option which is not active).
optionsReset-v3.patch (24.8 KB) - added by mimo 3 years ago.
improved version of the patch for the reset of options
optionsReset-v3.2.patch (30.1 KB) - added by mimo 3 years ago.
include some comments from elexis and leper, + add initialization of renderer options in the configDB which simplify the later option management
writeValue.patch (9.6 KB) - added by mimo 3 years ago.
initRenderer.patch (3.5 KB) - added by mimo 3 years ago.
including elexis's comment
optionsReset-v4.patch (20.1 KB) - added by mimo 3 years ago.
updated version of the patch, after r17737 and r17738

Download all attachments as: .zip

Change History (37)

comment:1 Changed 3 years ago by fabio

Description: modified (diff)

comment:2 Changed 3 years ago by mimo

In 17645:

Rework the logics of options loading to allow more flexibility (i.e. adding a min or max value for number inputs)
Fix some problems when reloading saved options, refs #3511
Add the material quality option, refs #3737
Reload and Save buttons are only enabled when some options have changed.

Changed 3 years ago by mimo

Attachment: optionsReset.patch added

comment:3 Changed 3 years ago by mimo

The previous patch allows to reset the options. In addition, now that we can have some local options which are not saved, there is an inconsistency with some options settings at several places in the code (as for the splashscreen one) which, when set, save all the current options. The patch adds also a fix for this case, with only the new option saved on file. The case of the splashscreen is fixed, but there are other places in the code which will be fixed afterwards.

comment:4 Changed 3 years ago by mimo

Keywords: review added
Summary: Improvements to options menu[PATCH] Improvements to options menu

The previous patch allows to reset the options. In addition, now that we can have some local options which are not saved, there is an inconsistency with some options settings at several places in the code (as for the splashscreen one) which, when set, save all the current options. The patch adds also a fix for this case, with only the new option saved on file. The case of the splashscreen is fixed, but there are other places in the code which will be fixed afterwards.

Changed 3 years ago by mimo

Attachment: optionsReset-v2.patch added

rebased patch after r17657

comment:5 Changed 3 years ago by Ben Brian

I would suggest using something other than "Quit" for that dialog, typically that implies quitting the application. I think "Close" would be more appropriate.

comment:6 Changed 3 years ago by mimo

In 17671:

rename the Close button, refs #3737

comment:7 Changed 3 years ago by fabio

Description: modified (diff)

comment:8 Changed 3 years ago by mimo

We could also implement a 2 level menu as described in #1811. I've closed #1811 and copy the remaining part here to have all discussion on one place.

We could have two modes for complicated settings like graphics: basic and advanced. Basic would only show simple options that anyone can understand (graphics detail: high/medium/low), while Advanced would show all the specific settings and numbers e.g. shadow map size and filtering, AO, particles, specular, normal mapping, parallax quality, gamma. Some options might not be available on the user's system so they should be disabled, explanations are good too.

comment:9 Changed 3 years ago by gameboy

We should also add the SMAA function. Anti aliasing.

Changed 3 years ago by mimo

Attachment: optionsDependances.patch added

I've attached a patch which reorganizes the options, reordering them a bit as proposed in fabio's patch in #2596, and adding some dependences between them (options are now disabled when dependent on an other option which is not active).

comment:10 Changed 3 years ago by mimo

In 17729:

reorganize options, adding dependences, refs #3737

comment:11 Changed 3 years ago by fabio

Description: modified (diff)

Changed 3 years ago by mimo

Attachment: optionsReset-v3.patch added

improved version of the patch for the reset of options

comment:12 Changed 3 years ago by mimo

Milestone: BacklogAlpha 20

Changed 3 years ago by mimo

Attachment: optionsReset-v3.2.patch added

include some comments from elexis and leper, + add initialization of renderer options in the configDB which simplify the later option management

comment:13 Changed 3 years ago by mimo

To ease the review, I've extracted two parts which can be commited separately a) initRenderer.patch which also fills the configDB when initializing the renderer, so that the content of the configDB is consistent with the renderer b) writeValue.patch which allows to write only one value in the user.conf file, while presently when we want to add a new value in this file, all the content of the user configDB is written

Changed 3 years ago by mimo

Attachment: writeValue.patch added

comment:14 Changed 3 years ago by elexis

g_ConfigDB.SetValueString(CFG_SYSTEM, "waterfancyeffects", g_WaterFancyEffects ? "true" : "false"); Does it have to be a string? new function SetValueBoolean?

Changed 3 years ago by mimo

Attachment: initRenderer.patch added

including elexis's comment

comment:15 Changed 3 years ago by mimo

In 17737:

init configDB with renderer initial values, refs #3737

comment:16 Changed 3 years ago by mimo

In 17738:

when saving an option to file, write only that value and not all the content of the user configDB, refs #3737

Changed 3 years ago by mimo

Attachment: optionsReset-v4.patch added

updated version of the patch, after r17737 and r17738

comment:17 Changed 3 years ago by elexis

  • since we now have SetValueBool?, hwo about an IsTrue function to remove the === "true" checks?
  • warn => space and capital letter
  • reset changes -> "set defaults" might be a better name
  • if (Engine["Renderer_Get" + keyRenderer + "Enabled"]() !== checked) not needed (see CRenderer::SetOptionBool) (the variable above becomes unneeded too)
  • CConfigDB::RemoveValue -> remove_if if you want to try something new and somewhat ugly
Last edited 3 years ago by elexis (previous) (diff)

comment:18 Changed 3 years ago by mimo

In 17739:

allows the user to restore the default game options, refs #3737

comment:19 Changed 3 years ago by mimo

Keywords: review removed
Milestone: Alpha 20Backlog
Summary: [PATCH] Improvements to options menuImprovements to options menu

Put it back to backlog, waiting for the remaining improvments to be implemented

comment:20 Changed 3 years ago by fabio

Description: modified (diff)

comment:21 Changed 3 years ago by fabio

I get the following:

Incompatible renderer option value for WaterShadow

To reproduce:

  • select Reset and restart game;
  • enable all (2) missing water options, save and restart;
  • open option menu and notice the warning.

comment:22 Changed 3 years ago by mimo

In 17744:

fix typos in renderer's initialization, refs #3737

comment:23 Changed 3 years ago by mimo

Thanks for the tests. It seems this effect was never settable by options because of old typos, and nobody never noticed it. Should be fixed now.

comment:24 Changed 3 years ago by Imarok

refs #4039

comment:25 Changed 3 years ago by fabio

Description: modified (diff)

comment:26 Changed 3 years ago by fabio

Description: modified (diff)

comment:27 Changed 22 months ago by elexis

Component: Core engineUI & Simulation
Description: modified (diff)
Milestone: BacklogAlpha 20
Resolution: fixed
Status: newclosed

The rework was great!

Rearrangement of options is still in mind but can be well planned separately. (fpre has a patch on phabricator for that too) The scale of the sound settings is not really relevant anymore as we now have the sliders. Removing the "REQUIRES GAME RESTART" part should be done where possible, but it might be better to write separate tickets for that.

comment:28 Changed 22 months ago by elexis

In 20010:

Rename WaterUgly? setting to WaterEffects? in order to remove the invertedboolean workaround of the options page.

Differential Revision: https://code.wildfiregames.com/D815
Refs #3737
Tested By: Stan

comment:29 Changed 22 months ago by elexis

In 20101:

Use tabs for the options page and rewrite it altogether.

Removes a lot of JS and XML duplication, unneeded for-loops.
Display min/max in the tooltip for number inputs.
Increase the width for dropdowns and dynamically maximize the width of the label.
Remember the selected tab when hotloading the page.

Differential Revision: https://code.wildfiregames.com/D805
Refs #3737
Graphical design agreed with: scythetwirler, mimo, Itms, fpre, Imarok, bb, leper, brian, feneur, implodedok
Testing By: Vladislav, mimo

Note: See TracTickets for help on using tickets.