#3511 closed enhancement (fixed)
[PATCH]Give the possibility not to save options for one session.
Reported by: | Stan | Owned by: | mimo |
---|---|---|---|
Priority: | Should Have | Milestone: | Alpha 20 |
Component: | UI & Simulation | Keywords: | patch |
Cc: | Patch: |
Description
As reported by mimo on IRC, currently everything single option is saved for next session.
It would be nice to ignore change for one session with a tickable option in the settings window such as
Ignore changes (Though this might be a little misleading)
This is different from the option of not saving session changes cause here it will only happen once.
Notice this setting should be always false.
Attachments (5)
Change History (26)
comment:1 by , 8 years ago
comment:2 by , 8 years ago
Keywords: | simple added |
---|
Having another button that does not reset the settings like Cancel
, but does not save would be the way to go.
comment:3 by , 8 years ago
Milestone: | Backlog → Alpha 20 |
---|
by , 8 years ago
Attachment: | patch_3511.patch added |
---|
comment:4 by , 8 years ago
Keywords: | review patch added; simple removed |
---|---|
Milestone: | Alpha 20 → Alpha 19 |
Summary: | Give the possibility not to save options for one session. → [PATCH]Give the possibility not to save options for one session. |
comment:5 by , 8 years ago
Milestone: | Alpha 19 → Alpha 20 |
---|
Thanks for the patch. Since a19 is already being packaged and this isn't a severe error, we can't include it in a19 unfortunately.
comment:6 by , 8 years ago
As this patch give us the possibility to change the settings without saving them, it would be useful to also add a button to reload the saved options.
comment:7 by , 8 years ago
Keywords: | review removed |
---|
To save for this session you do not need to write the config file at all. (This assumes no other code writes to the config file though (if some does that should likely be fixed))
Also see mimo's comment above.
comment:9 by , 8 years ago
I've uploaded a different patch which also fixes #2451 and also some cases where the graphic settings were not saved in the config file. ticket-3511-full.patch is the full patch, while ticket-3511-noswitch.patch is there to ease the review (same patch without the fix for switch indentation).
by , 8 years ago
Attachment: | ticket-3511-full.patch added |
---|
by , 8 years ago
Attachment: | ticket-3511-noswitch.patch added |
---|
comment:11 by , 8 years ago
Reload is a bit misleading, as you don't say what your reloading. Are you reloading the default values? Are you reloading initial settings? I think people would be more used to a Cancel
button.
I'm also rather unclear on what the changes to the renderer configuration type in r17625 do. As I read it, you added unreachable/duplicate code because the renderer already automatically saves its config.
follow-up: 13 comment:12 by , 8 years ago
Reloading the last saved user values (ok, we can make the tooltip more clear). After playing (and possibly screwing up) your options, i think reverting to your saved (user) options is what you want.
I don't think the renderer saves its values (at least it was not saved by the previous save button, so how and where would it be saved ?). Now, these values are saved in the user config file, so they can be restored.
follow-up: 16 comment:13 by , 8 years ago
Replying to mimo:
.... I don't think the renderer saves its values (at least it was not saved by the previous save button, so how and where would it be saved ?). Now, these values are saved in the user config file, so they can be restored.
The renderer options save fine for me right now without updating to your revision.
comment:14 by , 8 years ago
I've just tested to revert, and indeed renderer options were correctly saved. Don't know what happened, maybe a corrupted user.cfg in my tests. I will double check this tomorrow, and revert some renderer changes if not needed.
comment:15 by , 8 years ago
Thanks for working on this, the options menu really needed some update. I add here some other problems with options menu (possibly worth new tickets but reporting here currently since it still WIP) I notice.
- I would rename "Reload default values from file" to "Reload previous saved settings" and "Save changes to file" to "Save changes" or "Save settings" (avoiding the file term);
- initial settings don't match the effective renderer option. To reproduce:
- remove user.cfg (so that default value will be used)
- if under Linux with mesa drivers start the game with LIBGL_ALWAYS_SOFTWARE=1 ./0ad this should disable most graphics features (alternatively tweak hwdetecs.js to disable some settings for your card);
- start a match and notice that, for example, shadows are really disabled;
- open options menu and notice however that shadows are enabled; -> the options menu is not correctly loaded on default values for many other options.
- 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), 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 (possibly 11) scale both in options menu and 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?);
- sound settings should use a slider or a combo box rather than textual field (it is not clear what is the range currently);
- missing useful material quality setting in option menu (already filed as #2596), possibly using a slider/combo box.
comment:16 by , 8 years ago
Replying to Josh:
Replying to mimo:
.... I don't think the renderer saves its values (at least it was not saved by the previous save button, so how and where would it be saved ?). Now, these values are saved in the user config file, so they can be restored.
The renderer options save fine for me right now without updating to your revision.
In fact, I now remember why i changed it. It was not the Save, but the Cancel changes which was completely broken in the old version. Try to change some renderer options and then cancel them. Nothing will be canceled as can be checked by reopening the option menu. And that is expected because the Cancel only reload the options from the user file. That is really a (bad) consequence of having two different saved options.
With the changes, when canceling (or reloading as it is called now), i give now priority to the config options (and not the renderer ones as in the old version). But maybe it would be more consistent to always give priority to the config options and not the renderer ones.
by , 8 years ago
Attachment: | cleanOptions.diff added |
---|
comment:17 by , 8 years ago
cleanOptions.diff is a patch which revisits the logics of options.js, simplifying it and removing the need of the merge functions.
by , 8 years ago
Attachment: | cleanOptions.2.diff added |
---|
same as before, but with detection of changes to enable reload and save button only when needed
comment:18 by , 8 years ago
fabio, i've tried hacking hwdetect to start with most options disabled, and the display of options works for me. Can you check again with the latest patch (either cleanOptions.diff or cleanOptions.2.diff, the second one needs a compile), which i've used for my tests and which includes a fix for that problem.
For the four last bullets of your comment, could you open a new ticket.
comment:19 by , 8 years ago
mimo: great, I can confirm that your patch indeed fixes the value of initial settings in options menu! I tried cleanOptions.diff before and after applying it and with settings that should either initially enabled or disabled and all are OK now.
See also #3324