Skip to content

Share QML role names across game models - #28

Merged
btsouth merged 1 commit into
mainfrom
codex/cleanup-redundant-code
Sep 5, 2026
Merged

Share QML role names across game models#28
btsouth merged 1 commit into
mainfrom
codex/cleanup-redundant-code

Conversation

@btsouth

@btsouth btsouth commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Use one shared table for the 23 QML role names repeated across nine game models. Each model keeps its additional roles.

Validation:

  • All role IDs and per-model name mappings are unchanged.
  • Clean integration with current main; the 1.6 fixes are preserved.
  • Release build and all 41 tests pass on local integration candidate 2de11d5.
  • Existing PR CI passes on x86_64 and aarch64.

Maintainer authorized merging this behavior-preserving cleanup based on review and automated validation.

Summary by CodeRabbit

  • Refactor
    • Centralized game role-name mappings across supported game library integrations.
    • Preserved existing role names and exposed behavior while reducing duplicated mappings.
    • Maintained integration-specific roles such as installation status and launch targets.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 22aa278b-6548-4133-a10d-d7f731a0b832

📥 Commits

Reviewing files that changed from the base of the PR and between b8c0c5d and 8810be7.

📒 Files selected for processing (10)
  • src/library/BattleNetGameModel.cpp
  • src/library/FaugusGameModel.cpp
  • src/library/GameRoles.h
  • src/library/HeroicGameModel.cpp
  • src/library/LutrisGameModel.cpp
  • src/library/MockGameModel.cpp
  • src/library/Pcsx2GameModel.cpp
  • src/library/RetroArchGameModel.cpp
  • src/library/RyujinxGameModel.cpp
  • src/library/SteamGameModel.cpp

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change centralizes common Qt role-name mappings in GameRoles::names(). Game model implementations now reuse this mapping and add only their model-specific roles.

Changes

Role name centralization

Layer / File(s) Summary
Shared role-name helper
src/library/GameRoles.h
Adds GameRoles::names() with the common role-to-name mappings and the required Qt headers.
Game model migration
src/library/*GameModel.cpp
Updates Battle.net, Faugus, Heroic, Lutris, Mock, PCSX2, RetroArch, Ryujinx, and Steam models to use GameRoles::names(). Model-specific LaunchTarget and Installed roles remain explicitly added where required.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 8810b

Game models now share their common QML role names while preserving model-specific roles, with no concrete current-head behavior regression identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 10 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: sharing QML role names across game models.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/cleanup-redundant-code

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

Comment @coderabbitai help to get the list of available commands.

@btsouth
btsouth marked this pull request as ready for review September 5, 2026 02:47

@btsouth btsouth left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

No actionable findings. All nine models retain exactly the same role IDs and names, including their extra roles. The change merges cleanly with current main, preserves the 1.6 fixes, and the local integration candidate 2de11d5 passes all 41 release tests. Ready for the maintainer smoke test before merge.

@btsouth
btsouth merged commit 2de11d5 into main Sep 5, 2026
8 of 9 checks passed
@btsouth

btsouth commented Sep 5, 2026

Copy link
Copy Markdown
Owner Author

Merged after review and validation against current main. All role IDs and names are unchanged, and all 41 tests pass. This cleanup does not change the published 1.6.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant