feat(): Controller Performance Changes (WinHandler)

[phobos665]

Description

Very small changes to improve performance without negatively impacting Controller behaviour.

Found 2 issues to adjust so increase performance:

1. We create a byteArray[15] on every single controller call and then assign the values. Moved to a top-level declared variable so we don't need to constantly assign a new variable. Will need to be adjusted later so that we can pre-assign for multiple controllers when ready. (I believe that multiple-controllers is still WIP).
2. We have a race-condition where we may end up creating two executive threads. So now we check whether or not that one is running prior to starting up the runningThread. (Should remove us having 2 threads running at the same time).

Note: This still requires a bunch of testing etc.

Recording

Type of Change

• Bug fix
• Performance / stability improvement
• Compatibility improvements
• Other (requires prior approval)

Checklist

• If I have access to #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.
• This change aligns with the current project scope (core functionality, stability, or performance). If not, it has been explicitly approved beforehand.
• I have attached a recording of the change.
• I have read and agree to the contribution guidelines in CONTRIBUTING.md.

---

Summary by cubic

Pre-allocated the controller sdlButtons array and added a guard to prevent starting the send thread twice. This removes per-frame allocations and fixes a race that could create duplicate threads, improving controller performance and stability (single-controller path only).

^{Written for commit 9f32503. Summary will update on new commits. Review in cubic}

Summary by CodeRabbit

• Refactor
  • Improved gamepad input handling efficiency.
  • Refined thread management logic for better reliability.

[coderabbitai]

📝 Walkthrough

Walkthrough

WinHandler refactors memory allocation for SDL button state and thread startup safety. A persistent sdlButtons byte array replaces per-call local allocation across method boundaries. The start() method now guards send-thread initialization with running checks to prevent duplicate thread starts.

Changes

Gamepad memory and thread safety improvements

Layer / File(s)	Summary
Persistent button state buffer app/src/main/java/com/winlator/winhandler/WinHandler.java	Class-level sdlButtons byte array is allocated once and reused across calls. The local allocation in sendMemoryFileState() is removed, and sendVirtualGamepadState() now populates the persistent field instead of a local array.
Send-thread startup guards app/src/main/java/com/winlator/winhandler/WinHandler.java	Two conditional guards in start() check !this.running before setting running = true and invoking startSendThread(), preventing duplicate thread initialization.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A rabbit hops through memory lanes,
Reusing buffers, breaking chains,
Threads now guard against their double,
Saving allocations—trouble be gone!
Persistent fields, so smart and clean. 🎉

🚥 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%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically refers to controller performance changes, aligning with the main objective of optimizing the WinHandler through pre-allocation and thread guard logic.
Description check	✅ Passed	The description covers all required template sections: a clear explanation of changes and rationale, identified the type of change (performance improvement), and completed all critical checklist items. A recording was noted as not attached, but non-critical given testing acknowledgment.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[cubic-dev-ai]

No issues found across 1 file

_{Re-trigger cubic}

[coderabbitai]

Actionable comments posted: 1

🤖 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/winhandler/WinHandler.java`:
- Around line 551-554: The two unsynchronized check-then-set guards around the
field running permit concurrent start() calls to invoke startSendThread() twice;
replace both guards with a single thread-safe startup guard: change running to
an AtomicBoolean (or make it volatile and protect with a synchronized block) and
use compareAndSet(false, true) in start() to atomically flip the flag and call
startSendThread() exactly once; remove the duplicate check at the other site so
startSendThread() is only invoked from the single guarded path (referencing the
running field, start() method, and startSendThread()).

🪄 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: eb57b0ae-4ef6-490f-83f0-4324647c1556

📥 Commits

Reviewing files that changed from the base of the PR and between 089e177 and 9f32503.

📒 Files selected for processing (1)

• app/src/main/java/com/winlator/winhandler/WinHandler.java

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make send-thread startup guard atomic and single-sourced.

Line 551 and Line 575 both do unsynchronized check-then-set on running, so concurrent start() calls can still pass the guard and invoke startSendThread() twice. Consolidate to one guarded block and make visibility/thread-safety explicit.

Suggested fix

@@
-    private boolean running;
+    private volatile boolean running;
+    private final Object lifecycleLock = new Object();
@@
-        if(!this.running) {
-            this.running = true;
-            startSendThread();
-        }
+        synchronized (lifecycleLock) {
+            if (this.running) return;
+            this.running = true;
+            startSendThread();
+        }
@@
-        if(!running) {
-            running = true;
-            startSendThread();
-        }
+        // send thread already started in the lifecycle guard above

Also applies to: 575-578

🤖 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/winhandler/WinHandler.java` around lines 551 -
554, The two unsynchronized check-then-set guards around the field running
permit concurrent start() calls to invoke startSendThread() twice; replace both
guards with a single thread-safe startup guard: change running to an
AtomicBoolean (or make it volatile and protect with a synchronized block) and
use compareAndSet(false, true) in start() to atomically flip the flag and call
startSendThread() exactly once; remove the duplicate check at the other site so
startSendThread() is only invoked from the single guarded path (referencing the
running field, start() method, and startSendThread()).