Add game lifecycle management and instance import/export tools#117
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideAdds robust multi-instance lifecycle management for Minecraft processes, shared/legacy path resolution, and instance import/export/repair on the backend, and wires it through the React frontend with new game lifecycle state, instance controls, and improved Microsoft auth handling. Updated class diagram for instance and game process managementclassDiagram
class GameProcessState {
+running_game: AsyncMutex~Option~RunningGameProcess~~
+new() GameProcessState
+default() GameProcessState
}
class RunningGameProcess {
+child: Child
+instance_id: String
+version_id: String
}
class GameExitedEvent {
+instance_id: String
+version_id: String
+exit_code: Option~i32~
+was_stopped: bool
}
class InstanceRepairResult {
+restored_instances: usize
+removed_stale_entries: usize
+created_default_active: bool
+active_instance_id: Option~String~
}
class InstancePaths {
+root: PathBuf
+metadata_versions: PathBuf
+version_cache: PathBuf
+libraries: PathBuf
+assets: PathBuf
+mods: PathBuf
+config: PathBuf
+saves: PathBuf
+resourcepacks: PathBuf
+shaderpacks: PathBuf
+screenshots: PathBuf
}
class InstanceOperation {
<<enum>>
Launch
Install
Delete
ImportExport
+label() &str
}
class ExportedInstance {
+name: String
+version_id: Option~String~
+icon_path: Option~String~
+notes: Option~String~
+mod_loader: Option~String~
+mod_loader_version: Option~String~
+jvm_args_override: Option~String~
+memory_override: Option~MemoryOverride~
+java_path_override: Option~String~
}
class InstanceConfig {
+instances: Vec~Instance~
+active_instance_id: Option~String~
}
class InstanceState {
+instances: Mutex~InstanceConfig~
+file_path: PathBuf
-operation_locks: Mutex~HashMap~String, InstanceOperation~~
+new(app_handle: &AppHandle) InstanceState
+save() Result~(), String~
+create_instance(name: String, app_handle: &AppHandle) Result~Instance, String~
+delete_instance(id: &str) Result~(), String~
+update_instance(instance: Instance) Result~(), String~
+duplicate_instance(id: &str, new_name: String, app_handle: &AppHandle) Result~Instance, String~
+begin_operation(id: &str, operation: InstanceOperation) Result~(), String~
+end_operation(id: &str)
+resolve_paths(id: &str, config: &LauncherConfig, app_handle: &AppHandle) Result~InstancePaths, String~
+resolve_directory(id: &str, folder: &str, config: &LauncherConfig, app_handle: &AppHandle) Result~PathBuf, String~
+export_instance(id: &str, archive_path: &Path) Result~PathBuf, String~
+import_instance(archive_path: &Path, app_handle: &AppHandle, new_name: Option~String~) Result~Instance, String~
+repair_instances(app_handle: &AppHandle) Result~InstanceRepairResult, String~
-insert_instance(instance: Instance, set_active_when_empty: bool) Result~(), String~
-create_instance_directory_structure(instance_dir: &Path) Result~(), String~
-validate_instance_name(config: &InstanceConfig, name: &str, exclude_id: Option~&str~) Result~(), String~
-app_dir(app_handle: &AppHandle) Result~PathBuf, String~
-instances_dir(app_handle: &AppHandle) Result~PathBuf, String~
}
GameProcessState --> RunningGameProcess : holds
Tauri_main ..> GameProcessState : manage
Tauri_main ..> InstanceState : manage
InstanceState --> InstanceConfig : manages
InstanceState --> InstancePaths : returns
InstanceState --> InstanceRepairResult : returns
InstanceState --> InstanceOperation : uses
InstanceState --> ExportedInstance : serializes
ExportedInstance ..> Instance : snapshot of metadata
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
@SourceryAI title |
Workspace change through: 3a31d301 changesets found Planned changes to release
|
There was a problem hiding this comment.
Hey - I've found 5 issues, and left some high level feedback:
- The new
InstanceState::begin_operation/end_operationlocking pattern isn’t exception-safe: several call sites (delete_instance,duplicate_instance,export_instance,import_instance,install_*flows, etc.) can return early withErr(...)and never callend_operation, leaving the instance permanently “busy”; consider using a small RAII guard that always releases the lock inDrop, or a local helper that runsend_operationin amatch/finally-style block. - The CurseForge client now hard-fails if
CURSEFORGE_API_KEYis missing or empty, which will surface as a generic error to users; it may be better to degrade gracefully (e.g., return a clear, higher-level error from the modpack APIs or gate the buttons in the UI) instead of failing deep incf_post.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `InstanceState::begin_operation`/`end_operation` locking pattern isn’t exception-safe: several call sites (`delete_instance`, `duplicate_instance`, `export_instance`, `import_instance`, `install_*` flows, etc.) can return early with `Err(...)` and never call `end_operation`, leaving the instance permanently “busy”; consider using a small RAII guard that always releases the lock in `Drop`, or a local helper that runs `end_operation` in a `match`/`finally`-style block.
- The CurseForge client now hard-fails if `CURSEFORGE_API_KEY` is missing or empty, which will surface as a generic error to users; it may be better to degrade gracefully (e.g., return a clear, higher-level error from the modpack APIs or gate the buttons in the UI) instead of failing deep in `cf_post`.
## Individual Comments
### Comment 1
<location path="src-tauri/src/core/instance.rs" line_range="547-556" />
<code_context>
+ pub fn import_instance(
</code_context>
<issue_to_address>
**issue (bug_risk):** `import_instance` also lacks a guaranteed `end_operation` on error, which can leave new instances stuck in a busy state.
Here, `begin_operation(&imported.id, InstanceOperation::ImportExport)` is called but `end_operation` is only invoked on the happy path. Any failure while iterating archive entries or updating the hydrated instance (file I/O, JSON parsing, `update_instance`, etc.) will return early and leave the operation entry uncleared. Consider wrapping `begin_operation`/`end_operation` in a guard type so import/export (and any future operations) always release the lock on both success and error paths.
</issue_to_address>
### Comment 2
<location path="src-tauri/src/core/instance.rs" line_range="342-347" />
<code_context>
Ok(instance)
}
/// Delete an instance
pub fn delete_instance(&self, id: &str) -> Result<(), String> {
+ self.begin_operation(id, InstanceOperation::Delete)?;
</code_context>
<issue_to_address>
**issue (bug_risk):** `delete_instance` can also exit with an error without clearing the per-instance operation lock.
Here `begin_operation` is called but `end_operation` only runs after `remove_dir_all` succeeds. If the instance doesn’t exist or `remove_dir_all` fails (permissions, transient FS error, etc.), the function returns early and never clears the lock, leaving the instance permanently marked as busy. Please use an operation guard (as suggested for export/import) or similar RAII pattern here so `end_operation` is always called on all paths, including errors.
</issue_to_address>
### Comment 3
<location path="packages/ui/src/models/auth.ts" line_range="102-111" />
<code_context>
+ _pollLoginStatus: async (deviceCode, onSuccess) => {
</code_context>
<issue_to_address>
**issue (bug_risk):** The async mutex in `_pollLoginStatus` is used in a way that is incompatible with common `async-mutex` APIs and can cause incorrect locking behavior.
In this implementation you `await mutex.acquire()` but still call `mutex.release()` in `finally`. In libraries like `async-mutex`, `acquire()` returns a `Promise<Releaser>`, and you must call that releaser function instead of `mutex.release()`. Mixing these patterns can cause runtime errors or leave the mutex locked. Prefer the standard pattern:
```ts
const release = await mutex.acquire();
try {
// poll logic
} finally {
release();
}
```
or use `mutex.runExclusive(...)` if available, to avoid subtle deadlocks in the login polling flow.
</issue_to_address>
### Comment 4
<location path="src-tauri/src/core/instance.rs" line_range="615-624" />
<code_context>
+ pub fn repair_instances(&self, app_handle: &AppHandle) -> Result<InstanceRepairResult, String> {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** `repair_instances` works on a cloned config and then overwrites the shared state, which can race with concurrent instance mutations.
Because `repair_instances` builds a new config clone and then assigns it back with `*self.instances.lock().unwrap() = config.clone();`, any concurrent mutations (`create_instance`, `update_instance`, `delete_instance`, etc.) that occur between the clone and the final assignment can be lost. To avoid this race and ensure atomicity relative to other mutations (including concurrent Tauri command handlers), hold the mutex for the full repair operation or perform the repair directly on the locked config.
Suggested implementation:
```rust
let mut instances_guard = self.instances.lock().unwrap();
let config = &mut *instances_guard;
let mut restored_instances = 0usize;
let mut removed_stale_entries = 0usize;
config.instances.retain(|instance| {
```
```rust
```
</issue_to_address>
### Comment 5
<location path="packages/ui/src/stores/game-store.ts" line_range="63-72" />
<code_context>
+ return get().runningInstanceId !== null;
+ },
+
+ initLifecycle: async () => {
+ if (get().lifecycleUnlisten) {
+ return;
+ }
+
+ const unlisten = await listen<GameExitedEvent>("game-exited", (event) => {
+ const { instanceId, versionId, wasStopped } = event.payload;
+
+ set({
+ runningInstanceId: null,
+ runningVersionId: null,
+ launchingInstanceId: null,
+ stoppingInstanceId: null,
+ });
+
+ if (wasStopped) {
+ toast.success(`Stopped Minecraft ${versionId} for instance ${instanceId}`);
+ } else {
+ toast.info(`Minecraft ${versionId} exited for instance ${instanceId}`);
+ }
+ });
+
+ set({ lifecycleUnlisten: unlisten });
+ },
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The game lifecycle listener is never unsubscribed, which can leak handlers across application reloads or store reinitializations.
`lifecycleUnlisten` stores the `unlisten` callback but it’s never invoked. While `initLifecycle` prevents double-registration for a single store lifetime, when the store/app is recreated (e.g. HMR or reload), previous listeners remain registered. Consider exposing a cleanup hook (or automatically calling `lifecycleUnlisten` on teardown) so only one `game-exited` listener exists and to avoid duplicate toasts from stale listeners.
Suggested implementation:
```typescript
initLifecycle: async () => {
const existingUnlisten = get().lifecycleUnlisten;
// Ensure we never accumulate multiple listeners; clean up any existing one first.
if (existingUnlisten) {
existingUnlisten();
}
const unlisten = await listen<GameExitedEvent>("game-exited", (event) => {
const { instanceId, versionId, wasStopped } = event.payload;
set({
runningInstanceId: null,
runningVersionId: null,
launchingInstanceId: null,
stoppingInstanceId: null,
});
if (wasStopped) {
toast.success(`Stopped Minecraft ${versionId} for instance ${instanceId}`);
} else {
toast.info(`Minecraft ${versionId} exited for instance ${instanceId}`);
}
});
set({ lifecycleUnlisten: unlisten });
},
teardownLifecycle: () => {
const existingUnlisten = get().lifecycleUnlisten;
if (existingUnlisten) {
existingUnlisten();
set({ lifecycleUnlisten: null });
}
},
```
```typescript
get isGameRunning() {
return get().runningInstanceId !== null;
},
teardownLifecycle: () => {
const existingUnlisten = get().lifecycleUnlisten;
if (existingUnlisten) {
existingUnlisten();
set({ lifecycleUnlisten: null });
}
},
interface GameExitedEvent {
```
1. Ensure the `GameState` interface includes a `lifecycleUnlisten: (() => void) | null;` field if it does not already.
2. Call `gameStore.getState().teardownLifecycle()` from your application teardown logic (e.g. React root `useEffect` cleanup, or wherever the store is disposed) so that stale `game-exited` listeners are unsubscribed on reload/unmount.
3. If `lifecycleUnlisten` is currently typed as `undefined` instead of `null`, adjust the `set({ lifecycleUnlisten: null })` lines to match your existing convention (e.g. `undefined`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR adds centralized game lifecycle and instance lifecycle management across the Tauri backend and React UI, including shared-cache-aware path resolution and new instance import/export/repair tooling.
Changes:
- Backend: track a single running Minecraft process, add
stop_game, and emit structuredgame-exitedevents. - Backend: centralize per-instance/shared-cache directory resolution and add instance import/export/repair APIs.
- Frontend: wire new lifecycle APIs into Zustand stores and add per-instance start/stop + import/export/repair controls; pin Vite dev server host/port.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
src-tauri/tauri.conf.json |
Adjust dev URL and dev CSP settings for the UI dev environment. |
src-tauri/capabilities/default.json |
Allow remote dev URLs for the Tauri dev server. |
packages/ui/vite.config.ts |
Fix dev server host/port/strictPort to match Tauri dev URL. |
src-tauri/src/main.rs |
Add game process state, stop_game, structured game-exited, and shared-path usage in commands. |
src-tauri/src/core/instance.rs |
Add operation locks, shared-cache path resolution helpers, and instance import/export/repair functionality. |
src-tauri/src/core/modpack.rs |
Switch CurseForge API key handling to runtime env var lookup. |
packages/ui/src/client.ts |
Expose new backend commands (stop game, import/export/repair). |
packages/ui/src/stores/game-store.ts |
Add lifecycle listener + running/launching/stopping state; call new start/stop commands. |
packages/ui/src/models/instance.ts |
Wire import/export/repair to the instance model and refresh flow. |
packages/ui/src/models/auth.ts |
Improve MS login polling error handling and interval safety. |
packages/ui/src/pages/instances-view.tsx |
Add per-instance start/stop and import/export/repair UI actions. |
packages/ui/src/components/bottom-bar.tsx |
Add instance selection and start/stop controls using the new game store. |
packages/ui/src/pages/index.tsx |
Initialize game lifecycle listener at startup. |
.gitignore |
Ignore .vscode/. |
You can also share your feedback on Copilot code review. Take the survey.
| @@ -166,6 +370,8 @@ impl InstanceState { | |||
| .map_err(|e| format!("Failed to delete instance directory: {}", e))?; | |||
| } | |||
|
|
|||
| self.end_operation(id); | |||
| pub fn export_instance(&self, id: &str, archive_path: &Path) -> Result<PathBuf, String> { | ||
| self.begin_operation(id, InstanceOperation::ImportExport)?; | ||
| let instance = self | ||
| .get_instance(id) | ||
| .ok_or_else(|| format!("Instance {} not found", id))?; | ||
|
|
||
| if let Some(parent) = archive_path.parent() { | ||
| fs::create_dir_all(parent).map_err(|e| e.to_string())?; | ||
| } | ||
|
|
||
| let file = fs::File::create(archive_path).map_err(|e| e.to_string())?; | ||
| let mut writer = zip::ZipWriter::new(file); | ||
| let options = SimpleFileOptions::default() | ||
| .compression_method(zip::CompressionMethod::Deflated) | ||
| .unix_permissions(0o644); | ||
|
|
||
| let exported = ExportedInstance { | ||
| name: instance.name.clone(), | ||
| version_id: instance.version_id.clone(), | ||
| icon_path: instance.icon_path.clone(), | ||
| notes: instance.notes.clone(), | ||
| mod_loader: instance.mod_loader.clone(), | ||
| mod_loader_version: instance.mod_loader_version.clone(), | ||
| jvm_args_override: instance.jvm_args_override.clone(), | ||
| memory_override: instance.memory_override.clone(), | ||
| java_path_override: instance.java_path_override.clone(), | ||
| }; | ||
|
|
||
| writer | ||
| .start_file("dropout-instance.json", options) | ||
| .map_err(|e| e.to_string())?; | ||
| writer | ||
| .write_all( | ||
| serde_json::to_string_pretty(&exported) | ||
| .map_err(|e| e.to_string())? | ||
| .as_bytes(), | ||
| ) | ||
| .map_err(|e| e.to_string())?; | ||
|
|
||
| append_directory_to_zip(&mut writer, &instance.game_dir, &instance.game_dir, options)?; | ||
| writer.finish().map_err(|e| e.to_string())?; | ||
| self.end_operation(id); | ||
|
|
| for index in 0..archive.len() { | ||
| let mut entry = archive.by_index(index).map_err(|e| e.to_string())?; | ||
| let Some(enclosed_name) = entry.enclosed_name().map(|p| p.to_path_buf()) else { | ||
| continue; | ||
| }; | ||
|
|
||
| if enclosed_name == PathBuf::from("dropout-instance.json") { | ||
| continue; | ||
| } | ||
|
|
||
| let out_path = imported.game_dir.join(&enclosed_name); | ||
| if entry.name().ends_with('/') { | ||
| fs::create_dir_all(&out_path).map_err(|e| e.to_string())?; | ||
| continue; | ||
| } | ||
|
|
||
| if let Some(parent) = out_path.parent() { | ||
| fs::create_dir_all(parent).map_err(|e| e.to_string())?; | ||
| } | ||
|
|
||
| let mut output = fs::File::create(&out_path).map_err(|e| e.to_string())?; | ||
| std::io::copy(&mut entry, &mut output).map_err(|e| e.to_string())?; | ||
| } | ||
|
|
||
| let mut hydrated = imported.clone(); | ||
| hydrated.version_id = exported.version_id; | ||
| hydrated.icon_path = exported.icon_path; | ||
| hydrated.notes = exported.notes; | ||
| hydrated.mod_loader = exported.mod_loader; | ||
| hydrated.mod_loader_version = exported.mod_loader_version; | ||
| hydrated.jvm_args_override = exported.jvm_args_override; | ||
| hydrated.memory_override = exported.memory_override; | ||
| hydrated.java_path_override = exported.java_path_override; | ||
| self.update_instance(hydrated.clone())?; | ||
| self.end_operation(&imported.id); | ||
|
|
||
| Ok(hydrated) |
| await startGame( | ||
| account, | ||
| () => { | ||
| toast.info("Please login first"); | ||
| }, | ||
| instance.id, | ||
| instance.versionId, | ||
| () => undefined, | ||
| ); |
| running_game | ||
| .child | ||
| .start_kill() | ||
| .map_err(|e| format!("Failed to stop game process: {}", e))?; | ||
|
|
||
| running_game | ||
| .child | ||
| .wait() | ||
| .await | ||
| .map_err(|e| format!("Failed while waiting for the game to stop: {}", e))? | ||
| .code() | ||
| } | ||
| Err(error) => { | ||
| return Err(format!("Failed to inspect running game process: {}", error)); |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
# fix(modpack): 将 CurseForge API Key 改为编译期可选常量 修复 `env!()` 宏在开发者本地无 `CURSEFORGE_API_KEY` 时导致编译失败的问题,改用 `option_env!()` + build.rs 中的 `dotenvy` 读取 .env 文件,实现编译期嵌入、缺失时优雅降级。 ## 更改类型 - [x] Bug 修复(修复问题的非破坏性更改) - [ ] 新功能(添加功能的非破坏性更改) - [ ] 破坏性更改(会导致现有功能无法正常工作的修复或功能) - [ ] 文档更新 - [ ] UI/UX 改进 - [ ] 性能优化 - [ ] 代码重构(无功能性更改) - [x] 配置更改 - [ ] 测试添加或更新 ## LLM 生成代码声明 - [x] 此 PR 不包含 LLM 生成的代码,我**提供**质量担保 ## 相关 Issue 相关 #110 #117 ## 更改内容 ### 后端 (Rust) - modpack.rs:将 `env!("CURSEFORGE_API_KEY")` 替换为 `const CURSEFORGE_API_KEY: Option<&str> = option_env!("CURSEFORGE_API_KEY")`,key 不存在时编译为 `None`,调用 CurseForge 功能时返回友好错误而非 panic - build.rs:添加 `dotenvy::dotenv()` 调用,允许通过 .env 文件在编译期注入 key,并注册 `cargo:rerun-if-changed` / `cargo:rerun-if-env-changed` 确保增量构建正确 ### 前端 (Svelte) - 无 ### 配置 - Cargo.toml:在 `[build-dependencies]` 中添加 `dotenvy = { version = "0.15", default-features = false }` - .gitignore:添加 .env / `.env.local` 忽略规则,防止 key 被意外提交 - .env.example:新增示例文件,说明可选配置项及获取方式 ## 测试 ### 测试环境 - **操作系统**:Fedora Linux 6.19.6-300.fc44.x86_64 x86_64 - **DropOut 版本**:0.2.0-alpha.5 - **测试的 Minecraft 版本**:N/A - **Mod 加载器**:N/A ### 测试用例 - [ ] 已在 Windows 上测试 - [ ] 已在 macOS 上测试 - [x] 已在 Linux 上测试 - [ ] 已测试原版 Minecraft - [ ] 已测试 Fabric - [ ] 已测试 Forge - [ ] 已测试游戏启动 - [ ] 已测试登录流程 - [ ] 已测试 Java 检测/下载 ### 测试步骤 1. 不设置 `CURSEFORGE_API_KEY`,不创建 .env 文件,直接执行 `cargo check` → 应编译通过(无报错) 2. 创建 .env 文件并写入 `CURSEFORGE_API_KEY=test_key`,执行 `cargo check` → 应编译通过,key 被嵌入二进制 3. 不含 key 的构建中触发 CurseForge modpack 导入 → 应返回友好错误提示而非 panic ## 检查清单 ### 代码质量 - [x] 我的代码遵循项目的代码风格指南 - [x] 我已对自己的代码进行了自审 - [ ] 我已对难以理解的区域添加了注释 - [x] 我的更改没有产生新的警告或错误 ### 测试验证 - [x] 我已在本地测试了我的更改 - [ ] 我已添加测试来证明我的修复有效或功能正常工作 - [x] 新的和现有的单元测试在本地通过 - [x] 我至少在一个目标平台上进行了测试 ### 文档更新 - [ ] 我已相应地更新了文档 - [ ] 如有需要,我已更新 README - [ ] 我已在必要处添加/更新代码注释 ### 依赖项 - [x] 我已检查没有添加不必要的依赖项 - [x] 所有新依赖项都已正确记录 - [x] Cargo.lock 已更新 ## 附加说明 `dotenvy` 仅作为 **build-dependency**,不会进入最终二进制。官方发布构建通过 CI 环境变量注入 key,普通开发者无需任何操作即可正常编译和运行。 Co-authored-by: 简律纯 <i@jyunko.cn>
| items={versionOptions} | ||
| onValueChange={setSelectedVersion} | ||
| disabled={isLoadingVersions} | ||
| value={activeInstance?.id ?? null} |
There was a problem hiding this comment.
You cannot use a null value which will force a controlled form to uncontrolled.
Summary by Sourcery
Add centralized game process and instance lifecycle management, shared cache-aware path resolution, and instance import/export/repair capabilities across backend and UI.
New Features:
Enhancements:
Build: