Fix #4240: 旧版MC日志把每一条都给算成error - #6304
Conversation
|
艹,竟然有人想起来这条issue |
|
你能给一个所有日志都被算为error的一个版本范围吗 |
|
update:上面评论所有的版本都解决了,可以随时审查合并 |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the log level guessing logic in Log4jLevel.java by introducing helper methods and predefined marker arrays to simplify the parsing code. It also updates LauncherHelper.java to leverage these new methods. The review identified two critical issues: first, redundant calls to guessLevel in LauncherHelper.java that degrade performance and can lead to a potential NullPointerException when level remains null; second, a regression in Log4jLevel.java where the check for [STDERR] was omitted during the refactoring of error markers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the log level guessing logic in Log4jLevel.java by introducing helper methods and predefined marker arrays to simplify parsing. In LauncherHelper.java, the log level guessing is updated to handle error streams. The review feedback highlights two improvement opportunities: first, Level.getLocalizedName() depends on the JVM locale and may fail to match Chinese localized logs on non-Chinese systems, so explicit Chinese markers should be added; second, there is a redundant call to Log4jLevel.guessLevel(log) in LauncherHelper.java when level is already determined to be null, which can be simplified to directly use Log4jLevel.INFO.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17f18e230f
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| int i = 0; | ||
| for (Level level : levels) { | ||
| markers[i++] = '[' + level.getName() + ']'; | ||
| markers[i++] = '[' + level.getLocalizedName() + ']'; |
There was a problem hiding this comment.
Match the game JVM's locale when building markers
Level#getLocalizedName() uses HMCL's current default locale, which SettingsManager changes to the selected launcher UI language, while the separately launched game JVM derives its logging locale from its own JVM/OS settings. When those locales differ—for example, English HMCL on a Chinese OS—Chinese legacy [信息]/[警告] markers are absent from these arrays, so stderr lines still fall through to ERROR and the reported issue remains. Build markers for the possible game locales or recognize the localized level names independently of HMCL's locale.
Useful? React with 👍 / 👎.
| public static Log4jLevel guessLevel(String line, boolean isErrorStream) { | ||
| Log4jLevel level = guessLevel(line); | ||
| return level != null || !isErrorStream ? level : ERROR; |
There was a problem hiding this comment.
Declare nullable results in the new classifier API
This overload returns null whenever an unrecognized stdout line is supplied, but its return type and nullable local are unannotated; the added parseLevel return and guessStderrLevel fallback similarly have nullable contracts. Add @NotNullByDefault to the enum and explicitly annotate every nullable declaration so callers and nullability tooling receive the actual contract.
AGENTS.md reference: AGENTS.md:L7-L9
Useful? React with 👍 / 👎.
| public static final String JAVA_SYMBOL = "([a-zA-Z_$][a-zA-Z\\d_$]*\\.)+[a-zA-Z_$][a-zA-Z\\d_$]*"; | ||
| private static final String WRAPPED_PRINT_STREAM = "[java.lang.Throwable$WrappedPrintStream:println"; | ||
|
|
||
| private static final String[] INFO_MARKERS = markers( |
There was a problem hiding this comment.
Mark the marker arrays as unmodifiable
These static marker arrays are initialized once and treated as immutable lookup tables, but their types omit the required type-use @Unmodifiable annotation. Annotate each added array declaration as String @Unmodifiable [] so the immutability contract is explicit.
AGENTS.md reference: AGENTS.md:L11-L14
Useful? React with 👍 / 👎.






update:下面评论所有的版本都解决了
Close #4240
老版本日志的级别都是本地化中文的,所以还需要根据这个判断日志级别,不能不改Log4jLevel.java,否则全部日志都会变成信息级别