refactor(crash): 直接从启动器内 API 获取部分环境信息#3080
Closed
LuLu-ling wants to merge 0 commit into
Closed
Conversation
审阅者指南（在小型 PR 上折叠）审阅者指南重构崩溃运行时上下文的收集方式，从启动器状态中直接获取 Java、内存和账号信息，而不是解析启动器日志；同时为来自启动器的元数据引入新的 CrashLogKind，并更新解析器以使用该类型。 更新后的崩溃运行时上下文创建时序图sequenceDiagram
participant MinecraftCrashController
participant PageInstanceSetup
participant ModLaunch
participant mcLaunchJavaSelected
participant mcLoginLoader
MinecraftCrashController->>MinecraftCrashController: AnalyzeGameCrash(request)
MinecraftCrashController->>MinecraftCrashController: _Create(instance, launchScript)
MinecraftCrashController->>PageInstanceSetup: GetRam(instance)
PageInstanceSetup-->>MinecraftCrashController: ram
MinecraftCrashController->>ModLaunch: mcLaunchJavaSelected
ModLaunch-->>MinecraftCrashController: mcLaunchJavaSelected
MinecraftCrashController->>mcLaunchJavaSelected: Installation
mcLaunchJavaSelected-->>MinecraftCrashController: Installation.ToString()
mcLaunchJavaSelected-->>MinecraftCrashController: Installation.JavaFolder
MinecraftCrashController->>ModLaunch: mcLoginLoader
ModLaunch-->>MinecraftCrashController: mcLoginLoader
MinecraftCrashController->>mcLoginLoader: output
mcLoginLoader-->>MinecraftCrashController: output.Name, output.Type
MinecraftCrashController->>MinecraftCrashController: new CrashRuntimeContext
Note over MinecraftCrashController: CrashRuntimeContext 现在使用启动器状态
文件级变更
提示与命令与 Sourcery 交互
自定义你的体验访问你的 dashboard 以：
获取帮助Original review guide in EnglishReviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors crash runtime context collection to pull Java, memory, and account information directly from launcher state instead of parsing launcher logs, and introduces a new CrashLogKind for launcher-derived metadata while updating parsers to use it. Sequence diagram for updated crash runtime context creationsequenceDiagram
participant MinecraftCrashController
participant PageInstanceSetup
participant ModLaunch
participant mcLaunchJavaSelected
participant mcLoginLoader
MinecraftCrashController->>MinecraftCrashController: AnalyzeGameCrash(request)
MinecraftCrashController->>MinecraftCrashController: _Create(instance, launchScript)
MinecraftCrashController->>PageInstanceSetup: GetRam(instance)
PageInstanceSetup-->>MinecraftCrashController: ram
MinecraftCrashController->>ModLaunch: mcLaunchJavaSelected
ModLaunch-->>MinecraftCrashController: mcLaunchJavaSelected
MinecraftCrashController->>mcLaunchJavaSelected: Installation
mcLaunchJavaSelected-->>MinecraftCrashController: Installation.ToString()
mcLaunchJavaSelected-->>MinecraftCrashController: Installation.JavaFolder
MinecraftCrashController->>ModLaunch: mcLoginLoader
ModLaunch-->>MinecraftCrashController: mcLoginLoader
MinecraftCrashController->>mcLoginLoader: output
mcLoginLoader-->>MinecraftCrashController: output.Name, output.Type
MinecraftCrashController->>MinecraftCrashController: new CrashRuntimeContext
Note over MinecraftCrashController: CrashRuntimeContext now uses launcher state
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b00794438e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我在这里给出了一些高层面的反馈：
- 在
_Create中不对ModLaunch.mcLaunchJavaSelected.Installation和ModLaunch.mcLoginLoader.output进行空值检查就直接访问，会在崩溃上下文在非正常启动流程之外构建时带来空引用崩溃的风险；建议为这些访问加上保护，或者提供合理的备用处理。 - 在
CrashLogKind枚举中间添加Launcher会改变后续成员（例如EnvironmentSnapshot、ImportedText）的数值；如果这些数值会被持久化或跨边界使用，建议将新成员追加到末尾，或者为枚举显式指定数值以保持兼容性。
供 AI 代理使用的提示
Please address the comments from this code review:
## Overall Comments
- Accessing `ModLaunch.mcLaunchJavaSelected.Installation` and `ModLaunch.mcLoginLoader.output` without null checks in `_Create` risks null reference crashes when the crash context is built outside a normal launch flow; consider guarding these accesses or providing sensible fallbacks.
- Adding `Launcher` in the middle of the `CrashLogKind` enum shifts the numeric values of subsequent members (e.g., `EnvironmentSnapshot`, `ImportedText`); if these values are persisted or used across boundaries, consider appending the new member at the end or assigning explicit values to preserve compatibility.帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进评审质量。
Original comment in English
Hey - I've left some high level feedback:
- Accessing
ModLaunch.mcLaunchJavaSelected.InstallationandModLaunch.mcLoginLoader.outputwithout null checks in_Createrisks null reference crashes when the crash context is built outside a normal launch flow; consider guarding these accesses or providing sensible fallbacks. - Adding
Launcherin the middle of theCrashLogKindenum shifts the numeric values of subsequent members (e.g.,EnvironmentSnapshot,ImportedText); if these values are persisted or used across boundaries, consider appending the new member at the end or assigning explicit values to preserve compatibility.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Accessing `ModLaunch.mcLaunchJavaSelected.Installation` and `ModLaunch.mcLoginLoader.output` without null checks in `_Create` risks null reference crashes when the crash context is built outside a normal launch flow; consider guarding these accesses or providing sensible fallbacks.
- Adding `Launcher` in the middle of the `CrashLogKind` enum shifts the numeric values of subsequent members (e.g., `EnvironmentSnapshot`, `ImportedText`); if these values are persisted or used across boundaries, consider appending the new member at the end or assigning explicit values to preserve compatibility.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
pcl-ce-automation
Bot
added
🛠️ 等待审查
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48f16fd737
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+62
to
+66
| JavaInfo = ModLaunch.mcLaunchJavaSelected.Installation.ToString(), | ||
| JavaPath = ModLaunch.mcLaunchJavaSelected.Installation.JavaFolder, | ||
| AllocatedMemory = $"{ram.ToString("N1", CultureInfo.InvariantCulture)} GB（{(ram * 1024d).ToString("N0", CultureInfo.InvariantCulture)} MB）", | ||
| AccountName = ModLaunch.mcLoginLoader.output.Name, | ||
| AuthType = ModLaunch.mcLoginLoader.output.Type, |
There was a problem hiding this comment.
When another game launch starts before this crash-analysis thread runs (the watcher waits 2 seconds before calling AnalyzeGameCrash), these fields are read from the mutable ModLaunch.mcLaunchJavaSelected and mcLoginLoader.output singletons rather than from the launch that just crashed. Since those globals are reassigned during each launch, the report can attribute the crashed instance to the next launch's Java path/version or account, causing environment facts and diagnoses to be wrong; pass the selected Java/login data in MinecraftCrashUiRequest at launch/watch creation time instead.
Useful? React with 👍 / 👎.
LuLu-ling
force-pushed
the
refactor/crash-l
branch
from
48f16fd to
678ed1c
Compare
pcl-ce-automation
Bot
added
🛠️ 等待审查
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.
解决从启动器日志读取信息可能读到过时值的问题
Summary by Sourcery
优化崩溃分析运行时上下文，从启动器的运行时状态直接读取 Java、账号和内存信息，而不是解析启动器日志，并引入一种新的崩溃日志类型，用于表示由启动器提供的数据。
Enhancements:
Original summary in English
Summary by Sourcery
Refine crash analysis runtime context to read Java, account, and memory information directly from launcher runtime state instead of parsing launcher logs, and introduce a new crash log kind to represent launcher-provided data.
Enhancements: