-
Notifications
You must be signed in to change notification settings - Fork 243
Add Conditional Control logic to settings display for GUI #2400
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
base: Dev
Are you sure you want to change the base?
Changes from all commits
95576e6
5b10aeb
0c2ff6e
2b34c73
0a7adcd
899f42f
d0679d9
9ec057c
b66deda
62f83e4
b301132
66c9199
b4bd1ff
2b41ead
25b59e0
bfad649
ad27b3b
54db250
945d2c0
44c292b
edf61bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -753,7 +753,7 @@ export class GeneratorComponent implements OnInit { | |
| return; | ||
|
|
||
| //Check setting is enabled first | ||
| if (!this.global.generator_settingsVisibilityMap[setting.name]) | ||
| if (!this.settingIsEnabled(setting.name)) | ||
| return; | ||
|
|
||
| event.dataTransfer.dropEffect = 'link'; //Change cursor to link icon when in input area | ||
|
|
@@ -771,7 +771,7 @@ export class GeneratorComponent implements OnInit { | |
| return; | ||
|
|
||
| //Check setting is enabled first | ||
| if (!this.global.generator_settingsVisibilityMap[setting.name]) | ||
| if (!this.settingIsEnabled(setting.name)) | ||
| return; | ||
|
|
||
| let items = event.dataTransfer.items; | ||
|
|
@@ -1075,13 +1075,29 @@ export class GeneratorComponent implements OnInit { | |
| return typeof (variable); | ||
| } | ||
|
|
||
| settingIsEnabled(setting_name: string) { | ||
| return this.global.generator_settingsVisibilityMap[setting_name]; | ||
| } | ||
|
|
||
| settingIsFullyHidden(setting: any) { | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to above, this gives more control over what determines if a setting is "visible" or "hidden". |
||
| return !this.settingIsEnabled(setting.name) && setting.hide_when_disabled; | ||
| } | ||
|
|
||
| getSettingCurrentState(setting: any) { | ||
| return { | ||
| "value": this.global.generator_settingsMap[setting.name], | ||
| "visible": !this.settingIsFullyHidden(setting), | ||
| "enabled": this.settingIsEnabled(setting.name), | ||
| }; | ||
| } | ||
|
|
||
| getNextVisibleSetting(settings: any, startingIndex: number) { | ||
|
|
||
| if (settings.length > startingIndex) { | ||
| for (let i = startingIndex; i < settings.length; i++) { | ||
| let setting = settings[i]; | ||
|
|
||
| if (this.global.generator_settingsVisibilityMap[setting.name] || !setting.hide_when_disabled) | ||
| if (!this.settingIsFullyHidden(setting)) | ||
| return setting; | ||
| } | ||
| } | ||
|
|
@@ -1182,7 +1198,7 @@ export class GeneratorComponent implements OnInit { | |
| this.triggerTabVisibility(targetSetting, targetValue); | ||
| } | ||
|
|
||
| if ("controls_visibility_setting" in targetSetting) { | ||
| if ("controls_visibility_setting" in targetSetting || 'conditionally_controls_setting' in targetSetting) { | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New flag to be processed in the JSON. If this isn't added, then both "disable" logic and "conditional" logic will only get processed if the setting option already has "disable" logic. In other words, a setting option that can only conditionally alter other settings would never have those conditions processed. |
||
| triggeredChange = this.triggerSettingVisibility(targetSetting, targetValue, triggeredChange); | ||
| } | ||
|
|
||
|
|
@@ -1248,11 +1264,11 @@ export class GeneratorComponent implements OnInit { | |
| let enabledChildren = false; | ||
|
|
||
| //If a setting gets disabled, re-enable all the settings that this setting caused to deactivate. The later full check will fix any potential issues | ||
| if (targetValue == false && this.global.generator_settingsVisibilityMap[setting.name] == true) { | ||
| if (targetValue == false && this.settingIsEnabled(setting.name) == true) { | ||
| enabledChildren = this.clearDeactivationsOfSetting(setting); | ||
| } | ||
|
|
||
| if ((targetValue == true && this.global.generator_settingsVisibilityMap[setting.name] == false) || (enabledChildren)) //Only trigger change if a (sub) setting gets re-enabled | ||
| if ((targetValue == true && this.settingIsEnabled(setting.name) == false) || (enabledChildren)) //Only trigger change if a (sub) setting gets re-enabled | ||
| triggeredChange = true; | ||
|
|
||
| this.global.generator_settingsVisibilityMap[setting.name] = targetValue; | ||
|
|
@@ -1263,7 +1279,19 @@ export class GeneratorComponent implements OnInit { | |
| return triggeredChange; | ||
| } | ||
|
|
||
| // targetSetting = The current option of the setting to process. | ||
| // targetValue = 'true' if the settings this option controls should be enabled, 'false' if they should be disabled. | ||
| // (Note: This is passed in 'checkVisibility' as "option != value", in other words: "This option is NOT the option the setting is being changed to".) | ||
| // triggeredChange = Set to 'true' to force this function to return 'true', suggesting a change occurred regardless of how things processed. | ||
| // Otherwise, the function will return 'true' if a dependent setting's state was altered, otherwise it will return 'false'. | ||
|
Comment on lines
+1282
to
+1286
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My understanding on these values waffled for a long time as I kept confusing myself on what each variable represented. Finally just wrote it down so I could stop trying to remember or guess. |
||
| private triggerSettingVisibility(targetSetting: any, targetValue: boolean, triggeredChange: boolean) { | ||
| // Resolve logic that could conditionally update this setting. | ||
| let conditionalSettingUpdates = this.getConditionallyChangedSettingsForOption(targetSetting); | ||
|
|
||
| // NOTE: We are treating any setting under "controls_visibility_setting" as one | ||
| // that should be disabled by the current option. Could be worth renaming... | ||
| let settingsDisabled = []; // Setting names in here are being disabled and take priority over any changes made by conditional logic | ||
| if (targetSetting["controls_visibility_setting"] != null) { | ||
| targetSetting["controls_visibility_setting"].split(",").forEach(setting => { | ||
|
|
||
| //Ignore settings that don't exist in this specific app | ||
|
|
@@ -1272,19 +1300,118 @@ export class GeneratorComponent implements OnInit { | |
|
|
||
| let enabledChildren = false; | ||
|
|
||
| if (targetValue == false && this.global.generator_settingsVisibilityMap[setting] == true) { | ||
| // We are about to disable this setting. | ||
| // If this is currently enabled, attempt to re-enable any settings that it | ||
| // may be disabling on its own. If it's disabled, it shouldn't also disable other settings. | ||
| if (targetValue == false && this.settingIsEnabled(setting)) { | ||
| enabledChildren = this.clearDeactivationsOfSetting(this.global.findSettingByName(setting)); | ||
| settingsDisabled.push(setting); | ||
| } | ||
|
|
||
| if ((targetValue == true && this.global.generator_settingsVisibilityMap[setting] == false) || (enabledChildren)) //Only trigger change if a (sub) setting gets re-enabled | ||
| // We are about to enable this setting. | ||
| // If this setting is currently disabled, note that we are triggering a change. | ||
| // Alternatively, if disabling this setting causes any other settings to be | ||
| // enabled due to it being disabled, then also note that we are triggering a change. | ||
| if ((targetValue == true && !this.settingIsEnabled(setting)) || (enabledChildren)) //Only trigger change if a (sub) setting gets re-enabled | ||
| triggeredChange = true; | ||
|
|
||
| // targetValue = true => This setting will be enabled. | ||
| // targetValue = false => This setting will be disabled. | ||
| this.global.generator_settingsVisibilityMap[setting] = targetValue; | ||
| }); | ||
| } | ||
|
|
||
| // If a setting won't be forcibly disabled, allow conditions to update the setting | ||
| for (let settingName in conditionalSettingUpdates) { | ||
| if (!settingsDisabled.includes(settingName)) { | ||
| this.global.generator_settingsMap[settingName] = conditionalSettingUpdates[settingName]['value']; | ||
| this.global.generator_settingsVisibilityMap[settingName] = conditionalSettingUpdates[settingName]['enabled']; | ||
| // TODO: Revisit for 'visible' when/if the "visibility" and "enabled" logic are more decoupled and we have more direct control. (See 'settingIsEnabled' and 'settingIsFullyHidden') | ||
| triggeredChange = true; | ||
| } | ||
| } | ||
|
|
||
| return triggeredChange; | ||
| } | ||
|
|
||
| private getConditionallyChangedSettingsForOption(settingOption: any) { | ||
| let conditionalSettingUpdates = {}; | ||
| if (settingOption["conditionally_controls_setting"] != null) { | ||
| settingOption["conditionally_controls_setting"].forEach(setting => { | ||
|
|
||
| let dependentSetting = this.global.findSettingByName(setting); | ||
| if (dependentSetting.conditional_controls != null) { | ||
| let targetSettingState = this.getTargetSettingStateFromConditions(dependentSetting); | ||
| let currentSettingState = this.getSettingCurrentState(dependentSetting); | ||
|
|
||
| // If any part of the setting would change, save the new setting state for later | ||
| if (currentSettingState['value'] != targetSettingState['value'] || | ||
| currentSettingState['enabled'] != targetSettingState['enabled'] || | ||
| currentSettingState['visible'] != targetSettingState['visible'] | ||
| ) { | ||
| conditionalSettingUpdates[dependentSetting.name] = targetSettingState; | ||
| } | ||
| } | ||
| }); | ||
| } | ||
| return conditionalSettingUpdates; | ||
| } | ||
|
|
||
|
|
||
| private getTargetSettingStateFromConditions(setting: any) { | ||
| // Start with the current state as the target | ||
| // If no conditions change the target state, then we effectively just return the current state | ||
| let targetSettingState = this.getSettingCurrentState(setting); | ||
|
|
||
| // There may be multiple combinations of conditions that may alter this setting. | ||
| // We'll check each one, and if one of them passes we'll use that to determine the setting's state. | ||
| let settingConditions = setting.conditional_controls; | ||
| let conditionHasDisabled = false; | ||
| for (let conditionName in settingConditions) { | ||
| var conditionToTest = settingConditions[conditionName]; | ||
| let conditionList = conditionToTest['conditions']; | ||
| let conditionsPassed = []; | ||
| for (let i = 0; i < conditionList.length; i++) { | ||
| let condition = conditionList[i]; | ||
| let partialConditionPassed = false; | ||
| // Only one of these conditional settings has to match the given value | ||
| for (let conditionalSettingName in condition) { | ||
| // If the conditional setting is currently set to the conditional value... | ||
| if (condition[conditionalSettingName] == this.global.generator_settingsMap[conditionalSettingName]) { | ||
| partialConditionPassed = true; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| conditionsPassed.push(partialConditionPassed); | ||
| }; | ||
|
|
||
| // If one full condition passed, we'll use that condition's target state | ||
| if (!conditionsPassed.includes(false)) { | ||
| // TODO: Define priority rules so we know what should take precedent. | ||
| // - Option 1: First that passes has priority => just early exit | ||
| // - Option 2: Last that passes has priority => could result in mixed data sets if "condition1" sets some of the state and later "condition 3" sets other parts | ||
| // - Option 3: Manually define priority inside the blob => basically option 1 with extra logic. But what if two options have the same priority? First or last wins? | ||
| // If the condition sets one of these keys, we'll use that value. Otherwise use the current value. | ||
| targetSettingState['value'] = conditionToTest['value'] != null ? conditionToTest['value'] : targetSettingState['value']; | ||
| targetSettingState['enabled'] = conditionToTest['enabled'] != null ? conditionToTest['enabled'] : targetSettingState['enabled']; | ||
| targetSettingState['visible'] = conditionToTest['visible'] != null ? conditionToTest['visible'] : targetSettingState['visible']; | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMPORTANT: This 'visible' flag currently has no effect. This is because currently the only control over visibility is the I'm on the fence about removing this option entirely. It could be nice to have it ready to roll, but part of me also doesn't like the idea of having a flag that could be set which does absolutely nothing. Could result in some confusion if someone tries to set in in That said, I'm reasonably confident that anyone updating settings there has a 99% chance to just use what's already in |
||
| if (targetSettingState['enabled'] == false) { | ||
| conditionHasDisabled = true; | ||
| } | ||
| break; // First condition that passes wins and takes priority | ||
| } | ||
| } | ||
|
|
||
| // The setting is currently disabled, but no conditions are attempting to disable it. | ||
| // Let's re-enable it and the old "disable" logic can take priority if needed. | ||
| if (!conditionHasDisabled && targetSettingState['enabled'] == false) { | ||
| targetSettingState['enabled'] = true; | ||
| } | ||
|
|
||
| return targetSettingState; | ||
| } | ||
|
|
||
| clearDeactivationsOfSetting(setting: any) { | ||
|
|
||
| let enabledChildren = false; | ||
|
|
@@ -1319,7 +1446,7 @@ export class GeneratorComponent implements OnInit { | |
|
|
||
| this.global.getGlobalVar('generatorSettingsArray').forEach(tab => tab.sections.forEach(section => section.settings.forEach(checkSetting => { | ||
|
|
||
| if (skipSetting && checkSetting.name === skipSetting || !this.global.generator_settingsVisibilityMap[checkSetting.name]) //Disabled settings can not alter visibility anymore | ||
| if (skipSetting && checkSetting.name === skipSetting || !this.settingIsEnabled(checkSetting.name)) //Disabled settings can not alter visibility anymore | ||
| return; | ||
|
|
||
| if (checkSetting["type"] === "Checkbutton" || checkSetting["type"] === "Radiobutton" || checkSetting["type"] === "Combobox" || checkSetting["type"] === "SearchBox") { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -469,6 +469,7 @@ export class GUIGlobal implements OnDestroy { | |
|
|
||
| async parseGeneratorGUISettings(guiSettings, userSettings) { | ||
| const isRGBHex = /[0-9A-Fa-f]{6}/; | ||
| globalThis.Settings = {}; | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This enables the user to access the setting's details from the browser console. Handy for debugging, testing, confirming current state, and whatever else. |
||
|
|
||
| //Intialize settings maps | ||
| for (let tabIndex = 0; tabIndex < guiSettings.settingsArray.length; tabIndex++) { | ||
|
|
@@ -528,6 +529,20 @@ export class GUIGlobal implements OnDestroy { | |
|
|
||
| this.generator_settingsVisibilityMap[setting.name] = true; | ||
|
|
||
| // Bind a property as a function that returns an object representing this setting for easy debugging | ||
| // By using the setting name as the property name, it allows for auto-complete and fuzzy searching/suggestions | ||
| // This works by binding a property to a getter function that has the current 'this' value bound to the function context | ||
| Object.defineProperty(globalThis.Settings, setting.name, { | ||
| get: () => { | ||
| return { | ||
| // Object representing the current state of this setting. Add more values as you see fit. | ||
| enabled: this.generator_settingsVisibilityMap[setting.name], | ||
| value: this.generator_settingsMap[setting.name], | ||
| _json: this.findSettingByName(setting.name), | ||
| }; | ||
| }, | ||
| }); | ||
|
|
||
| if (setting.type == "SearchBox" && userSettings && setting.name in userSettings) { //Special parsing for SearchBox data | ||
|
|
||
| let valueArray = []; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this doesn't do anything different in its current state, this opens up possibilities in the (near?) future to more dynamically control what defines a setting as "enabled" or "disabled".