feat: add feature constraints

[alandtse]

Summary by CodeRabbit

• New Features
  • Cross-feature constraint system enforcing and explaining runtime setting incompatibilities with human-readable tooltips (including boot-disable notes).
  • Constraint-aware UI controls (checkboxes/sliders) that show forced values and disable inputs when constrained.
  • Homepage section listing active constraints and a reactive in-app warning dialog when new constraints appear.
  • VR and upscaling UI now surface constraint warnings/notes and respect forced setting values; improved VR-related guidance for TerrainBlending.
• Chores
  • Added setting to skip the reactive constraint warning popup.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a runtime feature-constraints system (types + query APIs), a Feature hook to report active constraints, constraint-aware UI helpers and home-page listing, VR and feature integrations that consult/force constrained settings, reactive in-menu warnings, several signature/include updates, and some duplicate function definitions.

Changes

Cohort / File(s)	Summary
Core constraint infra src/Feature.h, src/FeatureConstraints.h, src/FeatureConstraints.cpp	Introduce FeatureConstraints namespace, SettingId, Constraint, ConstraintResult types and APIs (GetConstraints, GetAllActiveConstraints, BuildConstraintTooltip, FormatConstraintValue). Add Feature::GetActiveConstraints() hook and include.
Feature implementations (constraints / VR) src/Features/TerrainBlending.h, src/Features/TerrainBlending.cpp, src/Features/Upscaling.h, src/Features/Upscaling.cpp	TerrainBlending now reports VR constraint and SupportsVR() returns true; Upscaling methods made const, added GetFrameGenerationFrameTime(), and both provide GetActiveConstraints() to declare depth-buffer-culling constraints when VR is active.
VR integration src/Features/VR.h, src/Features/VR.cpp	UpdateDepthBufferCulling signature changed to accept FeatureConstraints::SettingId; VR now queries constraint results, forces/ignores values based on constraints, logs constraint sources, and uses constraint-aware UI.
Constraint-aware UI src/Utils/UI.h, src/Utils/UI.cpp	Add Util::ConstrainedUI with Checkbox, SliderFloat, SliderInt that consult FeatureConstraints::GetConstraints() and render forced values disabled with tooltips. Note: duplicated ConstrainedUI definitions were introduced in the diff.
Home page / constraints listing src/Menu/HomePageRenderer.h, src/Menu/HomePageRenderer.cpp	Add RenderActiveConstraintsSection() and integrate it into home rendering to show an active-constraints table. Diff contains a duplicated implementation of this function.
Feature list & reactive warnings src/Menu/FeatureListRenderer.h, src/Menu/FeatureListRenderer.cpp	Add reactive constraint-warning modal, file-scope state to track known constraints, propagate pendingFeatureSelection through RenderRightColumn, convert DrawMenuVisitor helpers to instance methods, and add RenderReactiveConstraintWarningDialog(). Signature change: RenderRightColumn(..., std::string& pendingFeatureSelection).
Menu settings / serialization src/Menu.h, src/Menu.cpp	Add Menu::Settings::SkipConstraintWarning (bool) and include it in serialization defaults.
Misc / includes & globals src/Feature.h, src/Features/VR.h, src/Utils/UI.*, src/Menu/FeatureListRenderer.cpp	Multiple files now include FeatureConstraints.h; new file-scope globals for reactive-warning tracking added; several signature changes and a few duplicate definitions introduced.

Sequence Diagram(s)

sequenceDiagram
  participant Feature as "Feature (TerrainBlending / Upscaling)"
  participant Constraints as "FeatureConstraints"
  participant UI as "ConstrainedUI / HomePageRenderer"
  participant VR as "VR"
  Feature->>Constraints: GetActiveConstraints()
  Constraints->>Constraints: Aggregate per SettingId
  UI->>Constraints: GetConstraints(SettingId) for control rendering / tooltips
  Constraints-->>UI: ConstraintResult (isConstrained, forcedValue, sources)
  UI->>User: Render disabled control + tooltip when constrained
  VR->>Constraints: GetConstraints(SettingId) before applying change
  Constraints-->>VR: ConstraintResult
  VR->>VR: Apply forcedValue if constrained, else apply requested value
  VR-->>Logs: Log constraint sources when forcing a value

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• feat(VR): separate interior/exterior options for depth buffer culling #1362: Overlapping VR depth-buffer-culling refactor and call-site changes now made constraint-aware.
• refactor: break apart menu #1330: Touches Menu/FeatureListRenderer and Utils/UI areas that are modified here (menu UI and rendering integrations).
• fix(terrain blending): disable vr support #1799: Overlaps TerrainBlending VR support/behavior changes and may conflict with the new SupportsVR() change.

Suggested reviewers

• doodlum
• davo0411
• jiayev

Poem

🐇 I hopped through headers, found rules in tow,

Sliders hushed and checkboxes bowed low,
VR asked for silence on depth’s deep song,
Home page points who sang those notes so strong,
A carrot-cheer for constraints—hop along!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.89% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat: add feature constraints' clearly and concisely summarizes the main change—introducing a feature constraints system across multiple files and modules.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Using provided base ref: b931d7e
Using base ref: b931d7e
Base commit date: 2026-02-04T23:20:25-08:00 (Wednesday, February 04, 2026 11:20 PM)
No actionable suggestions for changed features.

[Copilot]

Pull request overview

This PR introduces a feature constraint system to manage runtime incompatibilities between graphics features. It replaces the previous ad-hoc approach (where VR manually checked if upscaling was active) with a systematic framework where features can declare constraints on other features' settings.

Changes:

• Added FeatureConstraints system (new files) that allows features to declare constraints on other features' settings at runtime
• Created ConstrainedUI helpers that automatically disable and tooltip constrained settings
• Refactored VR depth buffer culling to use constraints instead of directly checking upscaling state
• Implemented constraint providers in Upscaling and TerrainBlending features
• Added home page section showing active constraints to users

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/FeatureConstraints.h	New header defining constraint system API with SettingId, Constraint, and ConstraintResult types
src/FeatureConstraints.cpp	Implementation of constraint query functions including GetConstraints and GetAllActiveConstraints
src/Feature.h	Added GetActiveConstraints() virtual method to Feature base class
src/Utils/UI.h	Added ConstrainedUI namespace with constraint-aware checkbox and slider declarations
src/Utils/UI.cpp	Implementation of ConstrainedUI helpers with automatic disabling and tooltips
src/Features/Upscaling.h	Added GetActiveConstraints override declaration
src/Features/Upscaling.cpp	Implements constraints to disable VR depth buffer culling when upscaling is active
src/Features/TerrainBlending.h	Changed SupportsVR to true and added GetActiveConstraints override
src/Features/TerrainBlending.cpp	Implements constraints and UI warnings for VR depth buffer culling conflicts
src/Features/VR.cpp	Refactored to use ConstrainedUI helpers and constraint checking instead of direct upscaling queries
src/Menu/HomePageRenderer.h	Added RenderActiveConstraintsSection declaration
src/Menu/HomePageRenderer.cpp	Implements collapsible table showing all active constraints with tooltips

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (5)

src/FeatureConstraints.cpp (2)

43-49: Consider using std::unordered_set for processed settings lookup.

The linear search through processedSettings on every constraint has O(n²) complexity in the worst case. Using a hash-based container would improve this to O(n) average.

♻️ Proposed optimization

 std::vector<std::pair<SettingId, ConstraintResult>> GetAllActiveConstraints()
 {
 	std::vector<std::pair<SettingId, ConstraintResult>> allConstraints;
-	std::vector<SettingId> processedSettings;
+	std::unordered_set<std::string> processedKeys;  // featureShortName + settingPath

 	for (auto* feature : Feature::GetFeatureList()) {
 		if (!feature->loaded)
 			continue;

 		auto constraints = feature->GetActiveConstraints();
 		for (const auto& constraint : constraints) {
-			// Check if we've already processed this setting
-			bool found = false;
-			for (const auto& processed : processedSettings) {
-				if (processed == constraint.targetSetting) {
-					found = true;
-					break;
-				}
-			}
-
-			if (!found) {
-				processedSettings.push_back(constraint.targetSetting);
+			std::string key = constraint.targetSetting.featureShortName + "|" + constraint.targetSetting.settingPath;
+			if (processedKeys.insert(key).second) {
 				auto result = GetConstraints(constraint.targetSetting);
 				if (result.isConstrained) {
 					allConstraints.push_back({ constraint.targetSetting, result });
 				}
 			}
 		}
 	}

 	return allConstraints;
 }

---

106-132: WouldCauseConflicts only processes bool constraints.

Constraints with int or float forced values are silently ignored and won't appear in outConflicts. If this is intentional (only bool constraints are supported for conflict detection), consider documenting this limitation or logging a warning for non-bool constraints.

src/Features/Upscaling.cpp (1)

1114-1118: Avoid const_cast by making IsUpscalingActive() const.

The const_cast here is a workaround for IsUpscalingActive() not being declared const. Since IsUpscalingActive() only reads resolutionScale and calls GetUpscaleMethod() (which also only reads settings), both methods could be made const.

♻️ Proposed fix in header and implementation

In Upscaling.h:

-	bool IsUpscalingActive();
+	bool IsUpscalingActive() const;

In Upscaling.cpp:

-bool Upscaling::IsUpscalingActive()
+bool Upscaling::IsUpscalingActive() const
 {
 	auto method = GetUpscaleMethod();

Then remove the const_cast:

 std::vector<FeatureConstraints::Constraint> Upscaling::GetActiveConstraints() const
 {
 	std::vector<FeatureConstraints::Constraint> constraints;

-	// Check if upscaling is active - need to cast away const for method call
-	auto* self = const_cast<Upscaling*>(this);
-	if (!self->IsUpscalingActive()) {
+	if (!IsUpscalingActive()) {
 		return constraints;
 	}

src/Utils/UI.cpp (1)

1707-1830: Show forced values when constrained to avoid misleading UI state.

Right now the widgets display the stored value even when a constraint forces a different runtime value. Consider rendering with the forced value while leaving the persisted setting unchanged; this keeps the UI truthful without mutating user preferences.

♻️ Suggested pattern (apply to Checkbox/SliderFloat/SliderInt)

-	if (constraint.isConstrained) {
-		ImGui::BeginDisabled();
-	}
-
-	bool changed = ImGui::Checkbox(label, value);
+	bool displayValue = constraint.isConstrained ? std::get<bool>(constraint.forcedValue) : *value;
+	if (constraint.isConstrained) {
+		ImGui::BeginDisabled();
+	}
+	bool changed = ImGui::Checkbox(label, &displayValue);
+	if (!constraint.isConstrained) {
+		*value = displayValue;
+	}

src/FeatureConstraints.h (1)

1-104: Add a tracking issue reference if this feature has one.

Since this PR introduces a new feature, consider adding a GitHub keyword like “Implements #123” in the PR description if there’s an existing tracking issue. As per coding guidelines: Suggest adding appropriate GitHub keywords (e.g., “Implements #123” for features).

[github-actions]

✅ A pre-release build is available for this PR:
Download

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@src/Features/VR.cpp`:
- Around line 1651-1672: In UpdateDepthBufferCulling, avoid calling
std::get<bool> on constraint.forcedValue without checking its type: replace the
direct std::get<bool> usage with a safe check via
std::get_if<bool>(&constraint.forcedValue) and only apply/assign forcedValue
when the pointer is non-null; if it's null, log a warning (include
constraint.sources and any identifying info) and skip forcing so you don't throw
std::bad_variant_access; keep the existing gDepthBufferCulling update and
logger.info behavior when forcedValue is valid.

In `@src/Utils/UI.cpp`:
- Around line 1705-1791: When a setting is constrained (in Checkbox,
SliderFloat, SliderInt), the control should display the forced value instead of
the stored value without mutating the stored setting: obtain constraint via
FeatureConstraints::GetConstraints(settingId), and if constraint.isConstrained
create a temporary local value initialized from constraint.forcedValue (convert
to bool/float/int as appropriate), pass that temporary to
ImGui::Checkbox/SliderFloat/SliderInt so the UI shows the enforced value, keep
returning false and still call ImGui::BeginDisabled()/EndDisabled() and
RenderConstraintTooltip(constraint) as before; do not write back into the
original value pointer.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Menu/HomePageRenderer.cpp (1)

17-27: ⚠️ Potential issue | 🟡 Minor

Clickable “Constrained By” tooltip can imply multiple navigation targets.

The click navigates only to the first source, but the tooltip text uses the full constrainedBy list. Consider wording it as “first constraint source” (or the first source name) to avoid misleading users when multiple sources exist.

Also applies to: 254-343

🧹 Nitpick comments (2)

src/FeatureConstraints.cpp (1)

41-59: Consider avoiding repeated full scans when aggregating constraints.

GetAllActiveConstraints() calls GetConstraints() per unique setting, which re-iterates all features each time. If this ends up in a per-frame UI path, consider a single-pass aggregation (e.g., map keyed by SettingId) or caching for the current frame to keep it O(n) on feature count.

src/Menu/FeatureListRenderer.cpp (1)

659-692: Consider caching GetAllActiveConstraints() to reduce per-frame UI rendering cost.

GetAllActiveConstraints() is called every frame during feature settings rendering (line 660) and performs nested iteration over all features twice. You could cache the result with a dirty flag set when features change state, querying only when necessary rather than on every UI frame.

Regarding constraint "flapping" and the warning popup: the code's own comments (lines 653–658) explicitly document this as intentional design—it's meant to catch both same-frame changes (e.g., TerrainBlending toggle) and next-frame changes (e.g., Upscaling's deferred resolutionScale update via the render loop). This is the feature working as designed, not a UX problem requiring debouncing.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/Utils/UI.cpp`:
- Around line 1709-1808: The fallback branches in Checkbox, SliderFloat, and
SliderInt disable the control but don't call
RenderConstraintTooltip(constraint), so users don't see why the setting is
constrained; update each fallback path inside Checkbox, SliderFloat, and
SliderInt to call RenderConstraintTooltip(constraint) after the
BeginDisabled()/EndDisabled() block (before returning false) so the tooltip is
shown even when the forcedValue type mismatches, keeping the existing disabled
behavior and return values unchanged.

🧹 Nitpick comments (1)

src/Menu.cpp (1)

1-1: Consider a scoped Conventional Commit title + issue reference.

Suggested title: feat(menu): add feature constraints. If this implements a tracked feature, consider adding an issue reference like “Implements #123” or “Related to #123” in the PR description.

[doodlum]

Does terrain blending actually work in VR without issue?

[alandtse]

@doodlum yes it blends. It's just the extra bugs like the depth culling which I've confirmed. I don't know the cause of the issue trying to be fixed in #1789 though. But we can iteratively try to fix it.