Proposed additions/tweaks to the api

[Superd22]

Related to #1 & #6

• Added description to InitAs + made all paramaters except label optional.
• Added DoneWithSettings() to SettingsPage
• Added DefaultCallbacks to SettingsPage

[BlueRaja]

Does this even compile? You've changed the interfaces without changing the implementors...

Also, what is the purpose of DoneWithSettings and Default*CallBack?

[Superd22]

@BlueRaja No it does not, this is a false PR for design discussion in #1 Please see #1 and #6 (as stated in the PR, and the commit, and the code) for more information on the intended goal of both functions.

[andrewgu]

@Superd22 not sure I understand what the Default*Callback stuff is supposed to do. Are you trying to give the user a way to set a default callback to trigger? In which case I imagine it'd look more like

function SetDefaultBoolCallback(delegate<SomeDelegateName> callback);

Was that what you meant?

[Superd22]

@andrewgu I see two user cases. 1/ Your setting is complex and you wanna handle a custom logic each time it changes, you use the already defined delegate in InitAsCheckbox (or whatever)

2/ Your setting is really not complex, and you just wanna save it to your class when it changes. Rather than having to :

• a) declare a custom delegate when calling InitAs
• b) in that delegate which will get a MCM_API_Setting, get the name of the setting, and get the value of the setting.
• c) finally switch around all settings names and set your variables accordingly.

I propose that in case 2/, user skip all together a) and b) ie :

• When you InitAsCheckbox and do not declare a custom delegate, DefaultBoolCallback will be triggered each time that checkbox value is changed.
• This DefaultBoolCallback will not expose a MCM_API_Setting but directly a setting name and value.
• This allows the user to jump directly to step c).

Granted, it only saves the user a couple line (value = Setting.GetCheckboxValue(); && Setting.GetName()) but it makes sense in an abstraction point of view. The advanced user who wanna handle custom logic can set its own delegate and then receive a MCM_API_Setting. The base user who just wanna update his variables when settings are updated does not have to know anything about MCM_API_Setting and only gets the minimal ressources he needs : the name of the variable that was updated, and its new value.

[andrewgu]

@Superd22 I get what you're trying to do, just wondering if that's what you actually meant in the code. The way you defined it in the file wouldn't work, you would need a handler implemented on the MCM_API side, but without reflection that code won't be able to change a config variable on the client mod side.

Unless we take @BlueRaja 's idea and make those into macros. That way a preprocessor expansion will write in boilerplate code to accomplish the same effect.

[Superd22]

@andrewgu : I'm just trying to abstract MCM_API_Setting for people who do not care :

Old implementation :

...
P1S[0].InitAsCheckbox("Var one", VAR_ONE, CheckboxChangeHandler);       
P1S[1].InitAsCheckbox("Var_two", VAR_TWO, CheckboxChangeHandler);
...
simulated function CheckboxChangeHandler(MCM_API_Setting Setting) {
    local bool value;
    value = Setting.GetCheckboxValue();

    switch(Setting.GetName()) {
        case 'var_one': VAR_ONE= value; break;    
        case 'var_two': VAR_TWO= value; break;
    }

}

New implementation

...
// Notice no custom callback
P1S[0].InitAsCheckbox("Var one", VAR_ONE);      
P1S[1].InitAsCheckbox("Var_two", VAR_TWO);  
....

simulated function DefaultBoolCallback(name SettingName, bool Value) {
    switch(SettingName) {
        case 'var_one': VAR_ONE= value; break;    
        case 'var_two': VAR_TWO= value; break;
    }
}

Granted it changes nothing as to what's actually happening under the hood, but i'm under the impression less is more as far as wide-spread use is concerned.

In an ideal world the end-user who doesn't want to know how all of this works doesn't need to.

EDIT : just realized i did indeed fuck-up in the function declaration. Those are of course delegates.

[andrewgu]

Alright, I see what you're doing. Yes, the function definition needs to change. Your idea could work, but not the way you defined it. You would actually want it to look like this:

// MCM_API_SettingsPage.uc
delegate BooleanCallback(name SettingName, bool Value);
function SetDefaultBoolCallback(delegate<BooleanCallback> Callback);

Alternately you could make those callbacks optional variables when initializing a settings page. However, defining Default*Callback as a member of MCM_API_SettingsPage would not work because MCM_API_SettingsPage wouldn't be able to take a client implementation.

[Superd22]

@andrewgu : Yep, pretty much. That's the lesson to be learned : do not try to put your ideas in code the morning after a week of midterms and a night of drinking :+1:

[andrewgu]

Closing this pull request because we've integrated the suggestions from this branch into the main one (if not the literal code).