feat(nui-libgdx): replace touch pointer removal logic with configurable stickiness - #67
feat(nui-libgdx): replace touch pointer removal logic with configurable stickiness#67BenjaminAmos wants to merge 1 commit into
Conversation
06df70b to
87e428e
Compare
soloturn
left a comment
soloturn
left a comment
There was a problem hiding this comment.
AI-assisted review. Filed by agent driven by @soloturn via GDD.
Traced the state machine: pointerCooldowns[pointer] is set to pointerStickiness on touch, decremented once per update() call while > 0, and the pointer is only pushed off-screen once it hits 0 - a straightforward generalization of the original single-update grace period (which this replaces) into a configurable N-update one. Verified getPosition/update() still fall through to the same real-position read as before when the cooldown hasn't expired, so behavior for desktop (non-Android) callers and touched pointers is unchanged.
Compiled LibGDXMouseDevice.java plus its full dependency chain directly against gdx 1.9.14 and guava 23.0 (the versions this module actually pins to) - clean, no errors. The touched file hasn't changed on master since this PR's base commit, so it's still current despite the PR showing as "behind."
|
@BenjaminAmos you want to have this still ? |
87e428e to
46cd966
Compare
📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesPointer stickiness
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
nui-libgdx/src/main/java/org/terasology/nui/backends/libgdx/LibGDXMouseDevice.java (1)
53-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRefresh the pointer lifecycle documentation.
The documentation immediately above
pointerCooldownsstill namesremovePointerand describes a one-update delay. The implementation now stores a per-pointer countdown and decrements it inupdate(). Update the comment to describe the current cooldown behavior.🤖 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 `@nui-libgdx/src/main/java/org/terasology/nui/backends/libgdx/LibGDXMouseDevice.java` at line 53, Update the documentation immediately above LibGDXMouseDevice.pointerCooldowns to describe the per-pointer cooldown countdown and its decrement during update(), removing the outdated removePointer reference and one-update-delay description.
🤖 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
`@nui-libgdx/src/main/java/org/terasology/nui/backends/libgdx/LibGDXMouseDevice.java`:
- Around line 63-66: Update the public LibGDXMouseDevice constructor to validate
that pointerStickiness is non-negative before assigning it, rejecting negative
values while preserving zero as valid for immediate removal.
- Around line 122-126: Update the cooldown handling in
LibGDXMouseDevice.update() so active pointer positions are refreshed before
their cooldowns are decremented, or otherwise move decrementing until after
sampling. Ensure pointer stickiness no longer depends on getPosition(int
pointer) being called for every pointer before each update, while preserving the
existing cooldown behavior.
---
Nitpick comments:
In
`@nui-libgdx/src/main/java/org/terasology/nui/backends/libgdx/LibGDXMouseDevice.java`:
- Line 53: Update the documentation immediately above
LibGDXMouseDevice.pointerCooldowns to describe the per-pointer cooldown
countdown and its decrement during update(), removing the outdated removePointer
reference and one-update-delay description.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 28fb3222-ab1c-46db-9790-e64be4e2273a
📒 Files selected for processing (1)
nui-libgdx/src/main/java/org/terasology/nui/backends/libgdx/LibGDXMouseDevice.java
| public LibGDXMouseDevice(int pointerStickiness) { | ||
| NUIInputProcessor.init(); | ||
| removePointer = new boolean[MAX_POINTERS]; | ||
| this.pointerStickiness = pointerStickiness; | ||
| pointerCooldowns = new int[MAX_POINTERS]; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject negative pointerStickiness values.
The public constructor accepts a negative update count. For -1, Line 91 stores a negative cooldown, and Line 92 treats it as expired. Invalid input therefore silently becomes zero stickiness. Validate pointerStickiness >= 0; keep zero valid for immediate removal.
Proposed fix
public LibGDXMouseDevice(int pointerStickiness) {
+ if (pointerStickiness < 0) {
+ throw new IllegalArgumentException("pointerStickiness must be non-negative");
+ }
NUIInputProcessor.init();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public LibGDXMouseDevice(int pointerStickiness) { | |
| NUIInputProcessor.init(); | |
| removePointer = new boolean[MAX_POINTERS]; | |
| this.pointerStickiness = pointerStickiness; | |
| pointerCooldowns = new int[MAX_POINTERS]; | |
| public LibGDXMouseDevice(int pointerStickiness) { | |
| if (pointerStickiness < 0) { | |
| throw new IllegalArgumentException("pointerStickiness must be non-negative"); | |
| } | |
| NUIInputProcessor.init(); | |
| this.pointerStickiness = pointerStickiness; | |
| pointerCooldowns = new int[MAX_POINTERS]; |
🤖 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
`@nui-libgdx/src/main/java/org/terasology/nui/backends/libgdx/LibGDXMouseDevice.java`
around lines 63 - 66, Update the public LibGDXMouseDevice constructor to
validate that pointerStickiness is non-negative before assigning it, rejecting
negative values while preserving zero as valid for immediate removal.
| for (int pointer = 0; pointer < MAX_POINTERS; pointer++) { | ||
| if (pointerCooldowns[pointer] > 0) { | ||
| pointerCooldowns[pointer]--; | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect pointer polling and device update call sites.
ast-grep run --lang java --pattern '$DEVICE.getPosition($_)' .
ast-grep run --lang java --pattern '$DEVICE.update()' .Repository: MovingBlocks/TeraNUI
Length of output: 158
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find target file"
fd -a 'LibGDXMouseDevice\.java$' . || true
echo "List Java mouse/device files"
fd -a 'Mouse.*\.java$|.*Mouse.*\.java$' . | sed -n '1,200p'
echo "Search for MouseDevice references"
rg -n "MouseDevice|getPosition\\(|\\.update\\(\\)" --glob '*.java' .Repository: MovingBlocks/TeraNUI
Length of output: 8662
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Target file outline"
ast-grep outline nui-libgdx/src/main/java/org/terasology/nui/backends/libgdx/LibGDXMouseDevice.java --view expanded || true
echo "Target file relevant content"
cat -n nui-libgdx/src/main/java/org/terasology/nui/backends/libgdx/LibGDXMouseDevice.java | sed -n '1,180p'
echo "InputDevice contract"
cat -n nui-input/src/main/java/org/terasology/input/device/InputDevice.java | sed -n '1,220p' || true
echo "MouseDevice contract"
cat -n nui-input/src/main/java/org/terasology/input/device/MouseDevice.java | sed -n '1,220p' || true
echo "KeyboardDevice/Device update call sites"
rg -n "extends .*Update|implements .*Update|void update\\(|\\.update\\(\\)|getPointer|getPointers|pointerCooldowns|pointerStickiness|pointerCooldown" --glob '*.java' .Repository: MovingBlocks/TeraNUI
Length of output: 14410
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find update callers with mouse context"
rg -n -C 3 "\.update\(\)" --glob '*.java' . | rg -C 5 "mouse|Mouse|InputDevice|InputProcessor|NUIInputProcessor" || true
echo "Search NUIInputProcessor update and sampling"
fd -a 'NUIInputProcessor\.java$|.*InputProcessor.*\.java$' .
rg -n "NUIInputProcessor|MouseInputQueue|mouse|Pointer|update\\(|getPosition" --glob '*.java' nui nui-input nui-libgdx | sed -n '1,260p'
echo "NullMouseDevice"
cat -n nui-input/src/main/java/org/terasology/input/device/nulldevices/NullMouseDevice.javaRepository: MovingBlocks/TeraNUI
Length of output: 37804
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "CanvasImpl pointer polling/update relevant content"
cat -n nui/src/main/java/org/terasology/nui/canvas/CanvasImpl.java | sed -n '1,260p'
echo "NUIInputProcessor relevant content"
cat -n nui-libgdx/src/main/java/org/terasology/nui/backends/libgdx/NUIInputProcessor.java | sed -n '1,200p'
echo "Programmatic check: update decrements before position sampling is required by contract? (source search)"
python3 - <<'PY'
from pathlib import Path
import re
target = Path("nui-libgdx/src/main/java/org/terasology/nui/backends/libgdx/LibGDXMouseDevice.java").read_text()
iface = Path("nui-input/src/main/java/org/terasology/input/device/MouseDevice.java").read_text()
canvas = Path("nui/src/main/java/org/terasology/nui/canvas/CanvasImpl.java").read_text()
print("LibGDXMouseDevice decres in update:", bool(re.search(r'public void update\(\).*pointerCooldowns\[pointer\]--', target, re.S)))
print("getPosition(int) reloads only on touched:", bool(re.search(r'if \(Gdx\.input\.isTouched\(pointer\)\).*pointerCooldowns\[pointer\] = pointerStickiness', target, re.S)))
print("getPosition(int) returns off-screen after release unless cooldown:", bool(re.search(r'} else if \(pointerCooldowns\[pointer\] <= 0\).*return new Vector2i\(Integer\.MAX_VALUE, Integer\.MAX_VALUE\)', target, re.S)))
print("CanvasImpl update starts with polling:", bool(re.search(r'for \(int ptr = 0; ptr < getMaxPointers\(\); ptr\+\+\) \{[\s\S]*getPosition\(ptr\);', canvas)))
print("MouseDevice contract requires poll-before-update:", bool(re.search(r'Decrement cooldown|sampling order|before calling|always poll', iface)))
# Behavioral probe of local state machine with sampled vs skipped pointer.
MAX_POINTERS = 20
stickiness = 3
cooldowns = [0]*MAX_POINTERS
def getPosition(p):
pos = (10,10)
if True:
if True:
cooldowns[p] = stickiness
elif cooldowns[p] <= 0:
cooldowns[p] = 0
return (2**31-1, 2**31-1)
return pos
def update():
global cooldowns
cooldowns = [c-1 if c > 0 else 0 for c in cooldowns]
cooldowns[0] = 3
update() # skipped until release
print("skipped until release position after update:", getPosition(0))
cooldowns = [0]*MAX_POINTERS
cooldowns[0] = 3
getPosition(0) # sampled touches reset
update() # after sampling release
print("sampled once after update:", getPosition(0))
PYRepository: MovingBlocks/TeraNUI
Length of output: 18746
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find MouseDevice implementations/update calls with exact Java syntax"
python3 - <<'PY'
from pathlib import Path
import re
paths=[]
for path in Path('.').rglob('*.java'):
text=path.read_text(errors='ignore')
if re.search(r'\b(mouse|mousedevice|libgdxmousedevice)\b|getPosition\s*\(|\bupdate\s*\(\s*\)', text, re.I):
paths.append(path)
for path in paths:
text=path.read_text(errors='ignore')
lines=text.splitlines()
print(f"\n{'='*60}\n{path}")
for i,line in enumerate(lines,1):
if any(s in line for s in ['getPosition(', '.update()', '.getMaxPointers()', 'new LibGDXMouseDevice', 'processMouse', 'canvas.update', 'mouse.update']):
lo=max(1,i-4); hi=min(len(lines),i+4)
for n in range(lo,hi+1):
print(f"{n:5d}: {lines[n-1]}")
PY
echo "Search exact CursorAttachment update/call path"
cat -n nui/src/main/java/org/terasology/nui/widgets/CursorAttachment.java | sed -n '45,110p'
rg -n "update\(" nui/src/main/java/org/terasology/nui/widgets/CursorAttachment.javaRepository: MovingBlocks/TeraNUI
Length of output: 24233
Guard pointer stickiness from update() skipping
update() decrements every active cooldown immediately, but getPosition(int pointer) is the only place that refreshes a touched pointer and it returns an off-screen position once the cooldown reaches zero. This makes pointer stickiness depend on external code polling every pointer before each update; update active pointers first, or move the decrement after sampling.
🤖 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
`@nui-libgdx/src/main/java/org/terasology/nui/backends/libgdx/LibGDXMouseDevice.java`
around lines 122 - 126, Update the cooldown handling in
LibGDXMouseDevice.update() so active pointer positions are refreshed before
their cooldowns are decremented, or otherwise move decrementing until after
sampling. Ensure pointer stickiness no longer depends on getPosition(int
pointer) being called for every pointer before each update, while preserving the
existing cooldown behavior.
|
Just from re-reading this, I can already see a potential issue. By decrementing always by 1 in the |
2 participants
This is a fix that makes the UI on Android when using
nui-libgdxa bit more responsive. I found that with my previous implementation of the touch removal logic some button release events were not being processed. This was due to the pointer being removed before the button had registered the mouse release event, so the pointer was no longer being registered as hovering over the button.The new logic makes the system more configurable by employing a "stickiness" value, which is the number of updates to retain the touch pointer for at it's former position after the touch has been released. A lower stickiness makes the UI more responsive but risks missing input events. A higher stickiness means that the UI will register mouse hover events for longer after the touch has been released.