diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/CompanionApplication.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/CompanionApplication.java index a859c8ef..d6cb3e94 100644 --- a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/CompanionApplication.java +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/CompanionApplication.java @@ -1,5 +1,6 @@ package com.github.minecraft_ta.totalDebugCompanion; +import com.github.minecraft_ta.totalDebugCompanion.project.CurrentProject; import com.github.minecraft_ta.totalDebugCompanion.util.Workers; import com.github.minecraft_ta.totalDebugCompanion.notification.NotificationCenter; import com.github.minecraft_ta.totalDebugCompanion.notification.NotificationCenter.Source; @@ -90,6 +91,8 @@ public final class CompanionApplication implements AutoCloseable, ProjectControl private final CompanionLaunchConfiguration launchConfiguration; private final Object lifecycleLock = new Object(); private volatile ProjectScope current; + /** The current project as the window follows it; {@link #makeCurrent} is the one place that changes it. */ + private final CurrentProject projectState = new CurrentProject(); /** Removes the current project's routes of the game's messages; under the lifecycle lock. */ private Runnable currentMessages = () -> { }; private final InstanceState emptyState = InstanceState.inMemory(); @@ -772,6 +775,7 @@ private void makeCurrent(ProjectScope scope) { this.currentMessages = () -> { }; current = scope; if (scope != null && session != null) this.currentMessages = scope.listen(session); + this.projectState.set(scope); } private void reportCleanupFailure(String description, Exception failure) { @@ -790,12 +794,6 @@ private void activateProfile(CompanionProfile requested) throws IOException { /** Shows the project's saved pack catalog and item icons, which stay browsable without a game connection. */ private void restoreCatalog(ProjectScope scope) { - scope.catalog().changed().subscribe(() -> { - if (currentScope() == scope) onUi(CompanionUi::catalogChanged); - }); - scope.changes().changed().subscribe(() -> { - if (currentScope() == scope) onUi(CompanionUi::changesRecorded); - }); itemIcons.setItemLookup(itemId -> scope.catalog().index().flatMap(index -> index.itemIcon(itemId))); // Independent tasks: unreadable icon archives must not keep the catalog from loading. itemIcons.restore(scope.paths().previews()); @@ -1085,7 +1083,7 @@ private Map runtimeContext() { public MainWindow createWindow() { if (!SwingUtilities.isEventDispatchThread()) throw new IllegalStateException("Create the window on the EDT"); synchronized (lifecycleLock) { checkWindowCreation(); } - MainWindow window = new MainWindow(this::currentScope, getDebuggerController(), codeInsightService, + MainWindow window = new MainWindow(this.projectState, getDebuggerController(), codeInsightService, scriptExecutions, executionRuns, notifications, editorRuns, runtimeIndexService, this::openDebugFrame, this::exit, this, this::setMcpEnabled, itemIcons); List queued; try { diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/project/CurrentProject.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/project/CurrentProject.java new file mode 100644 index 00000000..6b10e366 --- /dev/null +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/project/CurrentProject.java @@ -0,0 +1,67 @@ +package com.github.minecraft_ta.totalDebugCompanion.project; + +import com.github.minecraft_ta.totalDebugCompanion.util.Signal; + +import javax.swing.SwingUtilities; +import java.util.Objects; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Function; + +/** + * The project Companion shows now (docs/SYSTEMS.md, section 1). The window follows a signal of whichever project is + * current through {@link #follows}, which moves to the next project on a switch and tells nothing of one no longer + * current, so no follower checks which project a change came from. + */ +public final class CurrentProject { + private final Signal changed = new Signal(); + private volatile ProjectScope scope; + + /** The current project, or null without one. */ + public ProjectScope scope() { + return this.scope; + } + + /** Fires after another project became current, or none, on the thread that switched. */ + public Signal changed() { + return this.changed; + } + + /** Makes {@code scope} the current project, or none when null, and tells the followers where it changed. */ + public void set(ProjectScope scope) { + if (this.scope == scope) return; + this.scope = scope; + this.changed.fire(); + } + + /** + * Runs {@code told} on the Swing thread whenever the signal {@code signal} names of the current project fires, while + * that project is still current; follows the next project's after a switch. Returns what stops it. + */ + public Runnable follows(Function signal, Runnable told) { + Objects.requireNonNull(signal, "signal"); + Objects.requireNonNull(told, "told"); + Object lock = new Object(); + AtomicBoolean stopped = new AtomicBoolean(); + Runnable[] fromScope = {() -> { }}; + Runnable move = () -> { + synchronized (lock) { + if (stopped.get()) return; + fromScope[0].run(); + ProjectScope now = this.scope; + fromScope[0] = now == null ? () -> { } : signal.apply(now).subscribe(() -> SwingUtilities.invokeLater(() -> { + if (!stopped.get() && this.scope == now) told.run(); + })); + } + }; + Runnable fromProject = this.changed.subscribe(move); + move.run(); + return () -> { + stopped.set(true); + fromProject.run(); + synchronized (lock) { + fromScope[0].run(); + fromScope[0] = () -> { }; + } + }; + } +} diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/ui/CompanionUi.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/ui/CompanionUi.java index 6d929104..0689043f 100644 --- a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/ui/CompanionUi.java +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/ui/CompanionUi.java @@ -14,10 +14,6 @@ public interface CompanionUi { void refreshProfile(); void refreshProjects(); void runtimeChanged(); - /** The selected project's pack catalog changed state. */ - void catalogChanged(); - /** Companion's record of its changes to the pack changed. */ - void changesRecorded(); void setGameStatus(ServiceStatus status); void setMcpStatus(ServiceStatus status); void setRuntimeIndexStatus(RuntimeIndexService.Status status); diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/ui/views/MainWindow.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/ui/views/MainWindow.java index 69f278b7..4b3686c1 100644 --- a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/ui/views/MainWindow.java +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/ui/views/MainWindow.java @@ -1,5 +1,7 @@ package com.github.minecraft_ta.totalDebugCompanion.ui.views; +import java.util.List; +import com.github.minecraft_ta.totalDebugCompanion.project.CurrentProject; import com.github.minecraft_ta.totalDebugCompanion.inspection.ItemIconService; import com.github.minecraft_ta.totalDebugCompanion.ui.components.FlatIconButton; import com.github.minecraft_ta.totalDebugCompanion.ui.Tooltip; @@ -90,6 +92,7 @@ public class MainWindow extends JFrame implements AWTEventListener, CompanionUi private boolean disposed; private final Consumer themeListener = this::updateWindowIcon; private final Supplier project; + private final List stopFollowingProject; private final DebuggerSessionController debugger; private final CodeInsightService insights; private final ItemIconService itemIcons; @@ -102,7 +105,7 @@ public class MainWindow extends JFrame implements AWTEventListener, CompanionUi private final ProjectControls projects; private boolean gameConnected; - public MainWindow(Supplier project, DebuggerSessionController debugger, CodeInsightService insights, + public MainWindow(CurrentProject currentProject, DebuggerSessionController debugger, CodeInsightService insights, ScriptExecutionService scripts, ExecutionRuns executions, NotificationCenter notifications, EditorScriptRunService editorRuns, RuntimeIndexService indexLoader, FrameNavigation frameNavigation, Runnable exit, ProjectControls projects, Consumer toggleMcp, ItemIconService itemIcons) { this.projects = projects; @@ -110,7 +113,10 @@ public MainWindow(Supplier project, DebuggerSessionController debu this.notifications = notifications; this.editorRuns = editorRuns; this.projectSelector = new ProjectSelector(projects, notifications); - this.project = project; + this.project = currentProject::scope; + // The current project's catalog and change record, whichever project that is. + this.stopFollowingProject = List.of(currentProject.follows(scope -> scope.catalog().changed(), this::catalogChanged), + currentProject.follows(scope -> scope.changes().changed(), this::changesRecorded)); this.debugger = debugger; this.insights = insights; this.scripts = scripts; @@ -243,6 +249,7 @@ public void windowClosing(WindowEvent event) { @Override public void dispose() { if (!disposed) { disposed = true; + stopFollowingProject.forEach(Runnable::run); if (debuggerPopup != null) debuggerPopup.setVisible(false); projectSelector.dispose(); fileTreeView.dispose(); @@ -269,12 +276,12 @@ public void windowClosing(WindowEvent event) { refreshActions(); this.projectSelector.refresh(); } - @Override public void changesRecorded() { + private void changesRecorded() { // Only the Changes row counts the changes in effect. this.fileTreeView.refreshModpack(false); } - @Override public void catalogChanged() { + private void catalogChanged() { this.fileTreeView.refreshModpack(true); this.editorTabs.refreshTabIdentities(); if (this.searchEverywherePopup != null) this.searchEverywherePopup.catalogChanged(); diff --git a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/ApplicationNavigationTest.java b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/ApplicationNavigationTest.java index 741436ed..781a1a5c 100644 --- a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/ApplicationNavigationTest.java +++ b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/ApplicationNavigationTest.java @@ -362,10 +362,6 @@ public void setSwitching(boolean switching) { } public void refreshProfile() { onRefresh.run(); } public void refreshProjects() { } public void runtimeChanged() { } - @Override - public void catalogChanged() { } - @Override - public void changesRecorded() { } public void setGameStatus(ServiceStatus status) { } public void setMcpStatus(ServiceStatus status) { } public void setRuntimeIndexStatus(RuntimeIndexService.Status status) { } diff --git a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/CompanionReconnectTest.java b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/CompanionReconnectTest.java index 7aa41eea..d07472ee 100644 --- a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/CompanionReconnectTest.java +++ b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/CompanionReconnectTest.java @@ -452,10 +452,6 @@ public void setSwitching(boolean value) { } public void refreshProfile() { } public void refreshProjects() { } public void runtimeChanged() { } - @Override - public void catalogChanged() { } - @Override - public void changesRecorded() { } public void setGameStatus(ServiceStatus status) { game = status; gameChanged.accept(status); } public void setMcpStatus(ServiceStatus status) { } public void setRuntimeIndexStatus(RuntimeIndexService.Status status) { } diff --git a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/project/CurrentProjectTest.java b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/project/CurrentProjectTest.java new file mode 100644 index 00000000..620ac9ee --- /dev/null +++ b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/project/CurrentProjectTest.java @@ -0,0 +1,117 @@ +package com.github.minecraft_ta.totalDebugCompanion.project; + +import com.github.minecraft_ta.totalDebugCompanion.session.CompanionProfile; +import com.github.minecraft_ta.totalDebugCompanion.storage.ChangeRecord; +import com.github.minecraft_ta.totalDebugCompanion.storage.InstanceState; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import javax.swing.SwingUtilities; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** The project the window shows, whose changes reach the window only while it is the current one. */ +class CurrentProjectTest { + @TempDir Path directory; + + @Test + void aFollowerHearsOnlyTheCurrentProjectAndMovesWithASwitch() throws Exception { + try (ProjectScope first = scope("first"); ProjectScope second = scope("second")) { + CurrentProject current = new CurrentProject(); + current.set(first); + AtomicInteger told = new AtomicInteger(); + Runnable stop = current.follows(scope -> scope.changes().changed(), told::incrementAndGet); + + fire(first); + assertEquals(1, told.get(), "the current project's change is told"); + + current.set(second); + fire(first); + assertEquals(1, told.get(), "a project no longer current tells nothing"); + fire(second); + assertEquals(2, told.get(), "the follower moved to the project now current"); + + // A change the previous project told just before the switch arrives after it. + current.set(first); + SwingUtilities.invokeAndWait(() -> { + first.changes().changed().fire(); + current.set(second); + }); + SwingUtilities.invokeAndWait(() -> { }); + assertEquals(2, told.get(), "a change of the project before, queued before the switch, is not told"); + + SwingUtilities.invokeAndWait(() -> { + second.changes().changed().fire(); + stop.run(); + }); + SwingUtilities.invokeAndWait(() -> { }); + assertEquals(2, told.get(), "a change queued before stopping is not delivered afterward"); + fire(second); + assertEquals(2, told.get(), "a follower that stopped hears nothing"); + first.retire(); + second.retire(); + } + } + + @Test + void aSwitchDuringInitialBindingIsNotMissed() throws Exception { + try (ProjectScope first = scope("first"); ProjectScope second = scope("second")) { + CurrentProject current = new CurrentProject(); + current.set(first); + CountDownLatch binding = new CountDownLatch(1); + CountDownLatch switched = new CountDownLatch(1); + Runnable stopObservingSwitch = current.changed().subscribe(switched::countDown); + Thread switching = Thread.ofPlatform().daemon().name("Test project switch").start(() -> { + try { + if (binding.await(5, TimeUnit.SECONDS)) current.set(second); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + } + }); + Runnable stop = () -> { }; + try { + AtomicInteger told = new AtomicInteger(); + stop = current.follows(selected -> { + if (selected == first) { + binding.countDown(); + try { + assertTrue(switched.await(5, TimeUnit.SECONDS)); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + throw new AssertionError(interrupted); + } + } + return selected.changes().changed(); + }, told::incrementAndGet); + switching.join(5_000); + assertFalse(switching.isAlive()); + fire(second); + assertEquals(1, told.get(), "the follower binds to the project selected during its initial binding"); + fire(first); + assertEquals(1, told.get(), "the previous project has no remaining delivery"); + } finally { + stop.run(); + stopObservingSwitch.run(); + first.retire(); + second.retire(); + } + } + } + + private ProjectScope scope(String name) throws Exception { + Path game = Files.createDirectories(this.directory.resolve(name)); + return new ProjectScope(new Object(), new CompanionProfile(name, game, game), InstanceState.inMemory(), ChangeRecord.inMemory()); + } + + private static void fire(ProjectScope scope) throws Exception { + scope.changes().changed().fire(); + SwingUtilities.invokeAndWait(() -> { }); + } +} diff --git a/docs/SYSTEMS.md b/docs/SYSTEMS.md index 590b8bd5..7c807da9 100644 --- a/docs/SYSTEMS.md +++ b/docs/SYSTEMS.md @@ -66,7 +66,7 @@ Owners: | Owner | Signals | Replaces | |---|---|---| -| The application's projects | current project | the scope checks in `FileTreeView`, `CompanionApplication` and the UI | +| The application's projects | current project (`CurrentProject`, whose `follows` moves to the next project and drops what the one before told) | the scope checks in `CompanionApplication`'s relays | | `PackCatalogService` | catalog | `addListener` | | `GamePacks` | resource packs, datapacks | `addListener(side)`, `addResourcePackListener`, `addDatapackListener` | | `ChangeRecord` | changes | `addListener` | @@ -129,7 +129,7 @@ this.loader = new PageLoader<>(this::read, this::show, this::fail) - **A write checks where it goes when it is made**, not by what the page last read: the resource editor's save refuses a pack the tab no longer saves into, as the change pipeline refuses a copy changed since. - **A page that writes holds its reads** (`hold`, `release`): while its save runs, a read under way is not shown, since it may predate the write, and followed changes wait as while the page is hidden; released, the page reads once what concerned it meanwhile. A page with unsaved changes that must not be replaced by a read (`ConfigPanel`) holds its reads the same way while they last. A page does not read again because it saved; it hears the owner's signal like every other follower. - **A follow may say whether a change concerns the page**, asked when the page would read: the resource editor follows the whole change record, but only its own file's entry, and not its own save's. A read's preparation may also ask which followed signals led to it (`fired`): the resource editor lets Save wait only for a read after a pack change, which may move it to another pack, not for one after an edit elsewhere. -- **A read never changes an owner.** What the Changes page's labels do today with `ChangeRecord.observed` moves to the owners, which notice a value put back outside Companion on their own readings, on the write queue. +- **A read never changes an owner**, with one exception: the Changes page's read drops a change whose file holds its original value again, put back outside Companion (`ChangeRecord.observed`), and reads whenever it is shown. The configuration and resource files have no owner that reads them, so an owner would notice only when this page asked it to read anyway. Decided on 2026-09-30 instead of a step that moved the check into the owners. - A part of a page that reads on its own, such as the resources of a definition, has its own loader with that part as its page. - Work that only redraws from values in memory, such as icons again after new ones came, uses `loader.updates(signal, redraw)`: the same waiting and merging, with no read. A page with nothing to read has a loader that only redraws (`PageLoader.redraws(page)`). What shows outside the page, such as its tab's title, is redrawn from the owners' published values whenever their signal fires, shown or not (`retitles`). @@ -207,8 +207,7 @@ PRs on 1.21.1, stacked, each reviewed until clean. A shared mechanism comes with | 6 | The last watchers onto `FileWatch`: the Project tree's folders, told of entries only, and `ExternalEdits`, whose settle runs on the timer; `FileUtils`' pause for Companion's own moves becomes `FileWatch.pausing` | `FileUtils`, the watchers and schedulers of `ExternalEdits` | | 7 | All remaining pages on `page` and `follows`, the logs, configuration and resource packs pages reading whenever shown; the World tab's title from its owner; reads on the file workers | `ShownUpdates`, `whenShown`, `waitsWhileHidden` and `follow`, the pages' own subscriptions, the reads in constructors, the navigation refreshes | | 8a | The pipeline owning the write queue; the remaining executors and one-argument async calls onto `Workers` | `ConfigChanges`' executor, the UI classes' and `JsonStateWriter`'s executors, every use of the shared pool | -| 8b | The current project as state, which the Project tree and the main window follow; no file checks on the Swing thread | the scope checks, the `CompanionUi` relays | -| 8c | Owners noticing values put back outside Companion, on the write queue; connection numbers for waiting requests | `ChangeRecord.observed` from page reads and the Changes page's read whenever shown | +| 8b | The current project as state (`CurrentProject`), whose signals the main window follows | the `CompanionUi` relays and their scope checks | After the messages, A4 continues with categories registering their pages and Modpack rows. Splitting `CompanionApplication` (the game connection and the MCP server into their own classes) and the mod's `CompanionAppClient` (launching Companion) is easier then and is decided at that point. diff --git a/mod/src/test/java/com/github/minecraft_ta/totaldebug/client/companion/CompanionDiscoveryTest.java b/mod/src/test/java/com/github/minecraft_ta/totaldebug/client/companion/CompanionDiscoveryTest.java index 9d5804ae..507a91de 100644 --- a/mod/src/test/java/com/github/minecraft_ta/totaldebug/client/companion/CompanionDiscoveryTest.java +++ b/mod/src/test/java/com/github/minecraft_ta/totaldebug/client/companion/CompanionDiscoveryTest.java @@ -87,7 +87,7 @@ class CompanionDiscoveryTest { return CompanionDiscovery.Result.CONNECTED; }, connected::get, () -> true, () -> { })) { discovery.start(); - await(() -> attempts.get() >= 2); + await(() -> attempts.get() >= 2 && connected.get()); assertTrue(connected.get()); } } @@ -113,7 +113,7 @@ class CompanionDiscoveryTest { assertEquals(1, attempts.get()); assertFalse(connected.get()); enabled.set(true); - await(() -> attempts.get() == 2); + await(() -> attempts.get() == 2 && connected.get()); assertTrue(connected.get()); } }