避免在代码中直接引用 Controllers.getStage() by Glavo · Pull Request #6431 · HMCL-dev/HMCL

Glavo · 2026-07-17T14:42:58Z

未来我们可能需要在以下情况支持运行过程中替换 Stage：

我们可以在隐藏启动器时直接析构掉现有 Stage，显示时再重新构造新的 Stage，从而避免此功能现在的一些问题。
未来我们需要实现通过 StageStyle.EXTENDED 支持 [Feature Request] 使用原生窗口装饰 #2081，但是该功能与窗口透明是冲突的。为了支持运行时修改窗口透明选项，我们可能需要在运行时重新构造 Stage。

为了减少未来的迁移成本，本 PR 清理了现有代码，避免这些代码直接依赖于 Controllers.getStage() 为未来的迁移造成困难。

…ialog handling

…height

Glavo · 2026-07-17T14:44:03Z

@codex review

gemini-code-assist

Code Review

This pull request refactors window state management and dialog presentation across the application, centralizing file/directory chooser dialogs and window size properties within Controllers.java. However, several critical bugs were identified in the implementation: storing normalized values in stageWidth and stageHeight breaks dialog size bindings, a typo uses stageX instead of stageY when saving the window's Y-coordinate, and the listener incorrectly divides vertical dimensions by the screen's width instead of its height. Additionally, a layout bug in ModListPageSkin.java binds maxWidthProperty() to the window's height instead of its width.

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 · 2026-07-17T14:44:58Z

    public static void saveWindowStates() {
-        saveWindowBounds();
-    }
-
-    private static void saveWindowBounds() {
-        if (stageX != null) {
-            state().setX(toContentX(stageX.get()) / SCREEN.getBounds().getWidth());
-        }
-        if (stageY != null) {
-            state().setY(toContentY(stageY.get()) / SCREEN.getBounds().getHeight());
-        }
-        if (stageHeight != null) {
-            state().setHeight(toContentHeight(stageHeight.get()));
-        }
-        if (stageWidth != null) {
-            state().setWidth(toContentWidth(stageWidth.get()));
-        }
+        state().setX(stageX.get());
+        state().setY(stageX.get());
+        state().setHeight(stageHeight.get());
+        state().setWidth(stageWidth.get());
    }


Critical Bug & Regression:\n\nStoring normalized values in stageWidth and stageHeight breaks windowWidthProperty() and windowHeightProperty(), which are used by dialogs (e.g., UpgradeDialog, ModInfoDialog) to bind their sizes. Since the values are normalized (between 0 and 1), the dialogs shrink to less than 1 pixel and become invisible.\n\nAdditionally, there is a typo on line 222 where state().setY(...) is set using stageX.get() instead of stageY.get().\n\nTo fix these issues, we should keep stageX, stageY, stageWidth, and stageHeight as absolute pixel values (matching their initialization in initialize), and perform the normalization/conversion only when saving the window states.

Suggested change

public static void saveWindowStates() {

saveWindowBounds();

}

private static void saveWindowBounds() {

if (stageX != null) {

state().setX(toContentX(stageX.get()) / SCREEN.getBounds().getWidth());

}

if (stageY != null) {

state().setY(toContentY(stageY.get()) / SCREEN.getBounds().getHeight());

}

if (stageHeight != null) {

state().setHeight(toContentHeight(stageHeight.get()));

}

if (stageWidth != null) {

state().setWidth(toContentWidth(stageWidth.get()));

}

state().setX(stageX.get());

state().setY(stageX.get());

state().setHeight(stageHeight.get());

state().setWidth(stageWidth.get());

}

public static void saveWindowStates() {

state().setX((stageX.get() + CUSTOM_DECORATION_SHADOW_SIZE) / SCREEN.getBounds().getWidth());

state().setY((stageY.get() + CUSTOM_DECORATION_SHADOW_SIZE) / SCREEN.getBounds().getHeight());

state().setHeight(Math.max(0.0, stageHeight.get() - CUSTOM_DECORATION_SHADOW_EXTENT));

state().setWidth(Math.max(0.0, stageWidth.get() - CUSTOM_DECORATION_SHADOW_EXTENT));

}

gemini-code-assist · 2026-07-17T14:44:58Z

        stageSizeChangeListener = o -> {
-            ReadOnlyDoubleProperty sourceProperty = (ReadOnlyDoubleProperty) o;
-            DoubleProperty targetProperty;
-            switch (sourceProperty.getName()) {
-                case "x": {
-                    targetProperty = stageX;
-                    break;
-                }
-                case "y": {
-                    targetProperty = stageY;
-                    break;
-                }
-                case "width": {
-                    targetProperty = stageWidth;
-                    break;
-                }
-                case "height": {
-                    targetProperty = stageHeight;
-                    break;
-                }
-                default: {
-                    targetProperty = null;
-                }
-            }
-
-            if (targetProperty != null
-                    && Controllers.stage != null
+            if (Controllers.stage != null
                    && !Controllers.stage.isIconified()
                    // https://github.com/HMCL-dev/HMCL/issues/4290
                    && (OperatingSystem.CURRENT_OS == OperatingSystem.MACOS ||
                    !Controllers.stage.isFullScreen() && !Controllers.stage.isMaximized())
            ) {
-                targetProperty.set(sourceProperty.get());
+                ReadOnlyDoubleProperty property = (ReadOnlyDoubleProperty) o;
+                switch (property.getName()) {
+                    case "x" ->
+                            stageX.set((property.get() + CUSTOM_DECORATION_SHADOW_SIZE) / SCREEN.getBounds().getWidth());
+                    case "y" ->
+                            stageY.set((property.get() + CUSTOM_DECORATION_SHADOW_SIZE) / SCREEN.getBounds().getWidth());
+                    case "width" ->
+                            stageWidth.set(Math.max(0.0, property.get() - CUSTOM_DECORATION_SHADOW_EXTENT) / SCREEN.getBounds().getWidth());
+                    case "height" ->
+                            stageHeight.set(Math.max(0.0, property.get() - CUSTOM_DECORATION_SHADOW_EXTENT) / SCREEN.getBounds().getWidth());
+                }
            }
        };


Critical Bug & Inconsistency:\n\n1. Storing normalized values in stageWidth and stageHeight causes windowWidthProperty() and windowHeightProperty() to return values between 0 and 1, which breaks dialog size bindings (making them invisible).\n2. The listener is inconsistent with initialize(...) where these properties are initialized with absolute pixel values (e.g., stageX.set(initX)).\n3. For "y" and "height", the code incorrectly divides by SCREEN.getBounds().getWidth() instead of SCREEN.getBounds().getHeight().\n\nWe should update the listener to store absolute pixel values directly, and handle normalization/conversion in saveWindowStates() instead.

stageSizeChangeListener = o -> { if (Controllers.stage != null && !Controllers.stage.isIconified() // https://github.com/HMCL-dev/HMCL/issues/4290 && (OperatingSystem.CURRENT_OS == OperatingSystem.MACOS || !Controllers.stage.isFullScreen() && !Controllers.stage.isMaximized()) ) { ReadOnlyDoubleProperty property = (ReadOnlyDoubleProperty) o; switch (property.getName()) { case "x" -> stageX.set(property.get()); case "y" -> stageY.set(property.get()); case "width" -> stageWidth.set(property.get()); case "height" -> stageHeight.set(property.get()); } } };

gemini-code-assist · 2026-07-17T14:44:58Z

+            prefWidthProperty().bind(Controllers.windowWidthProperty().multiply(0.7));
+            maxWidthProperty().bind(Controllers.windowHeightProperty().multiply(0.7));


Layout Bug: maxWidthProperty() is incorrectly bound to Controllers.windowHeightProperty() instead of Controllers.windowWidthProperty(). This constrains the maximum width of the dialog based on the window's height rather than its width.

Suggested change

prefWidthProperty().bind(Controllers.windowWidthProperty().multiply(0.7));

maxWidthProperty().bind(Controllers.windowHeightProperty().multiply(0.7));

prefWidthProperty().bind(Controllers.windowWidthProperty().multiply(0.7));

maxWidthProperty().bind(Controllers.windowWidthProperty().multiply(0.7));

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ffe2b3d7df

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

chatgpt-codex-connector · 2026-07-17T14:47:45Z

+                    case "width" ->
+                            stageWidth.set(Math.max(0.0, property.get() - CUSTOM_DECORATION_SHADOW_EXTENT) / SCREEN.getBounds().getWidth());
+                    case "height" ->
+                            stageHeight.set(Math.max(0.0, property.get() - CUSTOM_DECORATION_SHADOW_EXTENT) / SCREEN.getBounds().getWidth());


Keep window dimensions in pixel units

After any width or height change, these branches replace stageWidth and stageHeight with screen-normalized ratios, although the newly added UI bindings consume them as pixel dimensions. Resizing the launcher therefore reduces dialog limits such as UpgradeDialog to fractions of a pixel, and shutdown persists ratios even though initialize reads LauncherState.width and height as content pixels, so the next launch falls back to the minimum size.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-17T14:47:45Z

+        state().setX(stageX.get());
+        state().setY(stageX.get());


Persist the tracked Y coordinate

Whenever window state is saved after the position listener has run, this stores the normalized X coordinate as both X and Y. On the next launch, the vertical position is consequently reconstructed from the horizontal position rather than where the user left the window.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-17T14:47:45Z

+            double initY = initContentY - CUSTOM_DECORATION_SHADOW_SIZE;
            stage.setX(initX);
            stage.setY(initY);
            stageX.set(initX);


Normalize initial coordinates before caching them

If the launcher is closed without a subsequent X/Y property change, these caches still contain absolute stage coordinates, while saveWindowStates now writes them directly into fields documented and read as normalized coordinates. The next initialization multiplies those values by the screen dimensions, rejects the resulting off-screen position, and recenters the window instead of restoring it.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-17T14:47:45Z

+            prefWidthProperty().bind(Controllers.windowWidthProperty().multiply(0.7));
+            maxWidthProperty().bind(Controllers.windowHeightProperty().multiply(0.7));


Derive the dialog width from the window width

The maximum width is now bound to windowHeightProperty, whereas both width constraints previously used the stage width. On the normal wide launcher window, this clamps the mod-information dialog to roughly 70% of the much smaller height and overrides its correctly width-based preferred size.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-17T14:47:45Z

+    public static @Nullable Path showDialog(DirectoryChooser directoryChooser) {
+        return FileUtils.toPath(directoryChooser.showDialog(stage));
+    }


Add required documentation to the new dialog helpers

The repository AGENTS.md requires every method to have /// Markdown-style documentation, but this and the other three newly introduced dialog helper methods have no documentation describing their owner behavior, nullable cancellation result, or side effects.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-17T14:47:45Z

        decorator.showToast(content);
    }

+    public static @Nullable Path showDialog(DirectoryChooser directoryChooser) {


Declare the required class-level nullability default

These new helper signatures introduce implicitly non-null chooser parameters, but Controllers is not annotated with @NotNullByDefault. This violates the repository AGENTS.md requirement that every class carry that annotation and that nullability never be implicit.

Useful? React with 👍 / 👎.

Hylfrd · 2026-07-17T18:18:39Z

gemini：是的我是codex

Minecraft269 · 2026-07-18T02:59:50Z

gemini：是的我是codex

gemini死前的最后一舞

Glavo added 8 commits July 17, 2026 22:06

Refactor: Replace FileUtils calls with Controllers methods for file d…

1aecac4

…ialog handling

Refactor: Update saveWindowStates method to handle window state saving

cddefb3

Refactor: Simplify window state management and remove redundant methods

a52b00f

update

4e76144

Refactor: Update width and height bindings to use window properties

a7a60cb

Refactor: Update maxHeight calculation to use Controllers for window …

47ebdf6

…height

Refactor: Remove unused Stage import from ResourcePackListPage

031d1e1

Refactor: Remove unused getScene method from Controllers

ffe2b3d

github-actions Bot added the 100+ label Jul 17, 2026

gemini-code-assist Bot reviewed Jul 17, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jul 17, 2026

View reviewed changes

		prefWidthProperty().bind(Controllers.windowWidthProperty().multiply(0.7));
		maxWidthProperty().bind(Controllers.windowHeightProperty().multiply(0.7));

Uh oh!

Conversation

Glavo commented Jul 17, 2026

Uh oh!

Glavo commented Jul 17, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Hylfrd commented Jul 17, 2026

Uh oh!

Minecraft269 commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants