Improvements to CPU register view: #1983

rocketz · 2025-12-11T20:40:42Z

Colorized registers for easier overview
Easier in-line editing of values
Now also shows register as signed decimal

- Colorized registers for easier overview - Easier in-line editiing of values - Now also shows register as signed decimal

coderabbitai · 2025-12-11T20:40:56Z

Walkthrough

Reworked the Registers widget UI: removed the old inline edit flow, added contextMenu(...) entry points with radix-aware clipboard copy, introduced updateRegisterValue(...) to centralize parsing/apply logic, and updated GPR/PC rendering to use colored labels and inline hex/decimal inputs.

Changes

Cohort / File(s)	Summary
Header: Registers widget declarations `src/gui/widgets/registers.h`	Removed `makeEditableRegister` and deleted editor-related private state (`m_selected`, `m_registerEditor`, `m_editorToOpen`). Added declarations for `contextMenu(const char* name, uint32_t reg, uint32_t radix)` and `updateRegisterValue(PCSX::psxRegisters* registers, const std::string& val, int reg, int radix)`.
Implementation: Registers widget logic & UI `src/gui/widgets/registers.cc`	Replaced inline-edit flow with per-register `contextMenu` usage, implemented radix-aware clipboard copy, introduced `updateRegisterValue` to parse hex/decimal and write to GPR or PC, switched GPR and Misc PC displays to inline hex InputText controls, and added per-register color logic (`colorIndices`/`colors`).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Review updateRegisterValue for parsing correctness (hex/decimal), overflow handling, and register-index routing (GPR vs PC).
Verify consistency of PC inline-edit behavior across GPR and Misc tabs (selection, Enter-to-apply, focus).
Inspect context menu clipboard copy semantics for correct radix and formatting.
Check removal of editor state for any remaining references or edge cases in focus/edit lifecycle.
Validate color indexing arrays for off-by-one or out-of-range access.

Poem

🐇 I nibble bytes in fields of green,
I hop through hex and decimal scene.
A context menu, clip and paste—
Registers tidy, neat, and braced.
Hop on, commit; the rabbits feast!

Pre-merge checks and finishing touches

❌ 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%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: improvements to the CPU register view UI, which aligns with the colorization, inline editing, and decimal display enhancements in the changeset.
Description check	✅ Passed	The description directly relates to the changeset, covering three key improvements implemented: colorized registers, easier inline editing, and signed decimal display of registers.

✨ 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

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

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/gui/widgets/registers.cc (1)
106-123: Minor: Inconsistent brace style.

The else block uses a different brace style than the rest of the codebase (brace on new line vs same line). The logic itself is correct for handling both hex and signed decimal input.
     if (radix == 16) {
         newReg = strtoul(val.c_str(), &endPtr, 16);
-    }
-    else {
+    } else {
         newReg = strtoll(val.c_str(), &endPtr, 10);
     }

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b745534 and 984681f.

📒 Files selected for processing (2)

src/gui/widgets/registers.cc (6 hunks)
src/gui/widgets/registers.h (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/gui/widgets/registers.h (1)

src/gui/widgets/registers.cc (4)

contextMenu (34-54)

contextMenu (34-34)

updateRegisterValue (106-123)

updateRegisterValue (106-107)

🪛 Cppcheck (2.18.0)

src/gui/widgets/registers.cc

[error] 48-48: syntax error

(syntaxError)

[error] 186-186: failed to expand 'TYPESTRING', Invalid ## usage when expanding 'TYPESTRING64'

(preprocessorErrorDirective)

🪛 GitHub Check: CodeScene Code Health Review (main)

src/gui/widgets/registers.cc

[warning] 34-53: ❌ New issue: Bumpy Road Ahead
PCSX::Widgets::Registers::contextMenu has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is 2 blocks per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.

[warning] 139-422: ❌ Getting worse: Complex Method
PCSX::Widgets::Registers::draw increases in cyclomatic complexity from 60 to 66, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

[warning] 139-422: ❌ Getting worse: Bumpy Road Ahead
PCSX::Widgets::Registers::draw increases from 10 to 12 logical blocks with deeply nested code, threshold is 2 blocks per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.

[warning] 139-422: ❌ Getting worse: Complex Method
PCSX::Widgets::Registers::draw increases in cyclomatic complexity from 60 to 66, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

[warning] 139-422: ❌ Getting worse: Bumpy Road Ahead
PCSX::Widgets::Registers::draw increases from 10 to 12 logical blocks with deeply nested code, threshold is 2 blocks per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)

GitHub Check: macos-arm-build-and-test
GitHub Check: pcsx-redux (aarch64-linux)
GitHub Check: pcsx-redux (x86_64-linux)
GitHub Check: Codacy Static Code Analysis
GitHub Check: asan
GitHub Check: coverage
GitHub Check: aur-build
GitHub Check: cross-arm64
GitHub Check: build
GitHub Check: macos-build-and-test-toolchain
GitHub Check: toolchain

🔇 Additional comments (5)

src/gui/widgets/registers.h (1)

40-41: LGTM!

The new method declarations are clean and appropriately scoped as private members. The signatures align well with their implementations in the source file.

src/gui/widgets/registers.cc (4)

34-53: LGTM!

The context menu implementation is clean and the radix-aware clipboard copy is a good UX improvement. The static analysis "Bumpy Road" warning is expected for UI menu handling code and acceptable here.

140-147: LGTM!

The color indices and color arrays are properly sized and provide a clean way to visually distinguish register categories. The static const declaration is appropriate for this use case.

168-197: LGTM!

The GPR rendering logic is well-structured with proper handling for the read-only zero register, dual hex/decimal editing for other registers, and appropriate context menus. The colorization improves readability.

410-421: LGTM!

The Misc tab PC editing correctly uses ImGuiInputTextFlags_CharsHexadecimal with hex formatting and radix 16. This is the correct pattern that should also be used in the GPR tab.

src/gui/widgets/registers.cc

- Fixed issues robots picked up - Removed old support code for the modal edit-dialog

coderabbitai

Actionable comments posted: 0

♻️ Duplicate comments (1)

src/gui/widgets/registers.cc (1)

199-206: LGTM - Previous hex input filter bug has been fixed.

The PC input field now correctly uses ImGuiInputTextFlags_CharsHexadecimal to allow hex digits a-f.

🧹 Nitpick comments (1)

src/gui/widgets/registers.cc (1)

106-122: Consider using a named constant for the PC register index.

The magic number 34 is used to distinguish PC from GPR registers (lines 116, 203, 414). A named constant would improve readability and reduce the risk of inconsistencies.

+// Register indices: 0-31 = GPR, 32 = lo, 33 = hi, 34 = PC
+static constexpr int kPCRegisterIndex = 34;
+
 void PCSX::Widgets::Registers::updateRegisterValue(PCSX::psxRegisters* registers, const std::string& val, int reg,
                                                    int radix) {
     uint32_t newReg; 
     char* endPtr;
     if (radix == 16) {
         newReg = strtoul(val.c_str(), &endPtr, 16);
     } else {
         newReg = strtoll(val.c_str(), &endPtr, 10);
     }
     if (!*endPtr) {
-        if (reg == 34) {
+        if (reg == kPCRegisterIndex) {
             registers->pc = newReg;
         } else {
             registers->GPR.r[reg] = newReg;
         }
     }
 }

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 984681f and 759381c.

📒 Files selected for processing (2)

src/gui/widgets/registers.cc (6 hunks)
src/gui/widgets/registers.h (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

src/gui/widgets/registers.h

🧰 Additional context used

🪛 Cppcheck (2.18.0)

src/gui/widgets/registers.cc

[error] 48-48: syntax error

(syntaxError)

[error] 186-186: failed to expand 'TYPESTRING', Invalid ## usage when expanding 'TYPESTRING64'

(preprocessorErrorDirective)

🪛 GitHub Check: CodeScene Code Health Review (main)

src/gui/widgets/registers.cc

[warning] 34-53: ❌ New issue: Bumpy Road Ahead
PCSX::Widgets::Registers::contextMenu has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is 2 blocks per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.

[notice] 138-501: ✅ Getting better: Complex Method
PCSX::Widgets::Registers::draw decreases in cyclomatic complexity from 60 to 54, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)

GitHub Check: macos-arm-build-and-test
GitHub Check: pcsx-redux (x86_64-linux)
GitHub Check: pcsx-redux (aarch64-linux)
GitHub Check: Codacy Static Code Analysis
GitHub Check: aur-build
GitHub Check: build
GitHub Check: asan
GitHub Check: cross-arm64
GitHub Check: coverage
GitHub Check: macos-build-and-test-toolchain
GitHub Check: toolchain

🔇 Additional comments (3)

src/gui/widgets/registers.cc (3)

34-54: LGTM - Context menu implementation is correct.

The radix-aware clipboard copy logic handles both hex and signed decimal formats appropriately. The buffer size of 20 is sufficient for all cases. The Cppcheck syntax error at line 48 appears to be a false positive.

139-146: LGTM - Color scheme implementation is clean.

The color arrays are correctly sized for 34 registers, and the indexing in the loop is bounds-safe. The visual distinction between register categories (gray for special, cyan and white for others) improves readability.

408-418: LGTM - Misc tab PC editing is consistent with GPR tab.

The implementation correctly uses hex input flags and the shared updateRegisterValue function, ensuring consistent behavior across tabs.

Improvements to CPU register view:

984681f

- Colorized registers for easier overview - Easier in-line editiing of values - Now also shows register as signed decimal

pull-request-size bot added the size/L label Dec 11, 2025

coderabbitai bot reviewed Dec 11, 2025

View reviewed changes

src/gui/widgets/registers.cc Show resolved Hide resolved

Improvement to CPU registers view:

759381c

- Fixed issues robots picked up - Removed old support code for the modal edit-dialog

coderabbitai bot reviewed Dec 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Improvements to CPU register view: #1983

Improvements to CPU register view: #1983

Uh oh!

rocketz commented Dec 11, 2025

Uh oh!

coderabbitai bot commented Dec 11, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Improvements to CPU register view: #1983

Are you sure you want to change the base?

Improvements to CPU register view: #1983

Uh oh!

Conversation

rocketz commented Dec 11, 2025

Uh oh!

coderabbitai bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai bot commented Dec 11, 2025 •

edited

Loading