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

Serialization of FlexibleConfig #2893

Open
vampcat opened this issue Apr 10, 2017 · 15 comments
Open

Serialization of FlexibleConfig #2893

vampcat opened this issue Apr 10, 2017 · 15 comments
Labels
Topic: Architecture Requests, Issues and Changes related to software architecture, programming patterns, etc.

Comments

@vampcat
Copy link
Contributor

vampcat commented Apr 10, 2017

With PR #2757 almost ready to merge, it's time we start looking at the future of FlexibleConfig.

The currently open PR only deals with the actual implementation of FC, and not the storage/retrieval of data from the disk. This aspect is of utmost importance before this class can be used for practical purposes, since this is what makes settings persist across game sessions.

Here are the descriptions of a number of different, co-existing FCs:

  • global engine settings: these are settings currently stored in classes such as RenderingConfig and RenderingDebugConfig. An FC instance automatically stores the engine's hardcoded setting values and any value the user might have changed from the hardcoded ones. We'd want to store to disk only the user-customized ones in a *.cfg file in the user's home folder.
  • global module settings: these are settings that are specific to a module. Similarly to the global engine settings a module setting FC stores at runtime both the module's hardcoded defaults and any user-customized setting. However, only the user customized setting would make it into a file, perhaps in .../homeFolder/ModulesConfigurations/moduleName.cfg" - to be debated.
  • per-saved-game settings: a user should be able to override hardcoded/customized global engine/module setting within the context of a specific game. The overridden settings would be saved within the saved game file or within a parallel file.

Interestingly, the last description provides hints to previously unforeseen challenges. How do we connect one or more global FCs to one specific to the currently running game, so that the game-specific one simply defers to the global one for settings that the user has not yet customized? Or would this be the wrong approach? Should we instead make simple deep-copies of global FCs and use those within games, no dynamic linking involved? Or should the Setting class be expanded to always accommodate an hardcoded default value, a customized global value (if any) and a per-game value (if any)?

These are issues to be considered and also determine the path for the eventual development of a NUI frontend for FCs, as discussed in #2894.

Lastly, but compatibly with human resources, ideally this should be at least partially implemented before the start of GSoC, so that this class can be used during the work period. Specifically, it could be possible to at least implement global engine/module FCs for the benefit of GSOC projects but leave per-saved-game FCs at a lower priority.

Pinging @eviltak @emanuele3d @Cervator for feedback, and suggestions.

@emanuele3d emanuele3d added the Topic: Architecture Requests, Issues and Changes related to software architecture, programming patterns, etc. label Apr 10, 2017
This was referenced Apr 15, 2017
@eviltak
Copy link
Member

eviltak commented Apr 18, 2017

I propose keeping unique top-level FCs, instead of storing them per save game.

The idea is that the user (UI) should be able to define from which scope (of the three different Setting scopes, namely global engine, global module and per save game) a Setting should be loaded from and to which scope a modified Setting should be written (both preferably on a per-Setting basis), while keeping everything hidden from FC consuming engine code and third party modules, and preventing FC consumers to "hack" around Settings by using a Setting from a different Setting scope.

This relies on the making a single FlexibleConfig manage all Settings of a specific type/usage rather than having multiple uncoordinated and unlinked FCs manage similar settings of different scopes but of the same type/usage.

This may also require each FlexibleConfig to have its own unique ID, so that Settings can keep track of the FlexibleConfig they belong to.

The way I'm envisioning this is that during load time, the FlexibleConfig/Setting deserializer will look for the FCs' settings in the three different setting scopes, namely global engine, global module and per save game (ordered with increasing priority). If Settings with the same ID are present in multiple scopes, only the Setting in the scope with higher priority will be loaded into the FC, unless the desired scope is explicitly stated by the user.

At all places in the UI where a Setting can be modified, the options of

  • changing the Setting scope to which the modified Setting should be written
  • changing the Setting scope from which the Setting should be read

should be available. (These two options could potentially be merged.)

To help illustrate this further, consider the following example.

An engine FlexibleConfig defines a setting "SettingA". The user overrides "SettingA" in the save game scope, leaves its scope setting to the default (i.e. Settings are accessed from scopes according to scope priority) and saves the setting.

As soon as the setting is saved (serialized) on button click, "SettingA" is retrieved from the desired scope (which is "save game" (highest priority) in this case) and stored into the FC, replacing the value before the modification and notifying SettingA's subscribers. From this point on, any calls to SettingA's getValue method will return the "save game" scope's value of SettingA. The procedure will be similar for a Setting scope change.

Ping @emanuele3d @vampcat @Cervator

@vampcat
Copy link
Contributor Author

vampcat commented Apr 18, 2017

I forsee a lot of confusion and mixing of terms for the 3 levels of the FCs, so for the purpose of this post, I'll be calling the 3 levels Global, Module and Save, although the final names will probably be slightly different.

Unless I'm mistaken, having a single top level FC won't work, since we're hoping to provide 3 different levels for each setting. Unless we're willing to hack around the settingName to things like settingName_GLOBAL, settingName_MODULE and settingName_SAVE to save all 3 levels in one FC, 3 diffrent FCs are a better option.

That being said, I don't recommend we use 3 raw FCs either. I was planning on throwing together a PublishableSettings class, that:

  • Contains 3 different FCs, for all the different levels.
  • Has various getGlobal, setGlobal, getModule, setModule, setSave, getSave accessors that accept a single settingName argument, which will only be used by the front-end and the serializer to retrieve/update the values in the respective FCs.
  • Has a much more useful getSetting function that again takes in a single settingName argument, and returns the correct value by prioritizing Save, then Module and then Global. Note that this is the function that will be used by modules and other pieces of code that need to retrieve the value of the setting.

@eviltak This is in line with a lot of your points like "keeping everything hidden from FC consuming engine code and third party modules, and preventing FC consumers to "hack" around Settings by using a Setting from a different Setting scope", but takes a much.. simpler approach.

Now, onto finer points of your post.
"FlexibleConfig to have its own unique ID": If we need to store the FC that manages a setting, we can just store a reference. Why the ID?
Ideally, we'll only have one single PublishableSettings that stores all rendering-related settings, and when (de)serializing the FC, the path should be enough to inform the engine which FC/PublishableSettings a particular settings belongs to.

"(These two options could potentially be merged.)"
I'm hoping that they will be merged. A lot of visual clutter otherwise, and the way I'm seeing it, a single combo-box to set the level, coupled with the proper widget (slider, combo-box, input field) to set the value of setting should suffice for every setting, while providing complete control to the user.

Regarding the saving : The serizalization of an FC will store all it's contents to the disk, as they are.
The serialization of a PublishableSetting will involve serialization of the 3 constituent FCs, so it's pretty trivial.

If you feel I missed out something or I'm wrong on any count, please do let me know :)

@Cervator
Copy link
Member

Posting just to comment that I read it all, and think I understand it ... but I don't have a hugely strong opinion on it so far with what's been posted. Sounds like the ideal target is being narrowed in on. Will keep an eye out for further details and maybe some initial code sample work :-)

@eviltak
Copy link
Member

eviltak commented Apr 19, 2017

@vampcat I think you may not have fully understood my proposal (which I will elaborate further later in this post).

I see two major problems with your proposal:

  1. There are accessor methods for each setting Scope, in addition to a getSetting method which all consumers are expected to use, which contradicts my statement of keeping everything hidden from FC consumers.
    How can we ensure that consumers use only the getSetting method, and not any of the other accessors? To my knowledge, the only way to present only the getSetting method to consumers but allow the UI to use all the other accessors is by checking the type of the caller, which fully satisfies the definition of a hack.

  2. Another big problem with using different settings and FCs for each scope is managing setting subscribers. Which setting should a subscriber subscribe to, the Global FC, the Module FC or the Save FC? How are subscribers notified when a Setting scope changes? The only way to work around this is by adding the same subscriber to the setting in each scope, which is just one of the many redundancies caused by maintaining different FlexibleConfig/Setting objects for similar settings in each scope.


On the other hand, my proposal does not require

  • setting IDs to have scope identification suffixes like "_GLOBAL", "_MODULE" and "_SAVE"
  • the FlexibleConfig class (or even a different class for that matter) to contain different accessors for retrieving settings in different scopes as you propose
  • any other kind of identification or segregation for different Setting scopes.

Both your proposed approaches (I agree, the first one is more of a hack) do not "keep everything hidden from FC consuming engine code and third party modules and prevent FC consumers to 'hack' around Settings by using a Setting from a different Setting scope", since the settings in various scopes are still accessible in some way or the other.

My approach is much simpler than you think.

It doesn't require any modification of the FlexibleConfig and Setting implementations as implemented in #2757, nor does it involve adding extra boilerplate to store and handle settings in the various scopes.

Settings are stored in FCs normally, without any knowledge of which scope they belong to (be it through an ID suffix, a field, or any other mechanism) and without having multiple FCs containing similar settings but of different scopes. It is the (de)serializer that manages (manages is a loose term) the setting scopes.

The workflow will be as follows:

  1. A setting (and potentially a FC) will be defined in a FC by the engine/module in a startup method, which is run before FC deserialization. After definition, the setting will contain the hardcoded value (which can be changed), setting default value and value validator (both cannot be changed).
  2. When loading a setting (during FC deserialization), the deserializer will look for the serialized setting in each of the three Setting scopes' paths. If the setting was found in the desired scope (if not stated by the user, the highest priority scope is used), it will update the setting in the FC (the setting was already created in step 1) with the deserialized value and validator. If the setting wasn't found in any scope, do nothing. The setting will contain the hardcoded value.
  3. When modifying a setting, the setting is stored (serialized) in the desired scope's path as stated by the user. Now that the setting has been modified, the value in the setting is updated (which is just one line of code, BTW) to the new value even if the desired setting scope was changed. This is done because once the value in a setting is updated or scope of a setting is changed through the UI (ideally on a "Save" button click), we have no use for the previous value, be it of the same or a different scope.
  4. If the user simply changes a setting's desired scope without updating the value, the setting is simply loaded from the desired scope and the value of the setting is updated to the value of the setting in the new scope.

In the 3rd and 4th steps we overwrite the values in the setting because there is only one of two possibilities:

  1. If the desired scope of the setting was changed, the value in the previous desired scope will have already been written to disk, and an overwrite causes no harm.
  2. If the desired scope of the setting was not changed, the user meant to overwrite the setting's previous value.

Regarding the usage of unique IDs for FlexibleConfigs, it will be needed for storage and retrieval of various FlexibleConfigs created by the engine and third party modules, maybe through a class FlexibleConfigStore that uses a Map internally, but is out of scope for this issue.

I hope that makes it more clear.

@vampcat
Copy link
Contributor Author

vampcat commented Apr 19, 2017

That explanation helps clear things out a lot.

First, to address your points:

"How can we ensure that consumers use only the getSetting method, and not any of the other accessors?"
The expected users are modules. Terasology uses the @API annotations to expose only certain methods, and making only the getSetting() an API function should take care of that.

"Which setting should a subscriber subscribe to, the Global FC, the Module FC or the Save FC?"
No, it will only subscribe to the PublishableSettings class, and the class will take care of the rest. This also allows us to call the subscribee according to what we need - only when the final observable setting (Global + Module + Save) changes, when any individual FC changes and so on.

"Both your proposed approaches (I agree, the first one is more of a hack) do not "keep everything hidden from FC consuming engine code and third party modules and prevent FC consumers to 'hack' around Settings by using a Setting from a different Setting scope", since the settings in various scopes are still accessible in some way or the other."
I only propose one method, the other was just something I threw in as a comparison. However, this point stands nullified, because of the @API. However, since I forgot that someone reading this might not be familiar with that part of the code, it's my fault for not mentioning it explicitly.

That being said, I can certainly see the appeal of your approach, and would be more than happy to go with it. A few questions/comments though:

"If the user simply changes a setting's desired scope without updating the value, the setting is simply loaded from the desired scope and the value of the setting is updated to the value of the setting in the new scope"
Imagine the user has a setting (say appleColor) as blue in Save scope, and pink in Module scope. He's initially using Save scope, and decides to change the scope to Module, which makes his apple pink. Then, he exits the game and starts it again.. and finds a blue apple, because that value still exists and is prioritised by the de-serializer. The factor missing here is, changing scope also involves deleting the value in current scope, if we're moving from scope with higher priority to one with lower priority.

"Regarding the usage of unique IDs for FlexibleConfigs, it will be needed for storage and retrieval of various FlexibleConfigs created by the engine and third party modules, maybe through a class FlexibleConfigStore that uses a Map internally, but is out of scope for this issue."
I might be wrong, but I think @emanuele3d wants only one single FC to store the settings of all Modules, and I agree with him on this count. We won't be giving Module Developers the option to create their own FC (I see no reason to whitelist it, so they literally CANNOT use the class). That means the only FCs we'll have are the ones maintained by engine: the RenderingSettingsFC, maybe PlayerSettingsFC someday and so on. These will be stored in a map, certainly. I can totally imagine the RenderingSettingsFC being accessed via FlexibleConfigStore.get("rendering"); or something, but I still see no need to store the ID INSIDE the FC.

@eviltak
Copy link
Member

eviltak commented Apr 20, 2017

Imagine the user has a setting (say appleColor) as blue in Save scope, and pink in Module scope. He's initially using Save scope, and decides to change the scope to Module, which makes his apple pink. Then, he exits the game and starts it again.. and finds a blue apple, because that value still exists and is prioritised by the de-serializer. The factor missing here is, changing scope also involves deleting the value in current scope, if we're moving from scope with higher priority to one with lower priority.

That's the reason I mention a setting's "desired scope" everywhere. We will need to store a simple value (even an integer works) designating the setting's desired scope along with the setting (this would have been a requirement even when using another approach), which the deserializer takes into account if specified or if a value is available in the desired scope. If not, the deserializer falls back to the priority order.

@vampcat I see what you're getting at regarding the usage of FlexibleConfig. I have one question though. How will setting ID collisions be dealt with if each module will be using a single FC to store its settings? I think a better approach would be to use unique FCs internally for each module, without exposing any details about the FC to the module. This will help in a number of ways:

  • It will act as a sandbox, i.e. no module will be able to access and change another module's settings (I can't think of a use case for this unless there's a need for poorly done module interop, if that should even be allowed)
  • It will allow for easier segregation of settings for each module after serialization, in case we want to add something like exporting module specific configurations and will make it easier for module specific configs to be "shared" (for example recommended settings for high, medium and low end systems for a graphics enhancement module shared by the module developer)
  • It will prevent setting ID collisions (unlikely but still possible)

Modules will/will not be given access to the engine FCs, as decided upon later.

The engine will manage a FC for each loaded module that requests one through a class like FlexibleConfigStore.

@vampcat
Copy link
Contributor Author

vampcat commented Apr 20, 2017

"this would have been a requirement even when using another approach"
Not really. For instance, in your approach and in my earlier approach, we can just delete the higher priority (Save scope) value, making the system fallback on lower priority node (Module scope), eliminating the need of storing the scopeSelector (an int or whatever).
However, your approach has the added benefit of not destroying the value of Save scope variable when switching to Module scope, and I appreciate that.. although I do see a pitfall.

Consider SnowWhite module again. In one saved game instance, we use Save scope value. In another, we use Module scope. In another, Global scope. That means, we're saving the scopeSelector variable on a per-game basis.
Then, I make a new SnowWhite game. If I first enabled Module then Global, the scopeSelector would be Global, with the Module scope having the last value. Ideally, this would work.. but the scopeSelector is save-instance specific, so what scopeSelector will new instance use? Do we then add a new defaultScopeSelector for each module, to initialise scopeSelector in new instances?

Too many complications, imho.

@emanuele3d : i would love to hear what you have to day about a single FC vs PublishableSettings, and deleting higher priority value vs using scopeSelector.

"How will setting ID collisions be dealt with if each module will be using a single FC to store its settings?"
The settingName is a fully qualified string referring to the setting. So, simpleUri of our appleColor would be "snowWhite:appleColor".

Regarding sandboxing, if we need to, we can implement simple checks that compare the module name and the simpleUri of setting requested, if we really want to.

"It will prevent setting ID collisions (unlikely but still possible)"
Without proper qualification of settingName or different FCs, it's not unlikely, it's inevitable.

"It will allow for easier segregation of settings for each module after serialization"
I plan on segregating the settings for each module either way, into it's own file. Multiple FCs aren't required for that.

With all that said, I can work with either global FCs or module-specific FCs, so unless I find a reason to swing in either direction I'll leave the choice upto manu.

@emanuele3d
Copy link
Contributor

Just a few quick notes until I find the time to respond more in depth.

  1. keep in mind the lifecycle of settings. Modules settings do not exist until a game is loaded/created. Per-save settings do not exist until a game is loaded. And ideally no setting instance should exist unless it overrides a higher level scope. Granted, the last assertion depends a lot on the actual architecture.

  2. FCs so far have been thought as a replacement for the hardcoded configs we currently have, i.e. the RenderingConfig. The current pattern for these config is that they get stored within the Context, which is a <class, instance> map. I wouldn't change this aspect: the future RenderingConfig (a class/instance extending FlexibleConfig) would be stored the same way unless we find good reasons to do otherwise. Notice that I have considered storing -ALL- settings in a single, big FC. It is theoretically possible given the scoped setting IDs. But it probably won't hurt to keep settings more segregated, in various FCs.

  3. At this stage I do not see why Modules shouldn't be allowed to create their own FCs.

  4. From a usability perspective, for both engine developers, module developers and even users, it's important to keep things reasonably simple, i.e. hiding as much complex functionality as possible. For example, we can make assumptions depending on the context. I.e. if a user modifies a setting's value when no game is loaded, we can assume he intends to modify the global setting. Conversely, when a user modifies a value during a game, it can be assumed it's a per-save change. This eliminates the problem of the writing scope: it's defined by the context the user it's in.

I'll get back to you with more in the next few days.

@Cervator
Copy link
Member

Cervator commented May 7, 2017

Quick scope note (sort of re: @emanuele3d's 1st point): one of the GSOC items and/or part of what @Vizaxo might end up working on includes enhancing the world setup phases to include a preview/config stage before you fully create a world. Similar stuff could be done for saved worlds (that item didn't get a slot though - but maybe unofficial work?)

Existing world preview/details has this to a point, where configurable settings in a chosen world generator from a module is exposed in the UI before you enter the world. But I think that's more stuck in the world generator system as a one-time thing to expose rather than any sort of persisted config.

Just another thing to keep in mind, not otherwise trying to make a point :-)

@emanuele3d
Copy link
Contributor

@Cervator: good point. In my 1st point I sort of included world gen in the concept of Module settings not existing until a game is loaded/created, but I can see it was too subtle a distinction.

Indeed module settings (at this stage) begin to exist as soon as a game is being configured for the first time, which includes, for example, world generation. But there is a gap between the time when the game is being configured for the first time and the time the player is actually in-game. This gap does not exist when a game is loaded instead.

I think the key will be to restrict the scope of the setting changes depending on the user context.

  • Changing a setting when no game is active would be done through a UI labeled, for example, "Global Rendering Setting". This UI would only change global settings - no modules involved here. In this UI the user would be given the choice between using the hardcoded default or a value of her choice.
  • Again when no game is active, but assuming a screen containing a list of modules, the user would be able to open a "MyModule Settings" UI in which she can modify module-provided settings. Notice this has two cases: 1) module-specific settings 2) global settings the module wishes to override whenever a game using that module is created. In this UI case 1 is very similar to the handling of global setting - the user has the choice to use the module's hardcoded default or set a new value. Case 2 gets more complicated: in the most extreme case a user could have the choice between global/module hardcoded/custom values, for a total of four possible values for each setting. We could potentially limit the choice to three values by neglecting the hardcoded global default if a custom global value is overriding it.
  • Finally, in the context of a game creation screen and in-game, the user would have access only to the per-save UI which wouldn't allow modification of global or module setting, but would allow the user, in the worst case scenario, to choose between a global/module hardcoded defaults, global/module custom default and a per-save custom value overriding any of those. Again, we could potentially limit the choice from five to three values by neglecting the global/hardcoded defaults if custom values are overriding them. In this context the user would then only have the choice between the (current) global default, the (current) module default (if the module is overriding a global setting) and a custom value.

In this scenario the different scope UIs would simply have different pull-down menus for each setting, providing more or less options depending on the context.

On that base, if we really wanted to, we could then have an additional button/pull-down menu, allowing the user to make a given custom value a global or module default. But I'd suggest working on this only when everything else is settled and works.

Notice here I've been describing UIs fully aware this issue is about serialization rather than UI. I'm just using the UI-side of things to explore usage scenarios which in turn should help defining the underlying architecture.

No time to contribute more to the discussion right now. I will write as soon as I can a follow-up post that goes deeper into the architectural/programmatic side of this.

@emanuele3d
Copy link
Contributor

emanuele3d commented May 12, 2017

As we work out the bigger picture, and also noting that there is a fair amount of interest and a sense of urgency around FlexibleConfigs, I think we need make concrete progress. In this context I'd recommend a more pragmatic approach:

  • we park, for the time being, the per-save config feature
  • we park, for the time being, the module settings overriding global settings feature

instead:

  1. we focus on global configs functionality - inclusive of GUI and API
  2. we focus on module configs functionality - at a lower priority, using same GUI and API as above

To clarify the difference between the second bullet point and point 2: theoretically MyModule could override a global setting so that any game taking advantage of MyModule uses the overriding value rather than the global one. I.e. the global setting might be: "engine:RoseColor" = "pink" while MyModule might override with "engine:RoseColor" = "blue". All games using MyModule will then have blue roses by default.

In practice here I'm suggesting that to make some progress we will skip that aspect for the time being and MyModule can only publish settings in the form "MyModule:MySetting" - modules would still be able to access engine settings values via the API and modify them though, if necessary.

@vampcat, @eviltak, @Cervator: how does this sound?

Finally, a couple of issues we need to address sooner rather than later:

  1. do we agree, for the time being, that global engine settings such as those in RenderingConfig, NetworkConfig, InputBindings are to be stored in separate, serializable FC-based objects retrievable via Context, similarly to what is done now? I say yes to this even though it could be possible to store all settings in one big FC.
  2. do we agree that modules can publish their settings in their own separate, serializable FC-based config object, retrievable via Context? I say yes to this rather than modules publishing their settings right in the global config objects.

I look forward to your comments.

@SoniEx2
Copy link
Contributor

SoniEx2 commented Aug 12, 2017

How about private FCs? ("TCPWhitelistConfig"?)

@emanuele3d
Copy link
Contributor

@soni: please elaborate.

@SoniEx2
Copy link
Contributor

SoniEx2 commented Aug 12, 2017

Not exposed to modules.

@emanuele3d
Copy link
Contributor

In terms of functionality, I think engine-private settings could be useful.

In terms of implementation, at this stage I'd implement settings that have to stay engine-private storing them in a separate FC object, one that doesn't get published in the Context/CoreRegistry or one that gets published in a sub-context that the modules do not obtain. And perhaps there are other ways, i.e. relying on sandboxing functionality.

Initially though, I'd focus on providing public settings functionality as there are lots of details to iron out there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Topic: Architecture Requests, Issues and Changes related to software architecture, programming patterns, etc.
Projects
None yet
Development

No branches or pull requests

5 participants