FlexibleConfig

[emanuele3d]

Introduction
The current rendering config is a relatively straightforward (if long) Java class with hardcoded video settings and their handling. This includes accessors (get*(), set*(), is*()), but also functionality to subscribe to the config to be notified of changes - to specific values or to the whole config.

This works, but is inflexible. Ideally engine and modules should be able to publish their configuration settings, as needed. Here is a couple of simple scenarios to ponder about.

Scenario 1 - ShadowMapNode
The ShadowMapNode is responsible for generating a ShadowMap used to cast shadows from the main light (sun/moon). This node wishes to publish two settings to the rendering config: a) a boolean defining its enabled/disabled state b) an integer defining the size of the (square) shadow map, in pixels.

Sceneario 2 - RealCameras module
The RealCameras module (so far a fictional one) introduces a number of realistic effects for the camera. Among them are lens flares. These are generated by a specialized node injected into the render graph on module startup. The node renders up to 20 lens flare elements, each having a set of parameters to play with, such as the texture asset used, the distance along the flare axis and the maximum opacity of the element. All these settings need to be published at runtime on the rendering config, for the user to play with.

FlexibleConfig and Setting classes
Enter the FlexibleConfig class, barely more than a <ResourceURN, Setting> map:

• void add(Setting) - doesn't allow overwriting a stored setting object with a new one using the same ResourceUrn
• boolean remove(ResourceUrn) - return true if the setting has been removed from the config. This happens only if the setting no longer has subscribers.
• Setting get(ResourceUrn)
• boolean has(ResourceUrn)

Individual Setting instances would contain:

• an id - of type ResourceURN: the prefix is engine-set and is either "engine" or the fully qualified name of the module publishing the setting.
• a stored value
• a type - i.e. boolean, integer, float, double, one-selection/multiple-selection lists
• a default value
• a range - where applicable, i.e. integers, float, double
• a list of choices - where applicable, i.e. list-type settings
• a comma-separated list of ui-path
• a human-readable name - translation friendly, to be displayed in the UI
• a description - translation-friendly, potentially verbose, describing in plain language what a setting does.

Setting is largely immutable and contains the information from the FlexibleConfigs Features section. But it would have a setter methods for the stored value. It also provides subscribe()/unsubscribe() methods and a boolean hasSubscribers() method.

A few clarifications about the ui-path feature. A ui path would provide a hint to the ui on how to group settings together. I.e. all settings having a path /engine/shadows/shadowmap could be displayed together when the "shadows" list item is selected in the "engine" tab of the rendering config. It could be meaningful however for the same setting to be also be displayed in a separate tab/menu, hence the possibility for multiple ui paths. Ultimately however, it is up to the UI code to decide how to take advantage of them. I.e. a simpler UI might not provide "tabs" and might group settings between horizontal separators instead, potentially ignoring the deeper sections of a path.

How does this all sound? And notice that this issue specifically avoids dealing with the UI needed to display the content of a FlexibleConfig instance. That will be for a separate issue.

[Cervator]

I need to find a little more time to look this over, but my first thought is that it probably relates to the CoreRegistry->Context switch (config in different contexts), adding more config at the world level, being able to modify module config after creating a world, etc. Not that we need to do all the things before we can complete this, just something to keep in mind, even for myself when I get a chance to think more about it :-)

[emanuele3d]

Understood. Ok, let me know if there is any change I should make to the proposed architecture.

Meanwhile I have realized that if the subscribe/unsubscribe functionality is available through the Setting instance we also need functionality to mark a Setting as stale, meaning it is no longer associated with a config. I'm guessing we'll have to move the unsubscribe/subscribe functionality back into the FlexibleConfig itself, to keep things simple.

[eviltak]

Any update on this? I'd like to take up this issue.

[emanuele3d]

@Cervator: I just re-read your last comment. In the case of the RenderingConfig I'd normally implement it as a subclass of FlexibleConfig and I'd store its only instance in the main Context. Nodes can then publish their settings on it. Alternatively, as I can imagine it only as a singleton, I'd use a static method to retrieve it with RenderingConfig.getInstance(). Or even both.

I thought briefly of storing the settings straight into a "RenderingContext" but a context fundamentally is a <Class<?>, instance> map while a config would be more constrained: a <ResourceURN, Setting> map.

Can you let us know if this fits with what you mentioned?

@eviltak: thank you for volounteering. If cervator has no objections I'm happy for you to take care of this. I'll be in standby to review the PR when it becomes available.

[Cervator]

Ideally we would seek some feedback from @flo or maybe @rzats - I think they're more in tune with the whole Context bit. I'm not and keep failing to find the time to really dig in deeper :/

[flo]

Well, I haven't thought it through but in general it is better to get a instance of a class from the context than from a singleton. As you then have the option in future to change it to be not a singleton if we need to.

[rzats]

Looks like a good idea to me - it ties in nicely with the work done on tabbed UIs (thanks @jellysnake!) as well as making the rendering API more accessible to modules.

@eviltak - feel free to work on this (and ping me if you want it added as a GCI task!)

[emanuele3d]

@flo: as you wish. Question: could we have a Context.getMainContext() static method? Or should we always get it through injection? Or in another way? It is sometimes unclear to me where to get the context from.

[flo]

@emanuele3d the context should be obtained from whatever created the instance of the class. If available via injection. In cases of static method it would have to be passed in by argument.

[Cervator]

Bump - a related config issue for world gen just came up and this seemed the most sensible existing issue to mention it on, tho we may split it out into its own thing and get an Epic going already with all this config stuff soon.

The world gen tutorial uses an inner class implementing Component to store config that is then used on the world configuration page. I haven't noticed anything so far actually using the Component nature of that inner class, and it is confusing to me why it implements Component if not (it is an empty interface - might have just been convenient).

Maybe that system could be updated to use a more configuration-specific approach like suggested here, dropping Component if it doesn't relate to reduce confusion. Get on the Context train already! Maybe it'll help world-specific configuration too including enabling/disabling modules after world creation, and dare I say it, maybe even world-specific rendering settings if there'd ever be a purpose for that ... :-)

Some other more or less related issues: #2509, #2068 / #2074, #1843. #1770, #1709 (this last one is yours btw @emanuele3d - or would it in theory be complete by now?)

[eviltak]

I'd like to take this up as a GCI task @Cervator and @rzats.

[rzats]

@eviltak done! https://codein.withgoogle.com/tasks/4737967044165632/

[SoniEx2]

Does this let modules access arbitrary configs? Would be nice if HTTP module could create HTTP configs (see #2618).

Sucks that configs are currently limited to engine subsystems.

[emanuele3d]

@Cervator: I've implemented #1709 only in the RenderingConfig and RenderingDebugConfig. There are other Configs in Terasology and I have not touched those. So, it is complete from my (rendering) perspective but not from the perspective of the whole project.

This issue about FlexibleConfigs might superseed #1709 though. In the description above it is only implied but I should probably make it absolutely clear that the vision is for implementations of FlexibleConfigs to make all settings automatically observable.

[emanuele3d]

@SoniEx2: yes. Both engine and modules would be able to store and retrieve arbitrary settings from a FlexibleConfig implementation.

[SoniEx2]

@emanuele3d So how's this supposed to work with HTTP?

Any module can add stuff to their HTTP whitelist, literally making the whitelist useless?

Why isn't this module-restricted?

[emanuele3d]

How are the HTTP whitelist and Configs related?

[SoniEx2]

@emanuele3d The way #2618 works is that there's a whitelist for TCP sockets, and that whitelist cannot be accessed/modified by modules. However, if there was an HTTP module (read: not subsystem) then its TCP whitelist would be basically available to any module using it (albeit restricted to the HTTP protocol only), unless it also implemented a whitelist.

For HTTP to implement a whitelist, it needs to, well, have a config. That config needs to be accessible by it, but BY NO OTHER MODULE.

Then, when a module requests a connection, it looks through the whitelist and checks if the specific module can access the specific address, and if it can then everything's good and stuff.

You don't want modules to freely add or remove things to the HTTP whitelist. You want the server admin to control the HTTP whitelist.

Having an HTTP module would also be just the start. We'd also have a database module (with the same whitelist mechanism as stated above), maybe an IRC module (not entirely sure), and so on.

I don't think you want all those modules to be in the engine. (The database module itself would have to be massive, maybe bigger than the engine itself, due to how many database protocols there are out there.)

[SoniEx2]

Also you might wanna take a look at my changes to Context in #2618, I don't think you'll be able to sandbox module configs otherwise.

[emanuele3d]

Thank you for clarifying this for me @SoniEx2.

FlexibleConfig at this stage is just an idea for a way to handle existing configs and for modules to publish their settings. This will allow both users and code to access them in a more uniform fashion. That covers most needs at this stage.

If something, i.e. a module's setting, needs to remain entirely or selectively secure, it doesn't have to published via an object implementing FlexibleConfig. You can use your own config class, use a non-public FlexibleConfig or, if it makes sense, you could extend FlexibleConfigs and implement a SecureConfig.

Does that answer your question?

[SoniEx2]

Uh, alright.

I might be able to do it a different way that doesn't require any configs, with the minor drawback that one won't be able to restrict the protocols a module uses... But thanks anyway!

[eviltak]

To clarify, this issue is just for the implementation of rendering config, isn't it? Usages of RenderingConfig will (should, IMO) be updated to use FlexibleConfig in a separate issue and PR.

[emanuele3d]

The needs of the rendering code were the inspiration for this issue, but this issue is really only about writing/implementing the FlexibleConfig described above and any other related interface/class so that engine and modules can take advantage of it to store and retrieve settings.

Once that's done one separate, follow-up task emerges: a UI needs to be developed for the user to see and modify the content of a FlexibleConfig instance. I'm counting on @rzats to either chip-in on this or mentor somebody else on this.

Once the UI is available existing configs can be migrated to instances implementing the FlexibleConfig interface, among them the RenderingConfig and the RenderingDebugConfig.

So, for this issue I'm really only expecting a FlexibleConfig interface, a FlexibleConfigImpl class and any small satellite interface/class that is required, i.e. Setting/SettingImpl: no need to hook existing configs to it just yet.

Does this clarify this issue?

[eviltak]

@emanuele3d yes, that makes everything clear. I'd love to work on other FlexibleConfig implementations too even after GCI ends. From what I've seen of the code though, RenderingConfig can be completely replaced by a FlexibleConfigImpl instance. I don't think further implementations of FlexibleConfig will be needed, but that can be thought about later. I'll try submitting a PR closing this issue in the next 24 hours.

Also, do I need to create tests for FlexibleConfig? Might as well, since there is no way to test them without using this in place of another Config.

[emanuele3d]

Yes, especially in this case tests are a good idea, exactly for that reason.

Looking forward to the PR!

[eviltak]

@emanuele3d I'm deciding against using a FlexibleConfig interface and directly using a class, since I think it'd be better if we used composition if we do need to extend the default behavior. Any code that needs to be run apart from raising the property changed event can be added to the Setting instances.

I'm also going for a simpler unserializable event dispatching system that may or may not use the existing PropertyChangeListener and PropertyChangeEvent for backward compatibility and minimal refactoring, but will not use PropertyChangeSupport. Although, if you want this to be concurrent, I think I'll just go with PropertyChangeSupport.

Also, does Setting have to check if the value is valid (i.e. in range or a valid choice) before setting the value and firing the changed event? I assume it does.

[emanuele3d]

Regarding not using an interface: I'm not sure when interfaces became incompatible with composition, but I'm sure I'll understand what you mean when I see the code. 😉

Regarding using/not using PropertyChangeListener: I took advantage of it in the rendering configs due to the discussion I had with msteiger and immortius in #1709. I guess you have to keep in mind the risks while attempting to re-invent the wheel, but if in the process you invent a wing, or an anti-gravity plate, why not? 😄

Regarding Setting, when I wrote about ranges I was thinking more about aids to the UI than validity checks when storing a new value. But it sounds like a good idea to have validity checks. And I guess each Setting subclass, i.e. a FloatSetting, can implement the checks that make sense for it. The only thing I'd suggest in this context would be to keep as much as possible optional. I.e. sometimes it make sense for a setting of type float to have a range, but sometimes it make sense to leave it unbounded or only partially bounded. Something to keep in mind.

[eviltak]

@emanuele3d is it alright if the values returned by Setting (during value retrievals) are of type Object and have to be manually casted to the desired type? If it isn't alright, it's going to be really hard making FlexibleConfig flexible in terms of the types stored by Settings.

[emanuele3d]

Let's think for a moment how we would like an engine or module developer interact with a flexible config. Out of the top of my head, I envision it this way:

ResourceURN aSettingURN = new ResourceURN("engine:aSetting")
FloatSetting aSetting = (FloatSetting) aFlexibleConfig.get(aSettingURN)
float aValue = aFloatSetting.getValue();

So, as you can see I'm assuming whoever wants to retrieve a Setting knows what kind of setting she is getting. So the cast is not when one retrieves the value but when one retrieves the setting.

The UI on the other hand will have to find what each setting's type is. But it appears getClass() returns the subclass rather than the superclass and this should be useful to have an <Handler, SettingSubClass> map so that each Setting instance retrieved from a FlexibleConfig object can be processed by the right handler.

Does this help?

[eviltak]

@emanuele3d To make the FlexibleConfig more "flexible", I'm going for a more reusable approach, where there is only one Setting class which is generic (for now). This requires the retriever to manually cast the type to the type of the data stored in the Setting (Class<T> for a T in Setting<T> can be obtained). Casting a Setting to a Setting<T> causes an unchecked cast warning, but the value inside the Setting can be safely cast. I'm doing this to avoid creation of many Settings like FloatSetting, StringSetting, IntSetting and so on.

[emanuele3d]

Ok, please take the piece of fictional code above and show me how it would look like with your architecture.

[eviltak]

It would look similar to the following:

Setting setting = flexibleConfig.get(settingUrn);
float value = (float) setting.getValue();

[emanuele3d]

And where do you plan to add functionality such as ranges for numerical settings?

[eviltak]

Each Setting requires an instance of the SettingValueValidator<T> interface which has a method boolean isValid(T) as a validator. If not specified, the validator is set to an implementation of the said interface that returns true for any value passed to it. The PR will contain a RangedNumberValueValidator<T extends Number & Comparable> which implements bounded and semi bounded ranges for all numeric types.

[emanuele3d]

Understood.

As the UI will eventually go through the list of setting stored in the configs in bulk, how will it know how to deal with an instance? I.e. let's assume a float setting with a range. How will it find the retrieved setting contains a float and how will it retrieve its range, if any, to provide the appropriate widget for it?

[OvermindDL1]

Just as a side question, but any thought given to using a library like config as it seems to handle all this already while being extendable as well?

[eviltak]

@emanuele3d the same way it would know that if the object is a FloatSetting or not (I assume by checking using an instance of call). The range can be determined by casting the SettingValueValidator too, or maybe another method can be added to the interface that returns the data constraints in some sort of data structure.

[emanuele3d]

@OvermindDL1: the library you suggest would make sense IF:

cost of evaluation of library + cost of integration of library << cost of development of this proposal - development already occurred.

Both @eviltak and I have put some time into this. Constructive feedback is appreciated, but I'm not sure advising such a change in direction qualifies, no matter how well intentioned and even reasonable your suggestion is.

I am sure technically it would work. Ultimately given time and effort we could integrate and use any library. Whether it would save us time for this particular task, that's a much harder call to make and for the time being I see too feeble of a reason to change the course.

@eviltak: above you provided an example in which the nature of a setting was already known. Now, let's assume you are the UI builder code, you are iterating through the instances of Setting in a flexible config and an instance is now available to you in the variable aSetting. It's a float setting and it has a range, but you don't know that yet. Can you write a few lines of code to show how you would retrieve that information?

[SoniEx2]

<APNG> so like, terasology has a sandbox right?
<APNG> and most libraries mix interface and implementation right?
<APNG> and that's basically the issue
<APNG> it's why I didn't expose anything from java.net in the PR I made
<OvermindDL1> I noticed github yeah, it sucks.  ^.^
<OvermindDL1> Terasology runs all mods within a sandbox yes, they accept PRs if you need access to something else though that they can review
<APNG> basically, I hate it when the config library mixes config file reading with config parsing and stuff
<OvermindDL1> Hehe
<APNG> because then you expose the config API and bam stuff can suddenly access files through it
<APNG> (so, please, if you make a config library, split implementation and interface)
<OvermindDL1> Thankfully open source so it could just be included verbatum then fixed up
<OvermindDL1> I'm not going to make anything for it currently, too much real-life work.  ;-)
<APNG> it'd be so nice if more things split implementation from interface :/
<OvermindDL1> It'd be so nice if Java was actually a decent language that encouraged that...
<APNG> it doesn't forbid it
<APNG> and it's actually pretty easy to do it
<APNG> just look at terasology itself
<OvermindDL1> It does not forbid it, but it does not encourage proper programming styles regardless
<APNG> well yeah the java stdlib sucks
<APNG> that's why you make libraries that replace the java stdlib
<APNG> so, yeah, most libraries are unsuitable for use with terasology's sandbox, and that's why no config
<APNG> I just hope they're not mad at me after I ragequit and closed my PR
<OvermindDL1> Config is open source though as such it could be adapted to work with
<OvermindDL1> Heh, ragequit eh?
<APNG> I left the IRC channel
<APNG> then closed the PR
<APNG> it's open source, sure, but splitting implementation from interface requires a major version bump and rewriting everything
<APNG> if you're gonna rewrite it from scratch, might aswell uh... rewrite it from scratch, I guess
<OvermindDL1> Hehe, I'd probably use Rust at this point though

(@OvermindDL1 if you're not ok with this I'll delete)

[Cervator]

@SoniEx2 as far as splitting implementation from interface and other major version bumping needs then I have no issue with that as long as it follows good timing and has justifiable design + implementers. I foresee another round of backwards incompatible changes coming with GSOC 2017 (if we get in) and finishing the extraction and adoption of gestalt-entity-system over the coming months as well (hopefully Beta after all that?). Config and especially finishing CoreRegistry -> Context would also fit perfectly into that batch.

Probably some of that work including any related GSOC proposals would hang out in some longer term branches until they're ready for the big version bump. Maybe a super nice and fancy config could be done that way.

I think your PR #2618 could get merged to engine sooner with its simple approach of just doing its Socket or HTTP thing from the engine for a few months, while the BIG config/sandboxing thing might be a phase two that could eventually lead to better config and segregation in modules. The "small" config thing discussed here (small in comparison to rewriting all the things anyway) likewise might be a thing we can merge in the short term, unless somehow we should go straight for the big option. But this is something @emanuele3d is eager to put into use the first moment we can get good working code.

Ultimately decisions get based more on who is available to implement what, when, and how constructively we can discuss it all. We have the Socket PR, which is fine, but connecting it to any major rewriting that has yet to happen will block it. Likewise @emanuele3d and @eviltak here are already working on implementation details for the original purpose of this issue.

As far as typesafe's config goes I believe @immortius looked into it at one point and found some deficiencies. Maybe there are ways around that and with your and @OvermindDL1's help maybe we can get beyond all that and write the bestest config system ever. But code talks louder than IRC logs. One step at a time - we should probably treat your PR and the original issue here as one phase and major redesigns as a later one. How does that sound?

[SoniEx2]

@Cervator I was talking about using an external config library (aka what @OvermindDL1 talked about).

The best option is to use a good external config library. But such a config library needs split interfaces and implementation. Since none exist, converting one would require it to get a rewrite and major version bump.

[immortius]

A few thoughts/comments.

The current design of modules puts them in a sandbox, which restricts what classes and functionality they have access to from the engine and its dependencies (that is, java and other libraries it uses). However, it isn't designed to sandbox modules from each other - the architecture is more of a core-feature providing engine with game logic and asset providing modules. So the idea of a module introducing features to be secured for use by other modules is not supported as such.

I get the feeling that, @SoniEx2, you were envisioning modules as a more fundamental extension system with the ability to introduce engine features as well. So this conflict is probably the source of most of your issues. On the other hand I feel for your desire to introduce these features without having to touch engine code.

I guess I would propose some system for loading jars or possibly modules as part of the engine itself, outside the sandbox. These would have to be manually added to terasology's config, and would not be subject to automatic downloads. Basically, these would be true extensions to the engine and have full access to all engine and java features. Including the @API annotation, so these modules would be able to expose methods to the sandbox, with permission requirements. Then you would be able to make full use of the permission system - although I understand this hasn't been integrated with Terasology yet either in terms of user prompts and permission rejections.

Certainly agree with the frustration with the core and other libraries that don't split interfaces and implementation well (there is a lot of rubbish in the core API from the early java days too). The envisioned solution for these issues was for the engine to provide safe encapsulations around these things.

[SoniEx2]

@immortius I was talking about using an external config library in the engine, like how you currently use an external JSON library (GSON) in the engine.

[eviltak]

@emanuele3d That's the obstacle that I'm stumbling upon. There's no way to know what type a Setting returned from the FlexibleConfig is of. It is possible to store the Class of the Setting, but that too isn't of much help because type erasure removes all generic information pertaining to the class at compile time. All the methods I am aware of require knowledge of the type over which the Setting is generic.

Since the above problem is going to be prevalent no matter what method we use, there is no other option but to have loads of if-else ladders in the UI builder code, or we can have the Setting build the UI for itself, with a method that returns a UIWidget (or similar).

[emanuele3d]

Instinctively I find the idea of a method returning a widget to be tempting.

I don't like how responsibilities would be getting tangled though. The Setting class as originally envisioned is fairly "dumb": it provides accessors, un/subscribing functionality and you make a good point recommending validity checks.

By adding UI functionality right within this object (even though it could be provided by some kind of UIBuilder instance held within it) we would be blurring the line between data and presentation.

It makes sense for responsibilities to be clearly defined: a Setting instance provides valid data and a way to monitor it, while the UI is responsible for displaying the setting to the user in whichever way is appropriate. And even just the ability to quickly describe this in plain English has a value on its own, i.e. when writing documentation.

So. The setting instance needs to provide an easily retrievable indication of its type so that a simple map ui-side can easily associate settings to handlers. As you prefer not to have specialized Setting subclasses, i.e. FloatSetting, I'd suggest to store the type in a ResourceURN provided on construction, i.e.:

Setting aSetting = new Setting(new ResourceURN("engine:float"));

and have it accessible via a getType method or something similar.

I am concerned about this otherwise typeless approach though. But let's see where we get from here.

[SoniEx2]

There is another option, which is how Minecraft does blocks. However, in the Minecraft case, those things are immutable.

But the basic idea is to have typed keys.

get(new EnumKey<>("key", EnumType.class)) // returns Enum<EnumType> since EnumKey<T> extends Key<Enum<T>>

In this case, settings are stored in a Map instead of fields with get methods. And since the key is a full object, nothing stops us from adding an isValidValue(T) to the key, and calling it every time a config is set. Thus allowing things such as new FloatKey("key", 0.0f, 1.0f), a float key that only accepts numbers within the 0.0f to 1.0f range.

[eviltak]

@emanuele3d The approach isn't typeless; Setting is generic over the type of the value contained inside it. The approach that I currently use returns a fully typed Setting from the FlexibleConfig (eg. Setting<Integer>). We don't need specialized Setting subclasses because the SettingValueValidator<T> field in a Setting instance does all the validation work for us. As such, Setting purely performs the functions of storing a value and adding/removing subscribers, as you said.

I was proposing to let the SettingValueValidator implementations contain a method that builds the UI (since they are the one that contain the constraint information) [aside: I realize the interface can be renamed to SettingValueConstraint], but the UI-side map idea seems nice. My only question (which may be due to my inexperience with Java) with the map based approach is the following:

Consider that we have a SettingValueValidator<T> implementation, RangedNumberValueValidator<T extends Number & Comparable> (which is included in my code) and we add a UI widget builder method (on the UI-side) for it with the following signature: <T extends Number & Comparable> UIWidget buildRangedValueWidget(Setting<T> setting, SettingValueValidator<T> baseValidator). Except for the type constraints, all other methods that build a UI for some Setting<T> which uses some implementation of SettingValueValidator<T> have the same signature. How will we store the generic handlers in a Map with keys of type Class<? extends SettingValueValidator>? I'm sure this is workable though, with keys that store both the SettingValueValidator class and the class of the type over which it and the setting is generic. For instance, handlerMap.get(new Key<>(RangedNumberValueValidator.class, Integer.class) could point to an instance of SettingUiBuilder<RangedNumberValueValidator<Integer>> which contains a method buildUi which builds the UI for those specific types.

Another problem with this method is highlighted at the end of this comment.

---

@SoniEx2 I'm already following a similar but better method. The concept of putting validation in the keys doesn't seem intuitive to me.

FlexibleConfig stores Settings in a Map with an instance of Key<T> as the key corresponding to a value of Setting<T>. The Key<T> stores T.class. Because of type erasure, Settings in the map are stored with the raw type, Setting.

The get method has the following signature: <V> Setting<V> get(Key<V> key). Whenever a Setting is retrieved using the above get method, the raw Setting instance corresponding to key is retrieved, and casted to a generic version by performing an internal cast of the stored values, thus returning a generic Setting<T> with full type information. Of course, if the user passes in a Key<T> with the wrong class, an Exception will be raised. To prevent this, the <V> Key<V> add(Setting<V>) method returns a Key with the correct type information. This can then be used as the parameter to get.

---

The only pitfall of this method and using a UI-side map to build Setting UIs is that the Keys will either have to be stored as static final fields of a helper class or be generated at runtime, preventing the usage of loops.

Making the SettingValueValidator build the UI will allow you to simply loop through all the Setting values in the FlexibleConfig map and call the setting.getValueValidator().buildUi(setting) method to generate the corresponding UIWidget. This method also has a negative (which is potentially much worse), however. For instance, RangedNumberValueValidator.buildUi will have to handle different cases for Integer, Byte, Double, Float and so on. For a UniversalValueValidator which allows objects of all types (eg. DisplayMode) and always reports them as valid, this gets much worse. If we go with the UI-side map approach, we will be able to handle things more cleanly, which is why I'm leaning slightly towards the map based approach.

[eviltak]

PR #2757 has been created for this issue.

[OvermindDL1]

For note, the reason why I asked about config from typesafe was because you can pass it a Java class and it fills in the fields like a schema definition, along with proper errors if types do not fit and more, in addition you can define your own types (want a vector type that is an array of two integers, or want it as a string like ":" or so, you can. It is not really a suggestion though but more of just a style of doing things, you could custom make the same style as well. But overall I'm a fan of a config filling in a class instance instead of per-field parsing. Per-field parsing is very brittle in comparison.

[emanuele3d]

@eviltak: switching to the PR discussion - no point in continuing the discussion here at this stage.