Closed
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了两个问题，并给出了一些整体性的反馈：
- 60 秒重试延迟在多个地方被硬编码（
WINDOW_RETRY_DELAY_MS和 i18n 文案）；建议把它集中到一个常量或可配置项中，这样如果以后修改延迟，多个地方就不会出现不一致。 - 在使用
maaService.runAction(path, '', basePath, false)自动启动游戏时，建议在运行前校验一下gamePath是否指向了一个存在且可执行的文件，这样一旦配置错误就能更快失败，并给出更清晰的错误信息。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The 60-second retry delay is hard-coded in multiple places (WINDOW_RETRY_DELAY_MS and i18n messages); consider centralizing this as a single constant or configurable setting to avoid drift if the delay is ever changed.
- When auto-launching the game with `maaService.runAction(path, '', basePath, false)`, you might want to validate that `gamePath` points to an existing and executable file before attempting to run it, so that misconfigurations fail fast with a clearer message.
## Individual Comments
### Comment 1
<location> `src/stores/appStore.ts:1286` </location>
<code_context>
},
+ // 游戏路径设置（窗口未找到时自动启动游戏）
+ gamePath: '',
+ setGamePath: (path) => set({ gamePath: path }),
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The second `gamePath` initialization overwrites the value loaded from config, breaking persistence.
This `gamePath: ''` definition in the store’s initial state will override the earlier `config.settings.gamePath ?? ''` initialization, so any persisted path from config is discarded on startup. Please remove this initializer (and rely on `setGamePath`) or otherwise ensure the config-derived value is not overwritten.
</issue_to_address>
### Comment 2
<location> `src/components/Toolbar.tsx:486-489` </location>
<code_context>
+ }
+
+ log.info(`实例 ${targetInstance.name}: 未找到窗口，尝试启动游戏: ${path}`);
+ addLog(targetId, {
+ type: 'info',
+ message: t('action.autoLaunchGameStarting', { path }),
+ });
+
+ try {
+ await maaService.runAction(path, '', basePath, false);
+ } catch (launchErr) {
+ log.error(`实例 ${targetInstance.name}: 启动游戏失败:`, launchErr);
+ addLog(targetId, {
+ type: 'error',
+ message: t('action.autoLaunchGameFailed', { error: String(launchErr) }),
+ });
+ return false;
+ }
+
+ addLog(targetId, {
+ type: 'info',
+ message: t('action.autoLaunchGameWaiting', { seconds: 60 }),
+ });
+ await new Promise((resolve) => setTimeout(resolve, WINDOW_RETRY_DELAY_MS));
</code_context>
<issue_to_address>
**suggestion:** The hardcoded `60` seconds in the log can drift from `WINDOW_RETRY_DELAY_MS`.
Since `WINDOW_RETRY_DELAY_MS` is `60_000`, this log should derive the seconds from that constant (e.g. `WINDOW_RETRY_DELAY_MS / 1000`) so the message stays accurate if the delay changes.
```suggestion
addLog(targetId, {
type: 'info',
message: t('action.autoLaunchGameWaiting', { seconds: WINDOW_RETRY_DELAY_MS / 1000 }),
});
```
</issue_to_address>帮我提升审查质量！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The 60-second retry delay is hard-coded in multiple places (WINDOW_RETRY_DELAY_MS and i18n messages); consider centralizing this as a single constant or configurable setting to avoid drift if the delay is ever changed.
- When auto-launching the game with
maaService.runAction(path, '', basePath, false), you might want to validate thatgamePathpoints to an existing and executable file before attempting to run it, so that misconfigurations fail fast with a clearer message.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The 60-second retry delay is hard-coded in multiple places (WINDOW_RETRY_DELAY_MS and i18n messages); consider centralizing this as a single constant or configurable setting to avoid drift if the delay is ever changed.
- When auto-launching the game with `maaService.runAction(path, '', basePath, false)`, you might want to validate that `gamePath` points to an existing and executable file before attempting to run it, so that misconfigurations fail fast with a clearer message.
## Individual Comments
### Comment 1
<location> `src/stores/appStore.ts:1286` </location>
<code_context>
},
+ // 游戏路径设置（窗口未找到时自动启动游戏）
+ gamePath: '',
+ setGamePath: (path) => set({ gamePath: path }),
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The second `gamePath` initialization overwrites the value loaded from config, breaking persistence.
This `gamePath: ''` definition in the store’s initial state will override the earlier `config.settings.gamePath ?? ''` initialization, so any persisted path from config is discarded on startup. Please remove this initializer (and rely on `setGamePath`) or otherwise ensure the config-derived value is not overwritten.
</issue_to_address>
### Comment 2
<location> `src/components/Toolbar.tsx:486-489` </location>
<code_context>
+ }
+
+ log.info(`实例 ${targetInstance.name}: 未找到窗口，尝试启动游戏: ${path}`);
+ addLog(targetId, {
+ type: 'info',
+ message: t('action.autoLaunchGameStarting', { path }),
+ });
+
+ try {
+ await maaService.runAction(path, '', basePath, false);
+ } catch (launchErr) {
+ log.error(`实例 ${targetInstance.name}: 启动游戏失败:`, launchErr);
+ addLog(targetId, {
+ type: 'error',
+ message: t('action.autoLaunchGameFailed', { error: String(launchErr) }),
+ });
+ return false;
+ }
+
+ addLog(targetId, {
+ type: 'info',
+ message: t('action.autoLaunchGameWaiting', { seconds: 60 }),
+ });
+ await new Promise((resolve) => setTimeout(resolve, WINDOW_RETRY_DELAY_MS));
</code_context>
<issue_to_address>
**suggestion:** The hardcoded `60` seconds in the log can drift from `WINDOW_RETRY_DELAY_MS`.
Since `WINDOW_RETRY_DELAY_MS` is `60_000`, this log should derive the seconds from that constant (e.g. `WINDOW_RETRY_DELAY_MS / 1000`) so the message stays accurate if the delay changes.
```suggestion
addLog(targetId, {
type: 'info',
message: t('action.autoLaunchGameWaiting', { seconds: WINDOW_RETRY_DELAY_MS / 1000 }),
});
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| }, | ||
|
|
||
| // 游戏路径设置（窗口未找到时自动启动游戏） | ||
| gamePath: '', |
Contributor
There was a problem hiding this comment.
issue (bug_risk): 第二次对 gamePath 的初始化会覆盖从配置中加载的值，从而破坏持久化。
这个在 store 初始状态中的 gamePath: '' 定义会覆盖之前的 config.settings.gamePath ?? '' 初始化，因此启动时任何从配置中持久化的路径都会被丢弃。请移除这一初始化（并依赖 setGamePath），或者确保不会覆盖从配置中得到的值。
Original comment in English
issue (bug_risk): The second gamePath initialization overwrites the value loaded from config, breaking persistence.
This gamePath: '' definition in the store’s initial state will override the earlier config.settings.gamePath ?? '' initialization, so any persisted path from config is discarded on startup. Please remove this initializer (and rely on setGamePath) or otherwise ensure the config-derived value is not overwritten.
Comment on lines
+486
to
+489
| addLog(targetId, { | ||
| type: 'info', | ||
| message: t('action.autoLaunchGameWaiting', { seconds: 60 }), | ||
| }); |
Contributor
There was a problem hiding this comment.
suggestion: 日志中硬编码的 60 秒可能会与 WINDOW_RETRY_DELAY_MS 不一致。
由于 WINDOW_RETRY_DELAY_MS 是 60_000，这条日志应当从该常量推导秒数（例如 WINDOW_RETRY_DELAY_MS / 1000），这样当延迟修改时，日志信息依然是准确的。
Suggested change
| addLog(targetId, { | |
| type: 'info', | |
| message: t('action.autoLaunchGameWaiting', { seconds: 60 }), | |
| }); | |
| addLog(targetId, { | |
| type: 'info', | |
| message: t('action.autoLaunchGameWaiting', { seconds: WINDOW_RETRY_DELAY_MS / 1000 }), | |
| }); |
Original comment in English
suggestion: The hardcoded 60 seconds in the log can drift from WINDOW_RETRY_DELAY_MS.
Since WINDOW_RETRY_DELAY_MS is 60_000, this log should derive the seconds from that constant (e.g. WINDOW_RETRY_DELAY_MS / 1000) so the message stays accurate if the delay changes.
Suggested change
| addLog(targetId, { | |
| type: 'info', | |
| message: t('action.autoLaunchGameWaiting', { seconds: 60 }), | |
| }); | |
| addLog(targetId, { | |
| type: 'info', | |
| message: t('action.autoLaunchGameWaiting', { seconds: WINDOW_RETRY_DELAY_MS / 1000 }), | |
| }); |
Owner
|
不是有 “前置程序” 了吗 
|
Author
|
诶喔才发现 T^T, 那没事了, 写写其他的 |
Owner
|
要不给前置程序加一个“检测进程已存在就不启动”这种的 switch ？免得游戏已经在运行了又打开一次 |
Author
试了试好像终末地第二次打开只会多一个报错的窗口, 不会影响任务进行. 不知道其他游戏是怎样 |
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
2 participants
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.
在设置中添加了游戏路径的选项, 从而可以自动打开游戏并开始任务
例如 C:\Program Files\Hypergryph Launcher\games\EndField Game\Endfield.exe
Summary by Sourcery
添加可配置的游戏可执行文件路径，并使用该路径在未找到游戏窗口时自动启动游戏并重试连接。
新功能：
改进：
Original summary in English
Summary by Sourcery
Add a configurable game executable path and use it to auto-launch the game and retry connecting when no game window is found.
New Features:
Enhancements: