Add per-control appearance customization#1672
Open
Nightwalker743 wants to merge 7 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds control appearance editing and preview support, persists appearance fields, updates native rendering and touch behavior, refactors profile copying, and adds localized strings for appearance controls. ChangesControl Appearance Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ElementEditorDialog
participant ControlAppearance
participant ControlElement
participant InputControlsView
User->>ElementEditorDialog: edit appearance values
ElementEditorDialog->>ControlAppearance: capture current appearance
ControlAppearance->>ControlElement: apply appearance for preview
ControlElement->>InputControlsView: invalidate
User->>ElementEditorDialog: save changes
ElementEditorDialog->>ControlElement: persist edited properties
sequenceDiagram
participant XServerScreen
participant InputControlsProfileCopy
participant ControlsProfile
participant InputControlsView
XServerScreen->>InputControlsProfileCopy: copy profile elements
InputControlsProfileCopy->>ControlsProfile: load and clear elements
InputControlsProfileCopy->>InputControlsProfileCopy: recreate elements and bindings
InputControlsProfileCopy->>InputControlsView: invalidate view
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Nightwalker743
force-pushed
the
feature/on-screen-control-appearance
branch
from
036a8c8 to
c9e18bd
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 23 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/ui/component/dialog/ControlAppearanceEditor.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/dialog/ControlAppearanceEditor.kt:69">
P2: The color preset list is defined with raw hex literals inside the composable module. To stay consistent with the project's theme convention and the team's review guidance, these preset values should be declared as named constants in the shared theme (e.g., `ui/theme/Color.kt`) and referenced here. This makes palette updates easier and keeps UI colors centralized.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/ui/component/dialog/ControlAppearanceEditor.kt:523">
P2: Hardcoded preview text "DJ" should be extracted to strings.xml for localization. Even though it is preview placeholder text, it is displayed to the user in the shooter mode appearance preview, and the rest of this file consistently uses string resources for all user-facing text.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| } | ||
| } | ||
|
|
||
| private val controlColorPresets = listOf( |
Contributor
There was a problem hiding this comment.
P2: The color preset list is defined with raw hex literals inside the composable module. To stay consistent with the project's theme convention and the team's review guidance, these preset values should be declared as named constants in the shared theme (e.g., ui/theme/Color.kt) and referenced here. This makes palette updates easier and keeps UI colors centralized.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/component/dialog/ControlAppearanceEditor.kt, line 69:
<comment>The color preset list is defined with raw hex literals inside the composable module. To stay consistent with the project's theme convention and the team's review guidance, these preset values should be declared as named constants in the shared theme (e.g., `ui/theme/Color.kt`) and referenced here. This makes palette updates easier and keeps UI colors centralized.</comment>
<file context>
@@ -0,0 +1,619 @@
+ }
+}
+
+private val controlColorPresets = listOf(
+ 0xffffff, 0xf44336, 0xff9800, 0xffeb3b,
+ 0x4caf50, 0x03a9f4, 0x3f51b5, 0x9c27b0
</file context>
| contentAlignment = Alignment.Center | ||
| ) { | ||
| Text( | ||
| text = "DJ", |
Contributor
There was a problem hiding this comment.
P2: Hardcoded preview text "DJ" should be extracted to strings.xml for localization. Even though it is preview placeholder text, it is displayed to the user in the shooter mode appearance preview, and the rest of this file consistently uses string resources for all user-facing text.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/component/dialog/ControlAppearanceEditor.kt, line 523:
<comment>Hardcoded preview text "DJ" should be extracted to strings.xml for localization. Even though it is preview placeholder text, it is displayed to the user in the shooter mode appearance preview, and the rest of this file consistently uses string resources for all user-facing text.</comment>
<file context>
@@ -0,0 +1,619 @@
+ contentAlignment = Alignment.Center
+ ) {
+ Text(
+ text = "DJ",
+ color = drawColor,
+ style = MaterialTheme.typography.labelMedium,
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/gamenative/ui/component/dialog/ElementEditorDialog.kt (1)
1089-1122: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAvoid resetting the element type on discard
element.type = originalTypecallsreset()and clears persisted state likeiconIdfor non-SHOOTER_MODE elements; usesetTypeWithoutReset(originalType)so cancel restores the original element state.🤖 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 `@app/src/main/java/app/gamenative/ui/component/dialog/ElementEditorDialog.kt` around lines 1089 - 1122, The discard handler in ElementEditorDialog is resetting too much state by assigning element.type directly, which clears persisted fields like iconId for non-SHOOTER_MODE elements. Update the cancel/discard flow in the TextButton onClick block to restore the original type with setTypeWithoutReset(originalType) instead of direct type assignment, while keeping the rest of the original state restoration logic unchanged.
🧹 Nitpick comments (4)
app/src/main/java/app/gamenative/ui/component/dialog/ControlAppearanceEditor.kt (1)
92-539: 🚀 Performance & Scalability | 🔵 TrivialPreview composables look correct; some duplication in
RangeButtonPreview.The horizontal/vertical branches of
RangeButtonPreview(Lines 291-436) duplicate a large amount of near-identical path-building logic. Since this is preview-only rendering, it's not urgent, but could be extracted into a shared orientation-agnostic helper to reduce duplication.🤖 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 `@app/src/main/java/app/gamenative/ui/component/dialog/ControlAppearanceEditor.kt` around lines 92 - 539, The `RangeButtonPreview` composable contains heavily duplicated path-building logic in its horizontal and vertical orientation branches. Refactor this preview rendering by extracting the common outline/active-segment drawing steps into a shared orientation-agnostic helper, keeping `RangeButtonPreview` focused on choosing dimensions and delegating to that helper. Preserve the existing behavior for `active`, `segmentCount`, and the orientation-specific segment layout while reducing the repeated `Path` construction in `RangeButtonPreview`.app/src/main/res/values/strings.xml (1)
397-419: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGeneric string names risk future collisions.
preview,active, andapply_allare very generic top-level keys for a large shared strings file. They don't currently collide with existing keys, but as the file grows, similarly generic names are more likely to clash or be reused unintentionally. Consider more specific names likecontrol_preview,control_active_label,control_apply_allfor durability.🤖 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 `@app/src/main/res/values/strings.xml` around lines 397 - 419, The strings in the control appearance section use overly generic resource names that could collide later; rename the shared keys near the control appearance strings to more specific identifiers. Update the entries for preview, active, and apply_all to names scoped to this feature, such as control_preview, control_active_label, and control_apply_all, and make sure all references elsewhere in the app are updated to match the new string resource names.app/src/main/java/com/winlator/inputcontrols/ControlElement.java (2)
774-936: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCorrectly consumes activeColor/normalColor/oldColor; visually affected by the default-color issue noted above.
Segment logic itself (active-boundary detection, outline gap, active-segment overlay) is sound; the lack of default press feedback stems from
getAppearanceDrawColor(see comment on Lines 34-37/516-532/701-714).One minor inconsistency: the vertical branch clips with
inputControlsView.getPath()(Line 869) while the horizontal branch consistently uses the localpathvariable (Line 800) — same object, so harmless, but worth aligning for clarity.🧹 Minor consistency fix
canvas.save(); path.addRoundRect(boundingBox.left, startY, boundingBox.right, boundingBox.bottom, radius, radius, Path.Direction.CW); - canvas.clipPath(inputControlsView.getPath()); + canvas.clipPath(path);🤖 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 `@app/src/main/java/com/winlator/inputcontrols/ControlElement.java` around lines 774 - 936, The RANGE_BUTTON drawing code in ControlElement has a minor consistency issue: the vertical branch uses inputControlsView.getPath() for clipping while the horizontal branch uses the local path variable. Update the vertical branch to clip with the same local Path instance already reset and used in this case, keeping the clipping flow consistent and easier to maintain without changing behavior.
1001-1012: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache the tint filter in
drawIcon()PorterDuffColorFilteris created on every icon draw; reuse one per tint color instead of allocating during rendering.🤖 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 `@app/src/main/java/com/winlator/inputcontrols/ControlElement.java` around lines 1001 - 1012, The drawIcon() method in ControlElement is allocating a new PorterDuffColorFilter on every render, which should be avoided. Reuse the tint filter per tintColor instead of creating it each time drawIcon() runs, and keep the existing paint/color-filter flow in drawIcon() and its caller path intact while adding a cache keyed by the tint value.
🤖 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 `@app/src/main/java/com/winlator/inputcontrols/ControlElement.java`:
- Around line 34-37: The pressed-but-not-selected state in ControlElement still
uses buttonActiveColor directly, so when both defaults are 0x00ffffff the active
feedback becomes invisible. Update the relevant color selection logic in
ControlElement (including the pressed rendering paths in the draw/update
methods) to fall back to the regular button color or another visible default
when buttonActiveColor is unset or matches the transparent default, while
preserving custom active colors when provided.
---
Outside diff comments:
In `@app/src/main/java/app/gamenative/ui/component/dialog/ElementEditorDialog.kt`:
- Around line 1089-1122: The discard handler in ElementEditorDialog is resetting
too much state by assigning element.type directly, which clears persisted fields
like iconId for non-SHOOTER_MODE elements. Update the cancel/discard flow in the
TextButton onClick block to restore the original type with
setTypeWithoutReset(originalType) instead of direct type assignment, while
keeping the rest of the original state restoration logic unchanged.
---
Nitpick comments:
In
`@app/src/main/java/app/gamenative/ui/component/dialog/ControlAppearanceEditor.kt`:
- Around line 92-539: The `RangeButtonPreview` composable contains heavily
duplicated path-building logic in its horizontal and vertical orientation
branches. Refactor this preview rendering by extracting the common
outline/active-segment drawing steps into a shared orientation-agnostic helper,
keeping `RangeButtonPreview` focused on choosing dimensions and delegating to
that helper. Preserve the existing behavior for `active`, `segmentCount`, and
the orientation-specific segment layout while reducing the repeated `Path`
construction in `RangeButtonPreview`.
In `@app/src/main/java/com/winlator/inputcontrols/ControlElement.java`:
- Around line 774-936: The RANGE_BUTTON drawing code in ControlElement has a
minor consistency issue: the vertical branch uses inputControlsView.getPath()
for clipping while the horizontal branch uses the local path variable. Update
the vertical branch to clip with the same local Path instance already reset and
used in this case, keeping the clipping flow consistent and easier to maintain
without changing behavior.
- Around line 1001-1012: The drawIcon() method in ControlElement is allocating a
new PorterDuffColorFilter on every render, which should be avoided. Reuse the
tint filter per tintColor instead of creating it each time drawIcon() runs, and
keep the existing paint/color-filter flow in drawIcon() and its caller path
intact while adding a cache keyed by the tint value.
In `@app/src/main/res/values/strings.xml`:
- Around line 397-419: The strings in the control appearance section use overly
generic resource names that could collide later; rename the shared keys near the
control appearance strings to more specific identifiers. Update the entries for
preview, active, and apply_all to names scoped to this feature, such as
control_preview, control_active_label, and control_apply_all, and make sure all
references elsewhere in the app are updated to match the new string resource
names.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f45c24a0-cc01-4aab-9f5e-de8e7011cd49
📒 Files selected for processing (23)
app/src/main/java/app/gamenative/ui/component/dialog/ControlAppearanceEditor.ktapp/src/main/java/app/gamenative/ui/component/dialog/ElementEditorDialog.ktapp/src/main/java/app/gamenative/ui/screen/xserver/InputControlsProfileCopy.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/com/winlator/inputcontrols/ControlElement.javaapp/src/main/java/com/winlator/inputcontrols/ControlsProfile.javaapp/src/main/java/com/winlator/inputcontrols/RangeScroller.javaapp/src/main/java/com/winlator/widget/InputControlsView.javaapp/src/main/res/values-da/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values-ja/strings.xmlapp/src/main/res/values-ko/strings.xmlapp/src/main/res/values-pl/strings.xmlapp/src/main/res/values-pt-rBR/strings.xmlapp/src/main/res/values-ro/strings.xmlapp/src/main/res/values-ru/strings.xmlapp/src/main/res/values-uk/strings.xmlapp/src/main/res/values-zh-rCN/strings.xmlapp/src/main/res/values-zh-rTW/strings.xmlapp/src/main/res/values/strings.xml
Comment on lines
+34
to
+37
| public static final int DEFAULT_BUTTON_COLOR = 0x00ffffff; | ||
| public static final int DEFAULT_BUTTON_ACTIVE_COLOR = 0x00ffffff; | ||
| public static final float INHERIT_BUTTON_OPACITY = -1.0f; | ||
| public static final float DEFAULT_BUTTON_STROKE_SCALE = 1.0f; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'DEFAULT_BUTTON_ACTIVE_COLOR|DEFAULT_BUTTON_COLOR|buttonActiveColorCustom|getAppearanceDrawColor' --type=java --type=ktRepository: utkarshdalal/GameNative
Length of output: 188
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file structure first.
ast-grep outline app/src/main/java/com/winlator/inputcontrols/ControlElement.java --view expanded || true
echo '--- FILE SIZE ---'
wc -l app/src/main/java/com/winlator/inputcontrols/ControlElement.java
echo '--- CONSTANTS / HELPERS / DRAW ---'
rg -n 'DEFAULT_BUTTON_ACTIVE_COLOR|DEFAULT_BUTTON_COLOR|buttonActiveColorCustom|getAppearanceDrawColor|getEditorSelectionDrawColor|getRuntimeSelectedDrawColor|drawActiveSegmentOutline|activeStrokeWidth|activeSegment|secondaryColor' app/src/main/java/com/winlator/inputcontrols/ControlElement.java
echo '--- RELEVANT SECTIONS ---'
sed -n '1,130p' app/src/main/java/com/winlator/inputcontrols/ControlElement.java
echo '--- MIDDLE ---'
sed -n '480,560p' app/src/main/java/com/winlator/inputcontrols/ControlElement.java
echo '--- DRAW ---'
sed -n '680,740p' app/src/main/java/com/winlator/inputcontrols/ControlElement.java
echo '--- SEARCH WHOLE REPO FOR RELATED FIX ---'
rg -n 'default selected-control coloring|buttonActiveColorCustom|getSecondaryColor\(\)|DEFAULT_BUTTON_ACTIVE_COLOR|drawActiveSegmentOutline|active segment outline' app/src/main/javaRepository: utkarshdalal/GameNative
Length of output: 24195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline app/src/main/java/com/winlator/inputcontrols/ControlElement.java --view expanded || true
wc -l app/src/main/java/com/winlator/inputcontrols/ControlElement.java
rg -n 'DEFAULT_BUTTON_ACTIVE_COLOR|DEFAULT_BUTTON_COLOR|buttonActiveColorCustom|getAppearanceDrawColor|getEditorSelectionDrawColor|getRuntimeSelectedDrawColor|drawActiveSegmentOutline|activeStrokeWidth|activeSegment|secondaryColor' app/src/main/java/com/winlator/inputcontrols/ControlElement.java
sed -n '1,130p' app/src/main/java/com/winlator/inputcontrols/ControlElement.java
sed -n '480,560p' app/src/main/java/com/winlator/inputcontrols/ControlElement.java
sed -n '680,740p' app/src/main/java/com/winlator/inputcontrols/ControlElement.java
rg -n 'default selected-control coloring|buttonActiveColorCustom|getSecondaryColor\(\)|DEFAULT_BUTTON_ACTIVE_COLOR|drawActiveSegmentOutline|active segment outline' app/src/main/javaRepository: utkarshdalal/GameNative
Length of output: 24046
Default active color still needs a fallback ControlElement.java:35, 516-529, 706-710 still leaves the pressed-but-not-selected path using buttonActiveColor directly, and both defaults are 0x00ffffff. Without a custom active color, controls render the same color on press, so the new active-segment feedback stays invisible.
🤖 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 `@app/src/main/java/com/winlator/inputcontrols/ControlElement.java` around
lines 34 - 37, The pressed-but-not-selected state in ControlElement still uses
buttonActiveColor directly, so when both defaults are 0x00ffffff the active
feedback becomes invisible. Update the relevant color selection logic in
ControlElement (including the pressed rendering paths in the draw/update
methods) to fall back to the regular button color or another visible default
when buttonActiveColor is unset or matches the transparent default, while
preserving custom active colors when provided.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds more customization for on-screen controls.
This lets users customize each control’s color, pressed color, opacity, and line thickness. Buttons also get a per-button shooter mode look-through toggle.
Also added a live preview, copy/apply/reset appearance actions, and made sure copied profiles/layouts preserve the new appearance settings.
These were requested features from a few different threads:
https://discord.com/channels/1378308569287622737/1517555219955716308
https://discord.com/channels/1378308569287622737/1522259263169232896
https://discord.com/channels/1378308569287622737/1518119934784180375
Recording
https://drive.google.com/file/d/1Hh_61uJzpyi4u8LkgsW_ghpG-xdvZ4rH/view?usp=sharing
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Adds per‑control appearance customization for on‑screen controls with live preview and a per‑button shooter look‑through toggle. Copy/apply/reset now include control size, and profile/layout copy preserves these settings.
New Features
Bug Fixes
Written for commit 9e95b10. Summary will update on new commits.
Summary by CodeRabbit