FlexibleConfig #2757

eviltak · 2017-01-15T14:02:29Z

Closes #2668. Implements a FlexibleConfig and Setting class which allows for an extensible configuration system which does not have hard-coded settings.

Testing

Run tests (all relevant tests are in the org.terasology.config.flexible package)

Outstanding before merging

Add tests for each method in Setting
Add tests for each method in FlexibleConfig
Add JavaDoc
- Document RangedValueValidator
- Document Setting
- Document SettingImpl
- Document FlexibleConfig
- Document FlexibleConfigImpl
Update tests to be more "test" like (check @emanuele3d review)

* Add tests for Setting and RangedNumberValueValidator

* Add tests for unbounded and semi-bounded RangedNumberValueValidators

* Added `FlexibleConfig.Key<T>` which stores `T.class` for `Setting<T>` in the `settingMap` * Added package-private static method `Setting.cast` which converts a raw `Setting` instance to a `Setting<T>`

GooeyHub · 2017-01-15T14:02:30Z

Can one of the admins please verify this patch?

* Added tests for Setting.setValueValidator and Setting subscribers

eviltak · 2017-01-15T15:21:29Z

A few questions I'd like reviewers to answer:

Should FlexibleConfig.add raise an Exception if the key is already present in the map?
Should FlexibleConfig.get raise an Exception if the type of the Key and the type of the Setting don't match (i.e. key.vClass.isInstance(setting.getValue()) returns false)? I'm leaning towards yes on this one, but it depends on the project architecture.
Should FlexibleConfig.has be renamed to FlexibleConfig.contains?
Should SettingValueValidator be renamed to SettingValueConstraint (or something better)?
Should FlexibleConfig and Setting be refactored into an interface and its implementation?

* Fixed `FlexibleConfig.get` to perform a cast instead of creating a new instance. Removed now unnecessary `Setting.cast` method. * Added tests for each method in `FlexibleConfig`

Cervator · 2017-01-15T20:23:40Z

@GooeyHub: ok to test

@eviltak: thanks for all the work and consideration you've put into this topic :-)

Poking @emanuele3d and @flo for feedback

GooeyHub · 2017-01-15T20:32:43Z

Hooray Jenkins reported success with all tests good!

emanuele3d · 2017-01-16T20:41:44Z

@eviltak: this PR might need some more patience than usual unfortunately.

With the other folks interested in architecture we are currently looking at some high-level concepts that have been mentioned a number of time and never fully discussed. Configs is one of those concept and it's not quite clear yet if we are even going in the right direction with issue #2668 and therefore this PR.

So, you might have to wait a bit longer than usual to get a proper review.

eviltak · 2017-01-17T01:49:46Z

@emanuele3d that's alright. Any other things (that you mentioned) that you think I can help out in?

Cervator · 2017-01-17T02:40:01Z

Heya @eviltak - you can check out some of the related architecture talk over here if you're interested:

http://forum.terasology.org/threads/config-context-and-security.1761/

A ton of it is beyond the initial Config work which is sort of a phase 1 and the first thing we hope to get done, with this PR the focal point.

So far the main solid point I've taken to have some impact on this PR is the idea of moving to use SimpleUri instead of ResourceUrn - the latter is way more advanced than what we likely need in a key, and actually comes from gestalt-asset-core rather than the engine itself. Could you evaluate that possibility and see if it would cause any trouble to switch?

It might also give us a nice key naming convention simply consisting of a module name + object, with the module name possibly relating to an actual module or something else when appropriate (global rendering config or other stuff - more of a category there)

The idea behind that is covered in the thread: in short it would allow easy extraction of Config+Context+Injection into its own little external library at some point, without having to depend on gestalt-asset-core - maybe even including SimpleUri itself

On the type stuff and UI auto-building then my brain is still trying to digest that. I think I get what you mean but stuff like <V> Setting<V> get(Key<V> key) makes my brain melty. I would like to think we can get to where you can feed a settings object containing enough stuff including value ranges to a UI widget for it to auto-create the UI nicely. The config stuff shouldn't depend on any UI piece.

Thanks for sticking with this so far, and apologies for any missteps I may make in trying to understand all this, I really don't get enough exposure to fancy Java these days :-)

eviltak · 2017-01-17T11:50:30Z

@Cervator Switching to SimpleUri should just be a simple refactor, not taking more than a couple of minutes.

The reason for creating a separate generic Key class for the FlexibleConfig is primarily to be able to obtain the type T of the Setting<T> value corresponding to the key. This is done for the same reason that Florian on the forums (@temsa on GitHub, I guess) posted on that thread. By using classes as part of the keys you can let the FlexibleConfig know what the correct type of the corresponding Setting<T> should be, and let it itself verify that you always get the correct type. Of course, you could perform an unchecked cast to the generic variant that you think is the type and simply storing ResourceUrn/SimpleUri keys, but this opens up the possibilities to many ClassCastExceptions.

The reason for making Setting<T> generic is so that the compiler can tell you what type is stored inside the Setting instead of you having to guess it. Generics are used for the same reason CoreRegistry.get is generic over T and accepts a Class<T>, so that the code and the compiler can prevent human error. IMO WorldRenderer renderer = CoreRegistry.get(Context.class) making the compiler complain is much better than WorldRenderer renderer = (WorldRenderer) CoreRegistry.get("Context") causing a runtime exception. For the same reason, I prefer Setting<DisplayMode> setting = flexibleConfig.get(SettingKeys.RESOLUTION) (where SettingKeys.RESOLUTION is a Key<Resolution>) over Setting<DisplayMode> setting = (Setting<DisplayMode>) flexibleConfig.get(SettingUris.RESOLUTION).

emanuele3d · 2017-01-17T15:48:11Z

I understand you reasons @eviltak. Your approach is a virtuous one but not one that I support. I will write more about it later.

Cervator · 2017-01-17T15:48:47Z

Our Florian is @flo, like listed over in the Reviewers section in the sidebar. Dunno who that temsa is :-)

Thanks for the further examples! Is that getting into how you'd prepare SettingKeys.RESOLUTION and others to be able to easily know what type you're wanting to get back? From the earlier issue discussion I remembered:

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.

Is that what SettingKeys.RESOLUTION is an example of there? The helper class? It is of type DisplayMode ?

I do like it when in existing code you supply a Something.class when you know that's the class you want to get back.

eviltak · 2017-01-17T16:18:59Z

@Cervator You got that right. In my example, I wanted to show how my approach makes the compiler point out an error in the choice of type instead of a runtime exception (I'm sorry I didn't make that clear, updated the comment). SettingKeys.RESOLUTION is something similar to new Key<>("engine:ResolutionSetting", Resolution.class). Since setting in my example is a Setting<DisplayMode> but the get call returns a Setting<Resolution>, the compiler raises an error. But in the case where the returned setting is manually casted, it would result in a runtime exception.

Cervator · 2017-01-17T16:52:54Z

So if I understand it right that's then also what gets in the way of easy looping. You could loop over the map, but not easily get the type of what you were looking at.

A user wanting to get a single typed object would already know what they want, much like when we retrieve something from the CoreRegistry.

Cervator · 2017-01-17T18:23:17Z

A few more thoughts: I think I am seeing better what @emanuele3d is getting at. The very beginning of the original issue was very focused on the details for every individual config item, like the UI hint for instance. The types would be very limited as there just aren't that many - generally you have numbers, strings, and booleans (which sometimes may be in lists for stuff like radio buttons or checkboxes).

The discussion spiraled to all kinds of more advanced (and mostly unrelated) topics which introduced plenty of confusion and got us off track somewhat. Whitelists, security, all kinds of fancy stuff. That led to the forum thread to try splitting stuff apart again so we could just focus on plain config again here.

While the code here is super cool and I think I understand better what it does now it sort of ends up almost reimplementing Context by becoming a container for arbitrary classes that in turn could contain config (or anything, really). It is way more generic than the original vision for just handling relatively "dumb" config details.

Rather than having an exceedingly focused config object we store in a Context, we sort of have a config-flavored Context we store in a Context.

Hopefully I'm on track here and making some sort of sense. Maybe other config frameworks are leaking in, from situations where you really only have one sort of container of arbitrary objects that could contain config, but are classes in their own right. While here we have a split between the high level Context and a simple map of individual config items inside a FlexibleConfig instance.

emanuele3d · 2017-01-18T10:44:28Z

Just wanted to inform you all I am reviewing the code. Hopefully I will finish the review tonight (CET).

eviltak · 2017-01-18T17:05:02Z

@Cervator The reason I'm going for a generic approach is type safety and flexibility. The following types of values were found by a script that I wrote in the RenderingConfig class, which was my primary inspiration for FlexibleConfig:

PixelFormat
PerspectiveCameraSettings
int
DisplayModeSetting
boolean
ViewDistance
float
ScreenshotSize
String

Instead of having 9 (and possibly more for other Config implementations) different Setting implementations that do basically the same thing with variation only in specific areas (that's where the SettingValueValidator implementations come in), I thought that it'd be better to just require Setting publishers to define a SettingValueValidator that defines the constraints of the data (I really think it should be renamed to SettingValueConstraint now!) Also, this approach makes it much easier to define Setting instances of different types with the same constraints like no constraints, for example. Instead of creating multiple Setting impls for each type, you just pass in the same SettingValueValidator to a Setting instance generic over a different type.

Going with using separate implementations of a Setting interface for each object won't prevent storage of arbitrary classes that could contain absolutely everything either, since an arbitrary implementation of Setting can be created by the Setting publisher.

Regarding the similarity between the implementations of Context and FlexibleConfig, I'd say that FlexibleConfig/Setting is more than just a reimplementation of Context, the biggest addition being handling subscribers to value change events. If we do manage to create implementation/s of Context to use to store settings, it stretches the definition of "Context". As an external viewer of the code, I wouldn't expect Context implementations to have anything to do with storing configuration settings. If I have misunderstood what you meant to say, please correct me as what you wrote wasn't very clear on how you think Context can replace FlexibleConfig/Setting.

emanuele3d

@eviltak, thank you for your work. This PR is an excellent starting point.

Generally speaking, the main problem is that you built a Ferrari when all we needed was a relatively simple semi-truck. Fine engineering, no doubts about that, and I congratulate you on your java proficiency. But we need something simpler and more in line with the original design even though that design might appear less robust to you.

So, lots of changes, most minor, but some big ones too.

In regard to your last comment and the relationship between FlexibleConfigs/Settings and Contexts, I agree with your assessment. Cervator and I did discuss this at length about 24 hours ago and I think we are all a bit clearer about it - i.e. as you say, Contexts shouldn't be used to store Setting instances directly although it would make sense to me to use them to store FlexibleConfig instances, via sub-classes or, better perhaps, through a ConfigList instance.

emanuele3d · 2017-01-18T09:45:58Z

engine-tests/src/test/java/org/terasology/config/flexible/FlexibleConfigTest.java

+
+            assertEquals(50, config.get(key1).getValue().intValue());
+            assertEquals(25d, config.get(key2).getValue(), 0.00001d);
+        }


This test is meant to test the get method. While I understand that you are using the test to document the workflow, the assertions test the value in each setting. What you should test here is that the setting instance stored earlier is exactly the same as the retrieved one. Incidentally you can show how a Setting instance can be initialized, but I wouldn't go further. Although I'm sure some test purists would prefer you used a MockSetting class so that you can be sure no error can originate from it.

All other tests should be reconsidered in light of this. More complete workflow examples can be added to the javadocs of methods and classes.

emanuele3d · 2017-01-18T09:49:33Z

engine/src/main/java/org/terasology/config/flexible/FlexibleConfig.java

+import java.util.Map;
+
+public class FlexibleConfig {
+    private Map<Key<?>, Setting> settingMap;


This needs to go back toward the original design:

private Map<SimpleUri, Setting> settingMap;

I can change the Key and Setting class to use SimpleUri instead, but if we stop using Key as the map key type, we won't get typed Setting<T> instances from FlexibleConfig.get. We'll just get the raw type Setting.

Yes, that's ok with me.

emanuele3d · 2017-01-18T09:51:46Z

engine/src/main/java/org/terasology/config/flexible/FlexibleConfig.java

+
+        // Maybe throw an exception?
+        if (has(key))
+            return null;


Let's return a boolean. True if the setting has been added, false if the setting can't be added because one setting with the same uri is already in the config. This makes it consistent with the behavior of remove.

Also, let's make sure we do not allow null keys.

I return the Key because the Key is created by the FlexibleConfig itself, and needs to be used to access the setting from FlexibleConfig.get. Useful in cases like:

Key<Float> key = flexibleConfig.add(someFloatSetting); // ... Setting<Float> setting = flexibleConfig.get(key);

Of course, if keys are going to be manually created (like in a static helper class), I can create an overload of add: boolean add(Key<V>, Setting<V>) that returns true or false showing whether the Key was added. What do you think I should do with the existing add(Setting) method?

The key is the SimpleUri stored in the setting, so we don't need the config to create it and return it.

Of course we need to make sure that the map is aware of the fact that

new SimpleUri("module:setting") == new SimpleUri("module:setting")

that is, SimpleUris are considered identical if their content is identical.

I presume the SimpleUri.equals method handles that.

emanuele3d · 2017-01-18T10:13:47Z

engine/src/main/java/org/terasology/config/flexible/FlexibleConfig.java

+        Setting setting = get(id);
+
+        if (setting == null || setting.hasSubscribers())
+            return false;


Before we return false, let's issue two separate logger warnings depending if the key isn't in the map or if the given setting still has subscribers.

emanuele3d · 2017-01-18T10:19:52Z

engine/src/main/java/org/terasology/config/flexible/Setting.java

+    public Setting(ResourceUrn id, T defaultValue, SettingValueValidator<T> valueValidator) {
+        this.id = id;
+        // Precomputing for performance
+        this.idString = id.toString();


No need: settings shouldn't be retrieved continuously in hot code anyway. That's why we make them subscribable: only when something happens to the setting we react. In those circumstances a small performance loss is acceptable.

emanuele3d · 2017-01-18T22:45:12Z

engine/src/main/java/org/terasology/config/flexible/FlexibleConfig.java

+
+        if (setting == null) return null;
+
+        // Possibly raise an exception?


Once keys are SimpleUri this method should be as simple as:

return settingMap.get(key);

Same as above: if we stop using Key as the map key type, we won't get typed Setting<T> instances from FlexibleConfig.get. We'll just get the raw type Setting.

Yep, I confirm that's ok with me, user will have to know what type it is and cast it.

Alright. One thing though: Making the user cast it will cause loads of unchecked cast warnings. Instead, I propose to make the get method perform the cast for them and suppress the unchecked cast warning for it. Instead of Setting<Integer> setting = (Setting<Integer>) config.get(uri) polluting the codebase with unchecked cast warnings, we can have either Setting<Integer> setting = config.get(uri, Integer.class) or Setting<Integer> setting = config.get<Integer>(uri). Since we don't have a need for the Class (unless you want to return null instead of raising an exception if the types are invalid), I propose the second method.

I understand the idea of confining the warning to the get method. It's a good idea. Can you show me how you'd write the get method for this form:

Setting<Integer> setting = config.get(uri, Integer.class)

It would be defined as

<V> Setting<V> get(SimpleUri uri, Class<V> valueClass) { return (Setting<V>) settingMap.get(uri); }

Since the Class<V> will not be used (unless you want to return null instead of causing a class cast exception when the types do not match), the parameter can be removed. That's why I prefer config.get<Integer>(uri) instead of this signature. Plus, get<Integer>(uri) makes it more clear than get(uri, Integer.class) that the fetched type will be a Setting<Integer>, since the class parameter could be used for absolutely anything.

Ok @eviltak: let's proceed with option 2 and let's see how that looks like in the code.

My inexperience with Java got to me again 😛 Turns out that the get type argument is inferred from the type of the variable to be assigned. It simply is Setting<Integer> setting = config.get(uri); Simple, isn't it? Although we can't specify the type argument directly in the method like in C#: config.get<Integer>(uri) does not work. The result must be assigned to a local variable for it to work.

emanuele3d · 2017-01-18T22:48:06Z

engine/src/main/java/org/terasology/config/flexible/Setting.java

+        this.subscribers = Lists.newArrayList();
+    }
+
+    public Class<? extends SettingValueValidator> getValidatorClass() {


I don't think we need this method.

emanuele3d · 2017-01-18T22:53:21Z

engine/src/main/java/org/terasology/config/flexible/Setting.java

+
+        this.valueValidator = valueValidator;
+
+        this.subscribers = Lists.newArrayList();


Perhaps we can initialize this only when the first subscriber arrives and we can set it to null when the last one has been unsubscribed. Just a minor memory saving for those settings that aren't being actively monitored.

emanuele3d · 2017-01-18T22:55:58Z

engine/src/main/java/org/terasology/config/flexible/Setting.java

+        return value;
+    }
+
+    public boolean setValue(T value) {


Let's rename the input variable to newValue.

emanuele3d · 2017-01-18T23:02:12Z

engine/src/main/java/org/terasology/config/flexible/Setting.java

+    private String name;
+
+    private String description;
+    private SettingValueValidator<T> valueValidator;


Let's rename this variable to just validator. In the context of this class we know it validates the value.

emanuele3d · 2017-01-18T23:38:33Z

By the way @eviltak, feel free to hold on changing the test files. You can wait until the other classes have stabilized.

* `SimpleUri` replaces `ResourceUrn` as the id type for `Setting`.

eviltak · 2017-04-13T13:07:26Z

@emanuele3d Please review when possible.

GooeyHub · 2017-04-13T13:09:27Z

Hooray Jenkins reported success with all tests good!

GooeyHub · 2017-04-13T13:18:05Z

Hooray Jenkins reported success with all tests good!

GooeyHub · 2017-04-13T13:26:43Z

Hooray Jenkins reported success with all tests good!

GooeyHub · 2017-04-13T13:35:15Z

Hooray Jenkins reported success with all tests good!

emanuele3d · 2017-04-15T12:09:01Z

Review in progress....

emanuele3d

Just three small changes, two of which had fallen through the cracks! @eviltak: YOU CAN DO THIS! 😄

emanuele3d · 2017-04-15T12:23:54Z

engine-tests/src/test/java/org/terasology/config/flexible/FlexibleConfigTest.java

+        @Test
+        public void testContains() throws Exception {
+            SimpleUri id = new SimpleUri("engine-tests:TestSetting");
+


As per earlier feedback, this empty line and the identical one in the next test should be removed, the idea being the line above and the line below, together, they setup the test and should be grouped, while the assert line at the end, on its own, executes the test.

As you seem to have missed this and a few other items of feedback, you might want to adopt a more robust workflow:

if you just adopt the suggestion, just react with a thumb-up to it - after you have done the change in your local code.

if you don't want to adopt a suggestion or it needs further discussion leave a comment about it: we will discuss things further and the end result should be a thumb-up from you - again, after local changes - or from me if I retract the suggestion.

Either way you can then be sure that an item of feedback has been dealt with and it gets easy to check large-ish number of past feedback items in the main PR page.

emanuele3d · 2017-04-15T12:25:45Z

engine-tests/src/test/java/org/terasology/config/flexible/FlexibleConfigTest.java

+            config.add(setting);
+
+            setting.subscribe(propertyChangeEvent -> {
+            });


No thanks, it's ugly and greatly offends my sense of code aesthetics. :)

Besides, you let a machine reformat the code for you???? Shocking. How can an algorithm possibly put poetry in the layout of a piece of code??? Only a person can! (cough) At least for a few more years until Deep Learning has been unleashed on this issue too... 😨

emanuele3d · 2017-04-15T23:30:25Z

@eviltak: additional question for you.

A couple of FlexibleConfigs-related issues (#2893, #2894) are in the works to determine where to go next, after FCs get merged in the codebase. @vampcat seem to have a bit of free time available and would be happy to take care of them or at least help.

You have given a lot of time to this PR and it's only fair we ask you first: would you like to continue being the coding lead on this effort? Do you have enough time and energy to proceed with other FC-related issues? I'd be happy for you to do so.

Alternatively you could help with the review process, which tends to be somewhat less time-consuming compared to coding but just as important.

Let us know!

eviltak · 2017-04-16T12:21:41Z

@emanuele3d I'm more than fine with others pitching in. If @vampcat (or anyone else, for that matter) is ready to tackle those issues, they are free to do so. I may not have time to actually implement the changes, but I can and will post recommendations and answer/pose any questions if needed.

I'd love to help with the review process, if needed.

- removed a few unnecessary empty lines / unified a previously split line

GooeyHub · 2017-04-17T13:48:16Z

Hooray Jenkins reported success with all tests good!

emanuele3d · 2017-04-17T13:54:41Z

@Cervator: this PR is finally good to go. At 236 conversations (with this one), I believe it's the new record? 😊

@eviltak: thank you for your work and your persistence with this. I believe when Flexible Configs serialization is in place FCs will slowly become widespread in many parts of Terasology's codebase and your work will greatly pay off.

Regarding future FCs-related efforts, especially on Serialization, I then recommend for @vampcat to lead the charge. I will then be close behind leading/coordinating the review process and eviltak will take part in it in the measure that his schedule and level of interest will allow.

Again, @eviltak, thank you.

Cervator · 2017-04-18T04:25:47Z

Thanks @eviltak and reviewers! Merged at last :-) Especially appreciated are the new unit tests, ran them and all appeared well.

This is definitely another of our record PRs, there are a few though and they each tend to excel in one or more unique ways :D

Looking forward to more work in this area, as well as maybe a small chapter in a tutorial somewhere.

SoniEx2 · 2017-04-18T16:49:11Z

How do I use this?

eviltak · 2017-04-18T16:53:38Z

@SoniEx2 I'd recommend you wait until all the supporting structures (like serialization #2893 and UI support #2894) are in place, because this won't be much use otherwise. Expect a tutorial on usage after FlexibleConfig is completely ready for consumption both internally and externally.

SoniEx2 · 2017-04-18T17:04:05Z

I need something for #2901, even if it's just experimental.

eviltak · 2017-04-18T17:12:51Z

@SoniEx2 FlexibleConfig hasn't been integrated into the engine yet (i.e. there are no ways to create, store or retrieve a FlexibleConfig), so using it in its current state is out of the question. This PR only deals with the implementation, and the integration is going to be covered in future issues and PRs. I suggest you go with the typical hard coded class in the org.terasology.config package, or wait. All such hard coded configuration classes should be migrated to FCs in the near future.

eviltak added 4 commits January 12, 2017 16:32

Add FlexibleConfig, Setting, SettingValue

8fe159f

Add SettingValueValidator (with an impl) and tests

7165875

* Add tests for Setting and RangedNumberValueValidator

RangedNumberValueValidator can remove bounds

b4bc197

* Add tests for unbounded and semi-bounded RangedNumberValueValidators

Make FlexibleConfig.get return generic Setting<T>

e1fb7c0

* Added `FlexibleConfig.Key<T>` which stores `T.class` for `Setting<T>` in the `settingMap` * Added package-private static method `Setting.cast` which converts a raw `Setting` instance to a `Setting<T>`

eviltak mentioned this pull request Jan 15, 2017

FlexibleConfig #2668

Closed

Add tests for Setting

831892b

* Added tests for Setting.setValueValidator and Setting subscribers

Fix FlexibleConfig.get, Add FlexibleConfig tests

772413a

* Fixed `FlexibleConfig.get` to perform a cast instead of creating a new instance. Removed now unnecessary `Setting.cast` method. * Added tests for each method in `FlexibleConfig`

Cervator added Google Code-in Topic: Architecture Requests, Issues and Changes related to software architecture, programming patterns, etc. labels Jan 15, 2017

Cervator requested review from flo and emanuele3d January 15, 2017 20:23

Cervator assigned emanuele3d Jan 15, 2017

emanuele3d suggested changes Jan 18, 2017

View reviewed changes

Make FlexibleConfig.Key and Setting use SimpleUri

d3001c2

* `SimpleUri` replaces `ResourceUrn` as the id type for `Setting`.

Rename EPSILON to MAX_ALLOWED_ERROR

165e97d

emanuele3d suggested changes Apr 15, 2017

View reviewed changes

# Executed last requested changes.

3090f48

- removed a few unnecessary empty lines / unified a previously split line

emanuele3d assigned Cervator and unassigned emanuele3d Apr 17, 2017

emanuele3d approved these changes Apr 17, 2017

View reviewed changes

Cervator merged commit 3090f48 into MovingBlocks:develop Apr 18, 2017

Cervator added a commit that referenced this pull request Apr 18, 2017

Merge PR #2757 by @eviltak - new config framework

3d55a92

Cervator added this to the Alpha 8 milestone Apr 18, 2017

eviltak deleted the flexible-config branch January 1, 2018 11:06

eviltak mentioned this pull request Jan 19, 2018

FlexibleConfig improvements #3227

Merged

4 tasks

eviltak restored the flexible-config branch February 12, 2018 02:59

eviltak mentioned this pull request Feb 12, 2018

Sample direction that FCs are going towards [RFR] #3258

Closed

eviltak deleted the flexible-config branch July 24, 2019 07:53

eviltak mentioned this pull request Aug 13, 2019

AutoConfig #3723

Merged


		if (setting == null) return null;

		// Possibly raise an exception?


		this.valueValidator = valueValidator;

		this.subscribers = Lists.newArrayList();

FlexibleConfig #2757

FlexibleConfig #2757

Conversation

eviltak commented Jan 15, 2017 • edited Loading

Testing

Outstanding before merging

GooeyHub commented Jan 15, 2017

eviltak commented Jan 15, 2017 • edited Loading

Cervator commented Jan 15, 2017

GooeyHub commented Jan 15, 2017

emanuele3d commented Jan 16, 2017

eviltak commented Jan 17, 2017

Cervator commented Jan 17, 2017

eviltak commented Jan 17, 2017 • edited Loading

emanuele3d commented Jan 17, 2017 via email

Cervator commented Jan 17, 2017

eviltak commented Jan 17, 2017 • edited Loading

Cervator commented Jan 17, 2017

Cervator commented Jan 17, 2017 • edited Loading

emanuele3d commented Jan 18, 2017

eviltak commented Jan 18, 2017 • edited Loading

emanuele3d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eviltak Jan 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eviltak Jan 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emanuele3d commented Jan 18, 2017

eviltak commented Apr 13, 2017

GooeyHub commented Apr 13, 2017

GooeyHub commented Apr 13, 2017

GooeyHub commented Apr 13, 2017

GooeyHub commented Apr 13, 2017

emanuele3d commented Apr 15, 2017

emanuele3d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emanuele3d Apr 15, 2017 • edited Loading

Choose a reason for hiding this comment

emanuele3d commented Apr 15, 2017

eviltak commented Apr 16, 2017

GooeyHub commented Apr 17, 2017

emanuele3d commented Apr 17, 2017

Cervator commented Apr 18, 2017

SoniEx2 commented Apr 18, 2017

eviltak commented Apr 18, 2017

SoniEx2 commented Apr 18, 2017

eviltak commented Apr 18, 2017

eviltak commented Jan 15, 2017 •

edited

Loading

eviltak commented Jan 15, 2017 •

edited

Loading

eviltak commented Jan 17, 2017 •

edited

Loading

eviltak commented Jan 17, 2017 •

edited

Loading

Cervator commented Jan 17, 2017 •

edited

Loading

eviltak commented Jan 18, 2017 •

edited

Loading

eviltak Jan 19, 2017 •

edited

Loading

eviltak Jan 20, 2017 •

edited

Loading

emanuele3d Apr 15, 2017 •

edited

Loading