Skip to content

fix: toggle settings feature alignment#8484

Merged
lorenzo-ranciaffi merged 3 commits intodevfrom
fix/8424-toggle-settings-feature-alignment
Apr 27, 2026
Merged

fix: toggle settings feature alignment#8484
lorenzo-ranciaffi merged 3 commits intodevfrom
fix/8424-toggle-settings-feature-alignment

Conversation

@lorenzo-ranciaffi
Copy link
Copy Markdown
Contributor

Pull Request Description

Fixes #8424

What does this PR change?

This PR re-aligns all the toggle feature values to the proper configuration (every toggle was shifted by 1 by the improperly ordering of CHAT_REACTIONS_ENABLED_FEATURE on its creation).

Now all features (toggle, slider and dropdown) have a fixed index enum value so that every new addition can't affect the previous ones.

Test Steps

  1. Verify the steps in the linked issue
  2. Verify that all possible settings (especially toggles) modify the proper feature (e.g. v-sync toggle modify the v-sync behaviour, Sun Lens Flare enables/disables the sun lens flare and so on...)

Quality Checklist

  • Changes have been tested locally
  • Documentation has been updated (if required)
  • Performance impact has been considered
  • For SDK features: Test scene is included

Code Review Reference

Please review our Branch & PR Standards before submitting. It explains the automated review flow, QA/DEV approval requirements, and what each label does — especially useful for first-time contributors.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 27, 2026

@lorenzo-ranciaffi lorenzo-ranciaffi self-assigned this Apr 27, 2026
@lorenzo-ranciaffi lorenzo-ranciaffi marked this pull request as ready for review April 27, 2026 13:50
@lorenzo-ranciaffi lorenzo-ranciaffi requested review from a team as code owners April 27, 2026 13:50
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 27, 2026

Claude finished @lorenzo-ranciaffi's task in 1m 52s —— View job


Code Review

  • Gather context and read project docs
  • Review changed files
  • Analyze correctness of index assignments
  • Check for blocking issues
  • Post final review

Root-Cause Assessment

Problem: CHAT_REACTIONS_ENABLED_FEATURE was inserted into ToggleFeatures without an explicit integer value, silently shifting all subsequent enum members by +1. Because C# enums are persisted as their underlying integer in Unity .asset files, every toggle after that entry was wired to the wrong feature controller at runtime.

This PR fixes the root cause — not a symptom. Adding explicit integer literals to all three enums (ToggleFeatures, SliderFeatures, DropdownFeatures) and realigning the corresponding values in SettingsMenuConfiguration.asset directly prevents the class of bug where inserting a new member reorders existing mappings.


Correctness Verification

I cross-referenced every <Feature>k__BackingField: value in the asset against the updated enum definitions across all three binding types. All 35 entries are correctly aligned:

Binding type Entries checked All correct?
ToggleModuleBinding 12
SliderModuleBinding 12
DropdownModuleBinding 11

Selected spot-checks:

  • VSync (rid: ...747108) → Feature: 1 = GRAPHICS_VSYNC_TOGGLE_FEATURE = 1
  • Sun Lens Flare (rid: ...013200) → Feature: 13 = SUN_LENS_FLARE_FEATURE = 13 ✓ (was 11 before, the misalignment described in the issue)
  • Fullscreen (rid: ...260124) → Feature: 11 = FULLSCREEN_FEATURE = 11 ✓ (was 10 before)
  • Play Current Scene Streams Only (rid: ...600993) → Feature: 12 = PLAY_CURRENT_SCENE_STREAM_ONLY_FEATURE = 12 ✓ (was 11 before)

No Blocking Issues Found

The protective comment added above each enum is appropriate and correctly documents the invariant future contributors must respect.


REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Pure enum value anchoring and Unity asset data correction — no ECS systems, async patterns, or runtime logic modified.
QA_REQUIRED: YES

@lorenzo-ranciaffi lorenzo-ranciaffi moved this from Todo to With QA / Awaiting Review in Explorer Alpha Apr 27, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved by Claude — simple fix/chore with no blocking issues. QA approval is still required.

@github-project-automation github-project-automation Bot moved this from With QA / Awaiting Review to In Progress in Explorer Alpha Apr 27, 2026
@lorenzo-ranciaffi lorenzo-ranciaffi moved this from In Progress to With QA / Awaiting Review in Explorer Alpha Apr 27, 2026
@github-project-automation github-project-automation Bot moved this from With QA / Awaiting Review to In Progress in Explorer Alpha Apr 27, 2026
@lorenzo-ranciaffi lorenzo-ranciaffi moved this from In Progress to With QA / Awaiting Review in Explorer Alpha Apr 27, 2026
@lorenzo-ranciaffi lorenzo-ranciaffi enabled auto-merge (squash) April 27, 2026 15:12
@balducciv
Copy link
Copy Markdown

balducciv commented Apr 27, 2026

QA Pass ✅

Tested on v0.142.0-alpha-fix/8424-toggle-settings-feature-alignment-97b9020 on macOS (M3 Pro) and Windows.

Verified:

  • Original issue steps no longer reproduce
  • All toggles affect only their own named feature (V-Sync, Sun Lens Flare, See others' reactions, etc.)
  • Sliders and dropdowns correctly modify their respective settings
  • Settings persist correctly after relaunch with proper values

No regressions found. Logs are clean of any errors related to this fix.

Evidence

Mac

27.04.2026_12.38.52_REC.PR-8484.mac.mp4

Player.log

Windows

Player windows 8484.log

27.04.2026_12.43.33_REC.mp4

@github-project-automation github-project-automation Bot moved this from With QA / Awaiting Review to In Progress in Explorer Alpha Apr 27, 2026
@lorenzo-ranciaffi lorenzo-ranciaffi merged commit fb1a972 into dev Apr 27, 2026
25 of 29 checks passed
@lorenzo-ranciaffi lorenzo-ranciaffi deleted the fix/8424-toggle-settings-feature-alignment branch April 27, 2026 16:12
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Explorer Alpha Apr 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[QA] Settings | Sun Lens Flare toggle switches Explorer to fullscreen instead of enabling the effect

3 participants