[Enhancement] 原理图管理优化

[ToobLac]

Resolves #4052 Resolves #4518 Resolves #3957

#3597 应在本 PR 合并后并实现 NBT 修改功能后关闭

[Minecraft269]

/gemini review

[Wulian233]

@codex

[gemini-code-assist]

Code Review

This pull request refactors and extends the schematic management system, introducing a unified Schematic base class and supporting Litematic, Schem, and NBT Structure formats. It also updates the UI to support directory navigation, mod installation, and custom schematic directories. The code review identified several critical issues, most notably a deadlock risk in ModManager where lock.lock() is incorrectly called in a finally block instead of lock.unlock(). Additionally, feedback highlights potential race conditions in SchematicsPage during asynchronous operations and version switching, risks of duplicate elements upon retry of failed directory loads, missing dimension validation in SchemFile, and a potential integer overflow when calculating total volume in Schematic.

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.

[chatgpt-codex-connector]

💡 Codex Review

HMCL/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/SchematicsPage.java

Lines 253 to 254 in ed68b0d

	fileChooser.getExtensionFilters().add(new FileChooser.ExtensionFilter(
	i18n("extension.schematic"), "*.litematic"));

Include all supported schematic extensions

The page now loads and accepts drag-and-drop for .schem, .schematic, and .nbt via Schematic.isFileSchematic, but the Add dialog still filters only *.litematic. Users trying to import the newly supported formats through the toolbar will not be able to select those files, leaving the new support inaccessible from the primary add flow.

ℹ️ 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".

[Minecraft269]

/gemini review

[gemini-code-assist]

Code Review

This pull request expands schematic file support in HMCL beyond .litematic to include .schematic, .schem, and .nbt structures by introducing a unified Schematic base class and corresponding implementations. It updates the UI (SchematicsPage, MDListCell) to handle these new types, adds support for installing the Litematica mod, and refactors ModManager to manage supported mod loaders. The review feedback highlights several critical thread-safety and robustness issues, including potential NullPointerException risks when accessing the library analyzer, concurrency issues (race conditions and ConcurrentModificationException) in SchematicsPage and ModManager due to unsafe background thread operations, unhandled IOException propagation in directory pre-loading, and opportunities to improve NBT parsing robustness by allowing wider numeric tag conversions.

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-code-assist]

modManager.getLibraryAnalyzer() 可能会返回 null（例如在解析失败或未初始化时）。直接调用 getModLoaders() 可能会导致 NullPointerException。建议在调用前进行非空检查。

            var analyzer = modManager.getLibraryAnalyzer();
            var modLoaders = analyzer != null ? analyzer.getModLoaders() : Collections.<ModLoaderType>emptySet();

[gemini-code-assist]

1. load() 方法在后台线程中运行，并且会修改 this.children（非线程安全的 ArrayList）。如果用户快速切换目录，可能会导致多个线程并发调用同一个 DirItem 的 load() 方法，从而引发竞态条件（Race Condition）或 ConcurrentModificationException。建议将 load() 方法设为 synchronized。
2. for (var dir : dirChildren) 循环并不依赖 stream，不应该放在 try-with-resources 块内部。建议将其移到外面，以提高代码可读性并减少资源占用时间。

        synchronized void load() {
            if (this.loaded) return;
            this.children.clear();
            preLoad();
            for (var dir : dirChildren) {
                dir.preLoad();
                this.children.add(dir);
            }
            try (Stream<Path> stream = Files.list(path)) {
                stream.filter(Schematic::isFileSchematic)
                        .forEach(p -> {
                            try {
                                this.children.add(new SchematicItem(p));
                            } catch (IOException e) {
                                LOG.warning("Failed to load schematic file: " + p, e);
                            }
                        });
            } catch (NoSuchFileException ignored) {
            } catch (IOException e) {
                LOG.warning("Failed to load schematics in " + path, e);
            }

            this.children.sort(Comparator.naturalOrder());
            this.loaded = true;
        }

[gemini-code-assist]

supportedLoaders 是一个 EnumSet，它不是线程安全的。在 refresh() 方法中，supportedLoaders 会在后台线程被修改（通过 updateSupportedLoaders()）。而 getSupportedLoaders() 可能会在 JavaFX 线程中被并发调用（例如在 ModListPage 的 whenCompleteAsync 中）。这会导致并发修改异常（ConcurrentModificationException）或读取到脏数据。

建议在 getSupportedLoaders() 中获取 lock 锁以保证线程安全。

    public EnumSet<ModLoaderType> getSupportedLoaders() {
        lock.lock();
        try {
            return EnumSet.copyOf(supportedLoaders);
        } finally {
            lock.unlock();
        }
    }

[gemini-code-assist]

preLoad() 方法中直接调用了 Files.list(path)，这可能会抛出 IOException（例如目录不存在或无权限）。目前该异常被向外抛出，这会导致调用它的后台任务（如 refresh() 中的 Task.supplyAsync）直接失败，从而导致界面无法正常加载。

建议在 preLoad() 内部捕获并处理 IOException，避免异常传播导致整个加载任务崩溃。此外，由于 preLoad() 会在后台线程并发调用，建议将其设为 synchronized 以保证线程安全。

        synchronized void preLoad() {
            if (this.preLoaded) return;
            this.size = 0;
            this.dirChildren.clear();
            try (Stream<Path> stream = Files.list(path)) {
                stream.forEach(p -> {
                    boolean b1 = Files.isDirectory(p);
                    boolean b2 = Schematic.isFileSchematic(p);
                    if (b1 || b2) this.size++;
                    if (b1) {
                        var child = new DirItem(p, this);
                        this.dirChildren.add(child);
                    }
                });
            } catch (IOException e) {
                LOG.warning("Failed to pre-load schematics in " + path, e);
            }
            this.preLoaded = true;
        }

[gemini-code-assist]

为了保证多线程环境下的内存可见性和一致性，建议在 getGameVersion() 中也使用 lock 进行保护，或者将 gameVersion 字段声明为 volatile。

Suggested change

	public String getGameVersion() {
	return gameVersion;
	}
	public String getGameVersion() {
	lock.lock();
	try {
	return gameVersion;
	} finally {
	lock.unlock();
	}
	}

[gemini-code-assist]

目前的 tryGetInt 和 tryGetShort 实现非常严格，只支持精确的 IntTag 和 ShortTag。在实际的 Minecraft NBT 文件中，有些工具或库可能会将本应是 short 的值写成 int（或者反过来）。

为了提高解析的鲁棒性，建议让 tryGetInt 支持读取其他数值类型的 Tag（如 ByteTag, ShortTag, LongTag），并让 tryGetShort 复用 tryGetInt 的逻辑。

    public static OptionalInt tryGetInt(Tag tag) {
        if (tag instanceof IntTag) {
            return OptionalInt.of(((IntTag) tag).getValue());
        } else if (tag instanceof ShortTag) {
            return OptionalInt.of(((ShortTag) tag).getValue());
        } else if (tag instanceof ByteTag) {
            return OptionalInt.of(((ByteTag) tag).getValue());
        } else if (tag instanceof LongTag) {
            return OptionalInt.of((int) ((LongTag) tag).getValue());
        }
        return OptionalInt.empty();
    }

    public static short tryGetShort(Tag tag) {
        return (short) tryGetInt(tag).orElse(0);
    }