From 7ce78a81c172ea6603481e75e107027cb75eeb35 Mon Sep 17 00:00:00 2001 From: asvitkine Date: Wed, 20 Sep 2023 21:12:49 -0400 Subject: [PATCH] Fix NPE with UnitHelp. (#11977) This fixes the NPE with Unit Help while preserving the functionality where the progress dialog stays shown until Unit Help is opened. (This is particularly noticeable on the Warcraft map where otherwise there's a 9s delay on my machine after the progress dialog is closed and before unit help is shown.) The NPE was caused by my previous attempt at this doing Swing work on a background thread (which apparently worked fine on Mac, but not Windows). Instead, this PR reworks the logic with a more complicated solution that allows plumbing the expensive Swing work to be done to the background runner, so that it can run it before it closes the progress dialog. --- .../ui/background/BackgroundTaskRunner.java | 28 +++++++++++--- .../triplea/ui/menubar/help/UnitHelpMenu.java | 38 ++++++++++--------- .../engine/lobby/client/login/LobbyLogin.java | 1 + 3 files changed, 43 insertions(+), 24 deletions(-) diff --git a/game-app/game-core/src/main/java/games/strategy/engine/framework/ui/background/BackgroundTaskRunner.java b/game-app/game-core/src/main/java/games/strategy/engine/framework/ui/background/BackgroundTaskRunner.java index 5a36d024d94..7165f6d13b5 100644 --- a/game-app/game-core/src/main/java/games/strategy/engine/framework/ui/background/BackgroundTaskRunner.java +++ b/game-app/game-core/src/main/java/games/strategy/engine/framework/ui/background/BackgroundTaskRunner.java @@ -4,13 +4,16 @@ import static com.google.common.base.Preconditions.checkState; import com.google.common.base.Throwables; +import java.util.Optional; import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import java.util.function.Supplier; import javax.annotation.Nullable; import javax.swing.JFrame; import javax.swing.SwingUtilities; import javax.swing.SwingWorker; +import lombok.Getter; import lombok.experimental.UtilityClass; import org.triplea.java.Interruptibles; import org.triplea.java.function.ThrowingSupplier; @@ -19,7 +22,7 @@ /** Provides methods for running tasks in the background to avoid blocking the UI. */ @UtilityClass public final class BackgroundTaskRunner { - private static JFrame mainFrame; + @Getter private static JFrame mainFrame; public static void setMainFrame(final JFrame mainFrame) { checkState(BackgroundTaskRunner.mainFrame == null); @@ -72,7 +75,14 @@ public static void runInBackground(final String message, final Runnable backgrou */ public static T runInBackgroundAndReturn( final String message, final Supplier backgroundAction) throws InterruptedException { - return runInBackgroundAndReturn(message, backgroundAction::get, RuntimeException.class); + return runInBackgroundAndReturn(message, backgroundAction::get, null, RuntimeException.class); + } + + public static T runInBackgroundAndReturn( + Consumer runOnEdtBeforeDialogClose, String message, Supplier backgroundAction) + throws InterruptedException { + return runInBackgroundAndReturn( + message, backgroundAction::get, runOnEdtBeforeDialogClose, RuntimeException.class); } /** @@ -88,6 +98,9 @@ public static T runInBackgroundAndReturn( * @param The type of exception thrown by the background action. * @param message The message displayed to the user while the background action is running. * @param backgroundAction The action to run in the background. + * @param runOnEdtBeforeDialogClose Work to be done on EDT with the result, before closing the + * dialog. Some Swing operations can be slow (e.g. layout of lots of HTML like unit help), so + * this can be used to ensure we only close the progress dialog once this work has been done. * @param exceptionType The type of exception thrown by the background action. * @return The value returned by {@code backgroundAction}. * @throws IllegalStateException If this method is not called from the EDT. @@ -98,6 +111,7 @@ public static T runInBackgroundAndReturn( public static T runInBackgroundAndReturn( final String message, final ThrowingSupplier backgroundAction, + final Consumer runOnEdtBeforeDialogClose, final Class exceptionType) throws E, InterruptedException { checkState(SwingUtilities.isEventDispatchThread()); @@ -117,16 +131,18 @@ protected T doInBackground() throws Exception { @Override protected void done() { - waitDialog.setVisible(false); - waitDialog.dispose(); - try { - resultRef.set(get()); + T t = get(); + resultRef.set(t); + Optional.ofNullable(runOnEdtBeforeDialogClose).ifPresent(c -> c.accept(t)); } catch (final ExecutionException e) { exceptionRef.set(e.getCause()); } catch (final InterruptedException e) { Thread.currentThread().interrupt(); exceptionRef.set(e); + } finally { + waitDialog.setVisible(false); + waitDialog.dispose(); } } }; diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/ui/menubar/help/UnitHelpMenu.java b/game-app/game-core/src/main/java/games/strategy/triplea/ui/menubar/help/UnitHelpMenu.java index e12a3eec102..5b5e6c2fcd4 100644 --- a/game-app/game-core/src/main/java/games/strategy/triplea/ui/menubar/help/UnitHelpMenu.java +++ b/game-app/game-core/src/main/java/games/strategy/triplea/ui/menubar/help/UnitHelpMenu.java @@ -5,12 +5,9 @@ import games.strategy.triplea.ui.UiContext; import javax.swing.Action; import javax.swing.BorderFactory; -import javax.swing.JDialog; import javax.swing.JEditorPane; import javax.swing.JScrollPane; import lombok.experimental.UtilityClass; -import org.triplea.java.Interruptibles; -import org.triplea.java.Interruptibles.Result; import org.triplea.swing.SwingAction; @UtilityClass @@ -20,21 +17,26 @@ class UnitHelpMenu { Action buildMenu(final GameData gameData, final UiContext uiContext) { return SwingAction.of( unitHelpTitle, - e -> { - final Result result = - Interruptibles.awaitResult( - () -> - BackgroundTaskRunner.runInBackgroundAndReturn( - "Calculating Data", - () -> { - String text = UnitStatsTable.getUnitStatsTable(gameData, uiContext); - JEditorPane editorPane = new JEditorPane("text/html", text); - editorPane.setEditable(false); - JScrollPane scroll = new JScrollPane(editorPane); - scroll.setBorder(BorderFactory.createEmptyBorder()); - return InformationDialog.createDialog(scroll, unitHelpTitle); - })); - result.result.orElseThrow().setVisible(true); + event -> { + try { + BackgroundTaskRunner.runInBackgroundAndReturn( + UnitHelpMenu::showDialog, + "Calculating Data", + () -> UnitStatsTable.getUnitStatsTable(gameData, uiContext)); + } catch (InterruptedException e) { + // Nothing to do. + } }); } + + private static void showDialog(String text) { + final JEditorPane editorPane = new JEditorPane(); + editorPane.setContentType("text/html"); + editorPane.setEditable(false); + final JScrollPane scroll = new JScrollPane(editorPane); + scroll.setBorder(BorderFactory.createEmptyBorder()); + editorPane.setText(text); + editorPane.setCaretPosition(0); + InformationDialog.createDialog(scroll, unitHelpTitle).setVisible(true); + } } diff --git a/game-app/game-headed/src/main/java/games/strategy/engine/lobby/client/login/LobbyLogin.java b/game-app/game-headed/src/main/java/games/strategy/engine/lobby/client/login/LobbyLogin.java index 96b71077a70..45feec8c395 100644 --- a/game-app/game-headed/src/main/java/games/strategy/engine/lobby/client/login/LobbyLogin.java +++ b/game-app/game-headed/src/main/java/games/strategy/engine/lobby/client/login/LobbyLogin.java @@ -235,6 +235,7 @@ private void forgotPassword(final ForgotPasswordPanel panel) { .username(panel.getUserName()) .email(panel.getEmail()) .build()), + null, IOException.class) .getResponseMessage(); DialogBuilder.builder()