feat: task manager from qam#1121
Open
xXJSONDeruloXx wants to merge 4 commits intoutkarshdalal:masterfrom
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR introduces a new TOOLS tab to the QuickMenu component displaying active Wine processes with kill functionality. It extends QuickMenu's signature with wine handler and process list parameters, implements polling logic in XServerScreen to fetch process snapshots, enriches ProcessInfo with memory data, and adds localized UI strings across 16 languages. Changes
Sequence Diagram(s)sequenceDiagram
participant XServerScreen
participant QuickMenu
participant QuickMenuProcessRow
participant WinHandler
participant ProcessHelper
XServerScreen->>XServerScreen: User opens TOOLS tab
activate XServerScreen
XServerScreen->>XServerScreen: quickMenuToolsVisible = true
XServerScreen->>WinHandler: requestWineProcessSnapshot()
activate WinHandler
WinHandler->>ProcessHelper: listProcesses()
activate ProcessHelper
ProcessHelper->>ProcessHelper: Parse ps output<br/>(extract RSS bytes)
ProcessHelper-->>WinHandler: Return ProcessInfo list
deactivate ProcessHelper
WinHandler-->>XServerScreen: Process snapshot
deactivate WinHandler
XServerScreen->>XServerScreen: Sort by essentiality<br/>then memory usage
XServerScreen->>QuickMenu: Pass wineProcesses list
deactivate XServerScreen
XServerScreen->>QuickMenu: Render TOOLS tab
activate QuickMenu
QuickMenu->>QuickMenu: Show process count hint
loop For each process
QuickMenu->>QuickMenuProcessRow: Render process row
activate QuickMenuProcessRow
QuickMenuProcessRow->>QuickMenuProcessRow: Apply focus styling<br/>Marquee title
deactivate QuickMenuProcessRow
end
deactivate QuickMenu
User->>QuickMenuProcessRow: Tap/Click or Press Key
activate QuickMenuProcessRow
QuickMenuProcessRow->>QuickMenuProcessRow: onEndProcess callback
QuickMenuProcessRow->>WinHandler: killProcess(name, pid)
activate WinHandler
WinHandler->>WinHandler: Terminate process
deactivate WinHandler
deactivate QuickMenuProcessRow
XServerScreen->>XServerScreen: Poll interval delay
XServerScreen->>XServerScreen: Repeat fetch loop<br/>if TOOLS still visible
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/ui/component/QuickMenu.kt (2)
585-585: Optional: Hoist scroll state toQuickMenufor consistency.The
scrollStateis defined locally here, whereas other tabs (hudScrollState,effectsScrollState,controllerScrollState) have their scroll states defined at theQuickMenulevel. This means the TOOLS tab loses scroll position when switching tabs.Given the 2-second refresh interval for the process list, this may be acceptable, but hoisting the state would provide consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/component/QuickMenu.kt` at line 585, Move the local val scrollState = rememberScrollState() up into the QuickMenu composable alongside hudScrollState, effectsScrollState and controllerScrollState so the TOOLS tab preserves its scroll position; then replace the local declaration in the TOOLS tab with a reference to that hoisted scrollState (or pass it into the TOOLS tab child composable) so all tabs use scroll state defined at QuickMenu level for consistent behavior.
614-617: Consider adding a confirmation step before killing processes.Tapping a row immediately kills the process without confirmation. While this provides quick access, accidental taps could terminate important processes. Consider either:
- A brief toast/snackbar with undo capability, or
- A confirmation dialog for processes that aren't marked as safe to kill.
This is optional given the "tap to close" design intent mentioned in the PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/component/QuickMenu.kt` around lines 614 - 617, The quick-tap handler in QuickMenu.kt currently calls winHandler?.killProcess(process.name, process.pid) directly in the onEndProcess lambda; update this to present a confirmation UI (either a confirmation dialog for non-safe processes or a brief snackbar with an undo for quick closes) before calling winHandler?.killProcess. Specifically, change the onEndProcess flow to check a process safety flag (e.g., process.safeToKill or process.isCritical) and if not safe show a confirmation dialog and only call killProcess on confirm, or for quick-close show a snackbar that delays the actual call and cancels it if the user taps undo; ensure the final kill call still uses winHandler?.killProcess(process.name, process.pid).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 245-284: The current code in requestWineProcessSnapshot replaces
WinHandler's single OnGetProcessInfoListener and can race with
startExitWatchForUnmappedGameWindow which does the same; instead, create a
single shared dispatcher/listener in this class (e.g., a private val
processInfoDispatcher: OnGetProcessInfoListener or a small manager object) that
you register once via
winHandler.setOnGetProcessInfoListener(processInfoDispatcher) and have that
dispatcher multiplex events to callers. Change requestWineProcessSnapshot to
enqueue a CompletableDeferred and expectedCount into the dispatcher (keyed by a
request token or by storing the deferred + logic in a list) rather than calling
setOnGetProcessInfoListener, and update startExitWatchForUnmappedGameWindow to
use the same dispatcher for its callbacks; ensure the finally blocks only
unregister the dispatcher when no pending consumers remain and synchronize
access with a mutex to serialize ownership.
In `@app/src/main/res/values-fr/strings.xml`:
- Line 252: Replace the awkward French phrasing in the string resource
tools_wine_processes_running_hint: change the text "%1$d en cours,
touchez/cliquez sur un processus pour le fermer" to a natural French sentence
that includes the noun "processus", for example "%1$d processus en cours,
touchez ou cliquez sur un processus pour le fermer" (or "%1$d processus en
cours, appuyez ou cliquez sur un processus pour le fermer") so the UI copy reads
naturally.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/ui/component/QuickMenu.kt`:
- Line 585: Move the local val scrollState = rememberScrollState() up into the
QuickMenu composable alongside hudScrollState, effectsScrollState and
controllerScrollState so the TOOLS tab preserves its scroll position; then
replace the local declaration in the TOOLS tab with a reference to that hoisted
scrollState (or pass it into the TOOLS tab child composable) so all tabs use
scroll state defined at QuickMenu level for consistent behavior.
- Around line 614-617: The quick-tap handler in QuickMenu.kt currently calls
winHandler?.killProcess(process.name, process.pid) directly in the onEndProcess
lambda; update this to present a confirmation UI (either a confirmation dialog
for non-safe processes or a brief snackbar with an undo for quick closes) before
calling winHandler?.killProcess. Specifically, change the onEndProcess flow to
check a process safety flag (e.g., process.safeToKill or process.isCritical) and
if not safe show a confirmation dialog and only call killProcess on confirm, or
for quick-close show a snackbar that delays the actual call and cancels it if
the user taps undo; ensure the final kill call still uses
winHandler?.killProcess(process.name, process.pid).
🪄 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: 1c14806e-e3b7-43a5-bac7-75b53e23e6b9
📒 Files selected for processing (17)
app/src/main/java/app/gamenative/ui/component/QuickMenu.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/com/winlator/core/ProcessHelper.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-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
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 17 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/res/values-ru/strings.xml">
<violation number="1" location="app/src/main/res/values-ru/strings.xml:1168">
P2: Russian count-based hint uses a single fixed phrase, causing grammatically incorrect text for many numeric values.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt:277">
P1: Quick-menu process polling races with exit-watch by overwriting the single global WinHandler process listener, which can mix callbacks and corrupt process snapshots.</violation>
</file>
<file name="app/src/main/java/com/winlator/core/ProcessHelper.java">
<violation number="1" location="app/src/main/java/com/winlator/core/ProcessHelper.java:138">
P2: New RSS parsing from `ps` output is unchecked; malformed output can throw `NumberFormatException` and break subprocess listing flow.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| return try { | ||
| winHandler.setOnGetProcessInfoListener(listener) |
Contributor
There was a problem hiding this comment.
P1: Quick-menu process polling races with exit-watch by overwriting the single global WinHandler process listener, which can mix callbacks and corrupt process snapshots.
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/screen/xserver/XServerScreen.kt, line 277:
<comment>Quick-menu process polling races with exit-watch by overwriting the single global WinHandler process listener, which can mix callbacks and corrupt process snapshots.</comment>
<file context>
@@ -241,6 +242,48 @@ private fun buildEssentialProcessAllowlist(): Set<String> {
+ }
+
+ return try {
+ winHandler.setOnGetProcessInfoListener(listener)
+ winHandler.listProcesses()
+ withTimeoutOrNull(EXIT_PROCESS_RESPONSE_TIMEOUT_MS) {
</file context>
| <string name="tab_local">Пользовательское</string> | ||
| <string name="tab_steam">Steam</string> | ||
| <string name="task_manager">Менеджер задач</string> | ||
| <string name="tools_wine_processes_running_hint">%1$d запущено, нажмите/кликните по процессу, чтобы закрыть его</string> |
Contributor
There was a problem hiding this comment.
P2: Russian count-based hint uses a single fixed phrase, causing grammatically incorrect text for many numeric values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/res/values-ru/strings.xml, line 1168:
<comment>Russian count-based hint uses a single fixed phrase, causing grammatically incorrect text for many numeric values.</comment>
<file context>
@@ -1165,6 +1165,7 @@ https://gamenative.app
<string name="tab_local">Пользовательское</string>
<string name="tab_steam">Steam</string>
<string name="task_manager">Менеджер задач</string>
+ <string name="tools_wine_processes_running_hint">%1$d запущено, нажмите/кликните по процессу, чтобы закрыть его</string>
<string name="template_percent">%1$d процент.</string>
<string name="thumbstick_buttons">Кнопки джойстика</string>
</file context>
| String user = parts[0]; | ||
| int pid = Integer.parseInt(parts[1]); | ||
| int ppid = Integer.parseInt(parts[2]); | ||
| long rssKb = Long.parseLong(parts[4]); |
Contributor
There was a problem hiding this comment.
P2: New RSS parsing from ps output is unchecked; malformed output can throw NumberFormatException and break subprocess listing flow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/com/winlator/core/ProcessHelper.java, line 138:
<comment>New RSS parsing from `ps` output is unchecked; malformed output can throw `NumberFormatException` and break subprocess listing flow.</comment>
<file context>
@@ -111,59 +117,29 @@ public static int exec(String command, String[] envp, File workingDir, Callback<
String user = parts[0];
int pid = Integer.parseInt(parts[1]);
int ppid = Integer.parseInt(parts[2]);
+ long rssKb = Long.parseLong(parts[4]);
String processName = parts[8];
</file context>
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 a new view to quick menu that lets you see a list of running procs, and quickly kill them if needed.
Recording
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 a Task Manager tab in the Quick Menu with a live list of running Wine EXEs and memory usage. You can now see and end processes without leaving the game.
QuickMenuwith a live “Running EXEs” list; refreshes every 2s while the tab is open, shows non‑essential apps first, and lets you tap/click to end a process viaWinHandler(WOW64 processes marked “*32”).ProcessHelperparsespsRSS and exposes bytes for each process to support memory display.Written for commit 1604e34. Summary will update on new commits.
Summary by CodeRabbit