feat(stages): add alpha/beta pre-release markers to features#2511
feat(stages): add alpha/beta pre-release markers to features#2511SkrubbySkrubInAShrub wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces an end-to-end Alpha/Beta release-stage system: CMake parses feature INI files for Alpha/Beta flags and bakes named sets into a generated ChangesAlpha/Beta Release Stage System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.22.0)OpenGrep fatal error (exit code 2): [00.11][ERROR]: Error: exception Unix_error: No such file or directory stat src/Menu/FeatureListRenderer.cpp 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. Comment |
Actionable Suggestions
|
|
CI will complain about UW version due to it being bumped down to 0.2.0 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
CMakeLists.txt (1)
236-259: Add a conventional-commit title and issue reference in PR metadataSuggested title:
feat(features): add alpha/beta release stages
Suggested issue linkage (if applicable):Implements #<id>/Addresses #<id>.As per coding guidelines, "When reviewing PRs, please provide suggestions for: Conventional Commit Titles ... [and] Issue References."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CMakeLists.txt` around lines 236 - 259, The PR metadata needs to be updated to follow conventional commit guidelines. Update the pull request title to follow the conventional commit format as `feat(features): add alpha/beta release stages` and add an issue reference (either `Implements #<id>` or `Addresses #<id>` depending on the issue being resolved) in the PR description to establish traceability. These changes should be made in the PR title and description fields on the pull request, not in the code itself.Source: Coding guidelines
.claude/CLAUDE.md (1)
337-371: Suggested PR metadata update (title + issue linkage).Suggested Conventional Commit title:
feat(audit): add stage-aware version promotionsSuggested issue reference line in PR description (as applicable):
Implements #<issue-number>(orAddresses #<issue-number>)As per coding guidelines, "When reviewing PRs, please provide suggestions for Conventional Commit Titles ... [and] Issue References".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/CLAUDE.md around lines 337 - 371, Update the PR title to follow the Conventional Commit format by changing it to "feat(audit): add stage-aware version promotions" or a similarly structured title that clearly describes the feature being added. Then update the PR description to include an issue reference line such as "Implements #<issue-number>" or "Addresses #<issue-number>" as applicable to link this change to the relevant GitHub issue, per the coding guidelines for PR metadata.Source: Coding guidelines
src/Menu/FeatureListRenderer.cpp (1)
58-64: ⚡ Quick winUse a switch statement for explicit enum case handling.
The ternary operator only checks for
Alphaexplicitly, treating all other cases (includingRelease) asBeta/Warning. While this works today because the function is primarily called forBetaandAlphafeatures, it's semantically incorrect to return Warning forReleaseand fragile if the enum is extended. A switch statement would make all cases explicit and enable compile-time exhaustiveness checking.♻️ Suggested refactor with explicit enum handling
ImVec4 StageTagColor(Feature::ReleaseStage stage) { const auto& statusPalette = globals::menu->GetTheme().StatusPalette; - return stage == Feature::ReleaseStage::Alpha ? statusPalette.Error : statusPalette.Warning; + switch (stage) { + case Feature::ReleaseStage::Alpha: + return statusPalette.Error; + case Feature::ReleaseStage::Beta: + return statusPalette.Warning; + case Feature::ReleaseStage::Release: + // Release features do not render a stage marker, so this color is unused. + // Return a default to avoid undefined behavior if called incorrectly. + return ImVec4{}; + } + // Unreachable if enum is exhaustive + return ImVec4{}; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Menu/FeatureListRenderer.cpp` around lines 58 - 64, The StageTagColor function currently uses a ternary operator that only explicitly checks for Feature::ReleaseStage::Alpha and implicitly treats all other cases (including Release) as Warning, which is semantically incorrect and fragile. Replace the ternary operator with a switch statement that explicitly handles all enum cases (Alpha, Beta, Release, and any others) with their corresponding status palette colors. This will make the enum handling explicit and enable compile-time exhaustiveness checking.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/feature_version_audit.py`:
- Around line 1348-1352: The statement setting fa['needs_flag_removal'] = False
is executing unconditionally after the remove_stage_flag() call, even when the
function returns False (fails). Move the line `fa['needs_flag_removal'] = False`
inside the `if remove_stage_flag(fa['ini_path']):` block so that the flag is
only cleared when the flag removal actually succeeds. Apply the same fix at line
1374 where this pattern is also present.
---
Nitpick comments:
In @.claude/CLAUDE.md:
- Around line 337-371: Update the PR title to follow the Conventional Commit
format by changing it to "feat(audit): add stage-aware version promotions" or a
similarly structured title that clearly describes the feature being added. Then
update the PR description to include an issue reference line such as "Implements
#<issue-number>" or "Addresses #<issue-number>" as applicable to link this
change to the relevant GitHub issue, per the coding guidelines for PR metadata.
In `@CMakeLists.txt`:
- Around line 236-259: The PR metadata needs to be updated to follow
conventional commit guidelines. Update the pull request title to follow the
conventional commit format as `feat(features): add alpha/beta release stages`
and add an issue reference (either `Implements #<id>` or `Addresses #<id>`
depending on the issue being resolved) in the PR description to establish
traceability. These changes should be made in the PR title and description
fields on the pull request, not in the code itself.
In `@src/Menu/FeatureListRenderer.cpp`:
- Around line 58-64: The StageTagColor function currently uses a ternary
operator that only explicitly checks for Feature::ReleaseStage::Alpha and
implicitly treats all other cases (including Release) as Warning, which is
semantically incorrect and fragile. Replace the ternary operator with a switch
statement that explicitly handles all enum cases (Alpha, Beta, Release, and any
others) with their corresponding status palette colors. This will make the enum
handling explicit and enable compile-time exhaustiveness checking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c1e14fdb-fde4-4241-8383-0241f2f9cc5e
📒 Files selected for processing (10)
.claude/CLAUDE.mdCMakeLists.txtcmake/FeatureVersions.h.infeatures/Unified Water/Shaders/Features/UnifiedWater.inipackage/SKSE/Plugins/CommunityShaders/Translations/en.jsonsrc/Feature.cppsrc/Feature.hsrc/Features/UnifiedWater.hsrc/Menu/FeatureListRenderer.cpptools/feature_version_audit.py
💤 Files with no reviewable changes (1)
- src/Features/UnifiedWater.h
|
✅ A pre-release build is available for this PR: |
1 participant
Summary
Introduces a release-maturity stage system for features. A feature can now declare itself
AlphaorBetain its.ini[Info]section; the stage is baked intoFeatureVersions.hat build time alongside the existing core-feature list, and is used at runtime to auto-disable pre-release features, show colored[ALPHA]/[BETA]tags in the feature list and header, and drive stage-aware versioning in the version-audit tool.UnifiedWater is the first feature to use this: it is marked
Beta = Trueand versioned0-2-0accordingly.Changes
Build (CMakeLists.txt / cmake/FeatureVersions.h.in)
.iniforAlpha/Betaflags (line-anchored regex to avoid false positives from settings keys likeWaterAlpha = 1or Nexus metadata keys likenexusbeta = on). Truthy values aretrue,1,yes,on(case-insensitive);Alphatakes precedence overBeta.FEATURE_ALPHA_NAMESandFEATURE_BETA_NAMESlists and substitutes them into the newstd::unordered_set<std::string_view>constants inFeatureVersions.h, following the same pattern asFEATURE_CORE_NAMES.Runtime C++ API (src/Feature.h / src/Feature.cpp)
ReleaseStageenum (Release,Beta,Alpha) added toFeature.GetReleaseStage()looks the short name up in the baked sets. Virtual so features can override if needed.IsAlpha()/IsBeta()convenience predicates.static GetReleaseStageTag(ReleaseStage)returns the localized[ALPHA]/[BETA]string (empty for Release). Static and takes the already-resolved stage so callers avoid a redundant hash-set lookup.IsDisabledByDefault()now returnstruefor any non-Release stage; theUnifiedWateroverride that manually returnedtrueis removed.UI (src/Menu/FeatureListRenderer.cpp)
StageTagColor()helper maps stages toStatusPalette.Error(Alpha) andStatusPalette.Warning(Beta).ImGui::SameLineafter the selectable name; the feature header draws it bottom-aligned to the right of the feature title, before the version string.GetReleaseStage()once and pass the result through, so no redundant lookup occurs.Translations (package/SKSE/Plugins/CommunityShaders/Translations/en.json)
menu.features.tag_alpha:"[ALPHA]"menu.features.tag_beta:"[BETA]"UnifiedWater (features/Unified Water / src/Features/UnifiedWater.h)
1-0-2→0-2-0(entering Beta baseline per the versioning convention).Beta = Trueflag added to the ini.IsDisabledByDefault() override { return true; }removed.Version audit tool (tools/feature_version_audit.py)
Alpha/Betaflags from ini content (stage_from_content,get_stage_from_ini,get_prior_stage).propose_new_versionis now stage-aware:0-1-0; entering Beta:0-2-0.alpha → betatransition: minor bump, patch reset.1-0-0and setsneeds_flag_removal.0.x.1.x → 0-2-0would never satisfy the existing>check).remove_stage_flag()strips the flag line from the ini;--apply-bumpscalls it automatically on breaking-change promotion.needs_flag_removal.Summary by CodeRabbit
[ALPHA],[BETA]) now render in the Features UI with theme-colored styling