Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) {
Expand All @@ -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());
Expand Down Expand Up @@ -1085,7 +1083,7 @@ private Map<String, Object> 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<PendingNavigation> queued;
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<ProjectScope, Signal> 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();
}));
Comment on lines +51 to +53

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prevent callbacks after stopping a follower

When a project signal races with MainWindow.dispose(), its EDT lambda may already be queued even though the returned stop action removes the subscriptions; this check only verifies that the project is still current. If the scope remains current, catalogChanged or changesRecorded therefore runs after disposal and can repopulate the disposed FileTreeView or attach fresh listeners. Track the subscription's stopped state or generation here and reject queued deliveries after it is stopped.

AGENTS.md reference: AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

}
};
Runnable fromProject = this.changed.subscribe(move);
move.run();
return () -> {
stopped.set(true);
fromProject.run();
synchronized (lock) {
fromScope[0].run();
fromScope[0] = () -> { };
}
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -90,6 +92,7 @@ public class MainWindow extends JFrame implements AWTEventListener, CompanionUi
private boolean disposed;
private final Consumer<CompanionTheme> themeListener = this::updateWindowIcon;
private final Supplier<ProjectScope> project;
private final List<Runnable> stopFollowingProject;
private final DebuggerSessionController debugger;
private final CodeInsightService insights;
private final ItemIconService itemIcons;
Expand All @@ -102,15 +105,18 @@ public class MainWindow extends JFrame implements AWTEventListener, CompanionUi
private final ProjectControls projects;
private boolean gameConnected;

public MainWindow(Supplier<ProjectScope> 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<Boolean> toggleMcp, ItemIconService itemIcons) {
this.projects = projects;
this.itemIcons = itemIcons;
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;
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) { }
Expand Down
Original file line number Diff line number Diff line change
@@ -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(() -> { });
}
}
Loading