Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use XML to save/load options. #562

Open
kianzarrin opened this issue Nov 22, 2019 · 12 comments · May be fixed by #1704
Open

Use XML to save/load options. #562

kianzarrin opened this issue Nov 22, 2019 · 12 comments · May be fixed by #1704
Assignees
Labels
Blocking Another issue depends on this issue. code cleanup Refactor code, remove old code, improve maintainability enhancement Improve existing feature Settings Road config, mod options, config xml technical Tasks that need to be performed in order to improve quality and maintainability UI User interface updates

Comments

@kianzarrin
Copy link
Collaborator

kianzarrin commented Nov 22, 2019

I think it would be more organised to save/load data not by index but rather by dictionary keys (eg use XML). We should also have a class for backward compatibility to load from old saves.

Also It would be nice if we add a button to 'use current values for new games' like the real time mode (#363):
Screenshot (87)

@kianzarrin kianzarrin added feature A new distinct feature triage Awaiting issue categorisation labels Nov 22, 2019
@kianzarrin
Copy link
Collaborator Author

The fact that there is a lot of repeated code around is not helping the cause. Encapsulation can reduce a lot of such redundancies.

@originalfoo
Copy link
Member

originalfoo commented Nov 22, 2019

It would be nice if we add a button to 'use current values for new games' like the real time mode

Yes, I would love this, and it's something users have asked for many times over the years.

The fact that there is a lot of repeated code around is not helping the cause. Encapsulation can reduce a lot of such redundancies.

If we adopt something similar to what Real Time does, it has a central method that deals with config version compatibility so it's really clear what changes from version to version.

I guess the big chore will be making it backwards compatible (ie. load old versions of TM:PE settings that would be completely different format, and there are several old versions of settings).

@kvakvs recently split the mod options in to separate files for the various tabs as part of multiple massive code clean-ups, so he probably knows most about the current system if you need infos on that.

@originalfoo originalfoo added code cleanup Refactor code, remove old code, improve maintainability Settings Road config, mod options, config xml technical Tasks that need to be performed in order to improve quality and maintainability UI User interface updates enhancement Improve existing feature and removed feature A new distinct feature triage Awaiting issue categorisation labels Nov 22, 2019
@kianzarrin kianzarrin changed the title Improvements on saving loading options Improvements on saving/loading options Nov 22, 2019
@kianzarrin
Copy link
Collaborator Author

kianzarrin commented Nov 23, 2019

@aubergine10

@kvakvs recently split the mod options in to separate files for the various tabs as part of multiple massive code clean-ups, so he probably knows most about the current system if you need infos on that.

is there any issue regarding this. I didn't manage to find any. I'd like to @kvakvs code.

EDIT: Wait are you taking about the code that is already in master/origin ?

@originalfoo
Copy link
Member

Yes, the code that is already in master branch is the latest which includes the huge cleanup that kvakvs did some months ago.

I was meaning that if you have any questions on any of the config stuff, kvakvs is probably best person to ask as he had to pick through a lot of that stuff during the code cleanup.

@kianzarrin
Copy link
Collaborator Author

@aubergine10 lets move the discussion #1267 to here:

Example: Go look at GlobalConfig.Instance.Main.ScanForKnownIncompatibleModsAtStartup vs. Options.scanForKnownIncompatibleModsEnabled and how they both interact.

that sounds unreasonable. Is this a bug?

Maybe we should move them to options csv file for consistency.
First I'd need to know why it's done that way currently.

Providing support for alternate translation is no big deal. I can give the option for user to enter the translated label or provide translation delegate. but first we need to see if we really need it.

Notable example is the GlobalConfig.Instance.Main.DisplaySpeedLimitsMph option which has locale Translation.SpeedLimits.Get("Checkbox:Display speed limits mph").

@krzychu124 @kvakvs @DaEgi01 Do you think we should put all option translations under options.csv or do you think some options should be under speed limit csv?

@kianzarrin
Copy link
Collaborator Author

kianzarrin commented Jan 7, 2022

@aubergine10 some stuff about the CheckboxOption that you might be already aware of but I say it anyway just in case:

  • you can add your own events to run after the default event. eg: Handler = UpdateDedicatedTurningLanePolicyHandler,
    • you can also remove drop the default handler AFTER instantiation if you want (but I don't think if there is any use case.)
  • this is TRANSITIONAL code. ultimately I want to drop the Options class and use the SerializableUIOptionBase as single source of truth. I want to use xml both for in game stuff and global stuff. (basically what this issue says we should do)

As for global VS in-game: some options are global only. In future if we change in game options from main menu it should be used as basis for new game.

@kvakvs
Copy link
Collaborator

kvakvs commented Jan 7, 2022

@krzychu124 @kvakvs @DaEgi01 Do you think we should put all option translations under options.csv or do you think some options should be under speed limit csv?

The speed limits option was used in both Options tab, and in Speed limits UI. This might be not true anymore. But the hassle of moving string with all translations... doing that via CSV editor instead of Crowdin...

If its still used in both places the only solution that would fix your concern, would be duplicating string in both files. If its now only used in options, then you can safely move it, no problem.

@kianzarrin
Copy link
Collaborator Author

. But the hassle of moving string with all translations... doing that via CSV editor instead of Crowdin...

is it really that hard?

I mean if there is a valid reason for this then that is OK but if this is just a bug then I think its better to solve it rather than to find work around.

@originalfoo
Copy link
Member

IMO, all translations relating to options should be in Options.csv - but we can sort that out as subsequent task.

I'm currently working on porting all the options to the new UI format (eg. CheckboxOption) and will send that in as subsequent PR after #1267.

For persistency, there's 3 tangible situations:

  • None (eg. debug options which could be set in-game via a UI)
  • Global-only (eg. Language setting)
  • Global and Savegame (most settings)

(A fourth would be Savegame-only but I can't see any situation where that would be required)

Tagging: #1268 , #1262 , #1240 , #813 , #689 , #659 , #363 , #281 , #190 , #69 , #62

@originalfoo
Copy link
Member

originalfoo commented Jan 9, 2022

So, I've been chipping away at a mock of how things might look if we approched settings the same way as Real Time mod, and I don't think that's going to work for us:

  1. Many of our settings need to invoke code when changed; the attribute-driven UI approach isn't particularly good for that
    • I know we can use a naming convention to automagically invoke change handlers, but that's still icky imo
  2. We have vastly more settings - not just those shown on options tab, but also all the little microadjustment settings in global config, and the debug settings too
  3. We could potentially end up with 3 settings screens:
    • Mod options
    • Advanced stuff - a UI for micro settings in global config
    • Debug - eg. setting debug switches via in-game UI without need to fiddle with config xml
  4. We have some settings that don't persist (debug stuff), some that should always be global (eg. language), some that should be game/global depending on where user changes them

My suggestion would be an attribute that specifies persistence type (no attribute = no persistance), for example:

[OptionPersistance(Persist.GlobalAndSavegame)]
public bool someSetting { get; set; }

If UI updates that option via in-game pause menu, the change only gets stored in save game on next save / autosave. If they change it via main menu, it would immediately persist to global config.

If the setting were Persist.GlobalOnly then whenever it changes (via UI) it should immediately persist to global config, regardless of whether user is in-game or not.

Persistance would generally be triggered by UI which handles the change. The UI should be separate, eg. each tab in mod options has it's own file using the new UI stuff such as CheckboxOption.

Does that match more or less what you were thinking?

@kianzarrin
Copy link
Collaborator Author

I don't think our code should look like real-time mod. just that we should have the functionality to set options for new game.

@originalfoo
Copy link
Member

Yup, like I said in previous comment, I don't think the Real Time approach will work for us.

@kianzarrin kianzarrin added duplicate Duplicate of existing issue and removed duplicate Duplicate of existing issue labels Apr 3, 2022
@kianzarrin kianzarrin changed the title Improvements on saving/loading options Use XML to save/load options. Apr 3, 2022
@kianzarrin kianzarrin added the Blocking Another issue depends on this issue. label Apr 4, 2022
@kianzarrin kianzarrin self-assigned this Dec 2, 2022
@kianzarrin kianzarrin linked a pull request Dec 2, 2022 that will close this issue
@kianzarrin kianzarrin linked a pull request Dec 2, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocking Another issue depends on this issue. code cleanup Refactor code, remove old code, improve maintainability enhancement Improve existing feature Settings Road config, mod options, config xml technical Tasks that need to be performed in order to improve quality and maintainability UI User interface updates
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants