From f534f8341979dc27873b943641cd5f41bfb7be02 Mon Sep 17 00:00:00 2001 From: chsami <23292806+chsami@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:22:51 +0200 Subject: [PATCH] fix(ui): keep the saved sidebar tab and stdout intact while closing When the client closes, MicrobotPluginManager stops the Hub plugins and each removeNavigation runs from invokeLater after ConfigManager's final sendConfig. Removing the open tab then stored the fallback tab, so the in-memory value flipped and any later flush in the exit window persisted the wrong tab. ClientUI now marks itself closing on ClientShutdown and removeNavigation no longer stores the tab while closing. bindStdoutAppenders handed the real System.out to logback, and OutputStreamAppender.stop closes its stream, so a stopped or reset LoggerContext closed fd 1. The stream is now wrapped in a close shield. ClientUITest covers both tab paths and that console initialization binds the stdout appenders; ConsoleLogAppenderTest covers the close shield. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01LjtEMzQHGuq4M1Mpqk9pPV --- .../java/net/runelite/client/ui/ClientUI.java | 12 +- .../client/ui/ConsoleLogAppender.java | 25 +- .../net/runelite/client/ui/ClientUITest.java | 250 ++++++++++++++++++ .../client/ui/ConsoleLogAppenderTest.java | 36 +++ 4 files changed, 321 insertions(+), 2 deletions(-) create mode 100644 runelite-client/src/test/java/net/runelite/client/ui/ClientUITest.java diff --git a/runelite-client/src/main/java/net/runelite/client/ui/ClientUI.java b/runelite-client/src/main/java/net/runelite/client/ui/ClientUI.java index c4af522cd24..54d64977db0 100644 --- a/runelite-client/src/main/java/net/runelite/client/ui/ClientUI.java +++ b/runelite-client/src/main/java/net/runelite/client/ui/ClientUI.java @@ -168,6 +168,7 @@ public class ClientUI private boolean consoleVisible; private boolean sidebarTabRestored; private boolean updatingNavigation; + private boolean closing; @RequiredArgsConstructor private static class HistoryEntry @@ -278,7 +279,10 @@ void removeNavigation(NavigationButton navBtn) : selectedTabHistory.removeLast(); openPanel(entry.navBtn, entry.sidebarOpen); - saveSidebarTab(); + if (!closing) + { + saveSidebarTab(); + } } } finally @@ -290,6 +294,12 @@ void removeNavigation(NavigationButton navBtn) sidebarEntries.remove(navBtn); } + @Subscribe(priority = 100) + private void onClientShutdown(ClientShutdown event) + { + closing = true; + } + @Subscribe private void onGameStateChanged(final GameStateChanged event) { diff --git a/runelite-client/src/main/java/net/runelite/client/ui/ConsoleLogAppender.java b/runelite-client/src/main/java/net/runelite/client/ui/ConsoleLogAppender.java index 38f4b84ba01..8cc70aadd7b 100644 --- a/runelite-client/src/main/java/net/runelite/client/ui/ConsoleLogAppender.java +++ b/runelite-client/src/main/java/net/runelite/client/ui/ConsoleLogAppender.java @@ -12,6 +12,8 @@ import ch.qos.logback.core.AppenderBase; import ch.qos.logback.core.ConsoleAppender; import ch.qos.logback.core.joran.spi.ConsoleTarget; +import java.io.FilterOutputStream; +import java.io.IOException; import java.io.OutputStream; import java.util.Iterator; import java.util.function.Consumer; @@ -28,6 +30,7 @@ final class ConsoleLogAppender extends AppenderBase static void bindStdoutAppenders(LoggerContext context, OutputStream stdout) { + OutputStream shielded = new CloseShieldOutputStream(stdout); for (Logger logger : context.getLoggerList()) { Iterator> appenders = logger.iteratorForAppenders(); @@ -37,7 +40,7 @@ static void bindStdoutAppenders(LoggerContext context, OutputStream stdout) if (appender instanceof ConsoleAppender && ConsoleTarget.SystemOut.getName().equals(((ConsoleAppender) appender).getTarget())) { - ((ConsoleAppender) appender).setOutputStream(stdout); + ((ConsoleAppender) appender).setOutputStream(shielded); } } } @@ -63,4 +66,24 @@ protected void append(ILoggingEvent eventObject) String formatted = layout.doLayout(eventObject); logConsumer.accept(formatted); } + + private static final class CloseShieldOutputStream extends FilterOutputStream + { + private CloseShieldOutputStream(OutputStream out) + { + super(out); + } + + @Override + public void write(byte[] b, int off, int len) throws IOException + { + out.write(b, off, len); + } + + @Override + public void close() throws IOException + { + flush(); + } + } } diff --git a/runelite-client/src/test/java/net/runelite/client/ui/ClientUITest.java b/runelite-client/src/test/java/net/runelite/client/ui/ClientUITest.java new file mode 100644 index 00000000000..601081f83b6 --- /dev/null +++ b/runelite-client/src/test/java/net/runelite/client/ui/ClientUITest.java @@ -0,0 +1,250 @@ +package net.runelite.client.ui; + +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.encoder.PatternLayoutEncoder; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.ConsoleAppender; +import ch.qos.logback.core.joran.spi.ConsoleTarget; +import com.google.common.collect.Iterables; +import java.awt.image.BufferedImage; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.nio.charset.StandardCharsets; +import java.util.Collection; +import java.util.concurrent.atomic.AtomicReference; +import javax.inject.Provider; +import javax.swing.JScrollPane; +import javax.swing.JTabbedPane; +import javax.swing.JTextArea; +import javax.swing.SwingUtilities; +import javax.swing.text.Document; +import net.runelite.api.Client; +import net.runelite.client.callback.ClientThread; +import net.runelite.client.config.ConfigManager; +import net.runelite.client.config.RuneLiteConfig; +import net.runelite.client.eventbus.EventBus; +import net.runelite.client.events.ClientShutdown; +import net.runelite.client.input.MouseManager; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.slf4j.LoggerFactory; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +public class ClientUITest +{ + private static final String CONFIG_GROUP = "runelite"; + private static final String SIDEBAR_TAB = "clientSidebarTab"; + + private ConfigManager configManager; + private EventBus eventBus; + private ClientUI clientUI; + private PrintStream originalOut; + private ConsoleAppender stdoutAppender; + + @Before + public void setUp() throws Exception + { + originalOut = System.out; + configManager = mock(ConfigManager.class); + eventBus = new EventBus(); + Constructor constructor = ClientUI.class.getDeclaredConstructor(RuneLiteConfig.class, MouseManager.class, + Client.class, ConfigManager.class, Provider.class, EventBus.class, boolean.class, String.class); + constructor.setAccessible(true); + clientUI = constructor.newInstance(mock(RuneLiteConfig.class), null, null, configManager, + (Provider) () -> null, eventBus, false, "Microbot"); + eventBus.register(clientUI); + } + + @After + public void tearDown() throws Exception + { + System.setOut(originalOut); + LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory(); + Logger root = context.getLogger(Logger.ROOT_LOGGER_NAME); + ConsoleLogAppender consoleLogAppender = (ConsoleLogAppender) get("consoleLogAppender"); + if (consoleLogAppender != null) + { + root.detachAppender(consoleLogAppender); + consoleLogAppender.stop(); + } + if (stdoutAppender != null) + { + root.detachAppender(stdoutAppender); + stdoutAppender.stop(); + ConsoleLogAppender.bindStdoutAppenders(context, ConsoleTarget.SystemOut.getStream()); + } + } + + @Test + public void removingTheOpenTabStoresTheTabShownInstead() throws Exception + { + NavigationButton first = button("First", 1); + NavigationButton second = button("Second", 2); + onEdt(() -> + { + installSidebar(); + clientUI.addNavigation(first); + clientUI.addNavigation(second); + clientUI.openPanel(first, true); + clientUI.openPanel(second, true); + clientUI.removeNavigation(second); + }); + + verify(configManager).setConfiguration(CONFIG_GROUP, SIDEBAR_TAB, "First"); + } + + @Test + public void removingTheOpenTabWhileClosingKeepsTheSavedTab() throws Exception + { + NavigationButton first = button("First", 1); + NavigationButton second = button("Second", 2); + onEdt(() -> + { + installSidebar(); + clientUI.addNavigation(first); + clientUI.addNavigation(second); + clientUI.openPanel(first, true); + clientUI.openPanel(second, true); + }); + + eventBus.post(new ClientShutdown()); + onEdt(() -> clientUI.removeNavigation(second)); + + verify(configManager, never()).setConfiguration(eq(CONFIG_GROUP), eq(SIDEBAR_TAB), any(String.class)); + verify(configManager, never()).unsetConfiguration(CONFIG_GROUP, SIDEBAR_TAB); + } + + @Test + public void consoleInitializationKeepsStdoutLogLinesOutOfThePanel() throws Exception + { + ByteArrayOutputStream terminal = new ByteArrayOutputStream(); + System.setOut(new PrintStream(terminal, true, StandardCharsets.UTF_8)); + + LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory(); + PatternLayoutEncoder encoder = new PatternLayoutEncoder(); + encoder.setContext(context); + encoder.setPattern("%msg%n"); + encoder.start(); + stdoutAppender = new ConsoleAppender<>(); + stdoutAppender.setContext(context); + stdoutAppender.setEncoder(encoder); + stdoutAppender.start(); + Logger root = context.getLogger(Logger.ROOT_LOGGER_NAME); + root.addAppender(stdoutAppender); + + AtomicReference panel = new AtomicReference<>(); + onEdt(() -> panel.set(new LogConsolePanel())); + set("consolePanel", panel.get()); + Method initialize = ClientUI.class.getDeclaredMethod("initializeConsoleLogging"); + initialize.setAccessible(true); + initialize.invoke(clientUI); + + String line = "console init probe " + System.nanoTime(); + root.warn(line); + onEdt(() -> { }); + onEdt(() -> { }); + + JTextArea textArea = (JTextArea) ((JScrollPane) panel.get().getComponent(0)).getViewport().getView(); + AtomicReference text = new AtomicReference<>(); + onEdt(() -> + { + Document document = textArea.getDocument(); + text.set(document.getText(0, document.getLength())); + }); + + assertEquals(1, occurrences(text.get(), line)); + assertTrue(terminal.toString(StandardCharsets.UTF_8).contains(line)); + } + + private void installSidebar() throws Exception + { + JTabbedPane sidebar = new JTabbedPane(JTabbedPane.RIGHT); + sidebar.setSelectedIndex(-1); + Collection entries = (Collection) get("sidebarEntries"); + sidebar.addChangeListener(ev -> + { + int index = sidebar.getSelectedIndex(); + try + { + set("selectedTab", index < 0 ? null : Iterables.get(entries, index)); + } + catch (ReflectiveOperationException ex) + { + throw new IllegalStateException(ex); + } + }); + set("sidebar", sidebar); + } + + private static NavigationButton button(String tooltip, int priority) + { + return NavigationButton.builder() + .tooltip(tooltip) + .priority(priority) + .icon(new BufferedImage(16, 16, BufferedImage.TYPE_INT_ARGB)) + .panel(new PluginPanel() + { + }) + .build(); + } + + private Object get(String name) throws ReflectiveOperationException + { + Field field = ClientUI.class.getDeclaredField(name); + field.setAccessible(true); + return field.get(clientUI); + } + + private void set(String name, Object value) throws ReflectiveOperationException + { + Field field = ClientUI.class.getDeclaredField(name); + field.setAccessible(true); + field.set(clientUI, value); + } + + private static int occurrences(String text, String needle) + { + int count = 0; + for (int i = text.indexOf(needle); i >= 0; i = text.indexOf(needle, i + 1)) + { + count++; + } + return count; + } + + private static void onEdt(ThrowingRunnable runnable) throws Exception + { + AtomicReference failure = new AtomicReference<>(); + SwingUtilities.invokeAndWait(() -> + { + try + { + runnable.run(); + } + catch (Exception ex) + { + failure.set(ex); + } + }); + if (failure.get() != null) + { + throw failure.get(); + } + } + + private interface ThrowingRunnable + { + void run() throws Exception; + } +} diff --git a/runelite-client/src/test/java/net/runelite/client/ui/ConsoleLogAppenderTest.java b/runelite-client/src/test/java/net/runelite/client/ui/ConsoleLogAppenderTest.java index 07d470ca283..780cc8e6eab 100644 --- a/runelite-client/src/test/java/net/runelite/client/ui/ConsoleLogAppenderTest.java +++ b/runelite-client/src/test/java/net/runelite/client/ui/ConsoleLogAppenderTest.java @@ -14,6 +14,7 @@ import org.junit.Before; import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; public class ConsoleLogAppenderTest { @@ -72,6 +73,30 @@ public void logLineReachesTheConsoleOnceAndStdoutOnce() assertEquals(1, occurrences(out, "plain print")); } + @Test + public void stoppingTheLoggerContextLeavesStdoutOpen() + { + CloseTrackingStream terminal = new CloseTrackingStream(); + + Logger root = context.getLogger(Logger.ROOT_LOGGER_NAME); + PatternLayoutEncoder encoder = new PatternLayoutEncoder(); + encoder.setContext(context); + encoder.setPattern("%msg%n"); + encoder.start(); + ConsoleAppender stdout = new ConsoleAppender<>(); + stdout.setContext(context); + stdout.setEncoder(encoder); + stdout.start(); + root.addAppender(stdout); + + ConsoleLogAppender.bindStdoutAppenders(context, terminal); + root.info("before stop"); + context.stop(); + + assertFalse(terminal.closed); + assertEquals(1, occurrences(terminal.toString(StandardCharsets.UTF_8), "before stop")); + } + private static int occurrences(String text, String needle) { int count = 0; @@ -82,6 +107,17 @@ private static int occurrences(String text, String needle) return count; } + private static final class CloseTrackingStream extends ByteArrayOutputStream + { + private boolean closed; + + @Override + public void close() + { + closed = true; + } + } + private static final class Tee extends OutputStream { private final OutputStream first;