Companion's systems, first slice: signals and one page loader

AGENTS.md

@@ -16,6 +16,8 @@ UI text must serve an action: labels, actual state, errors, or necessary instruc
 
 Do not use the middle dot (`·`) as a separator anywhere: UI text, tooltips, tests or documentation. Separate metadata the way the surrounding view already does: muted secondary text or a secondary column in lists (`PrimarySecondaryText`), spacing between parts in the subject header, or commas inside a value.
 
+Companion's features use the systems in [docs/SYSTEMS.md](docs/SYSTEMS.md) to learn of changes, watch files, load pages, run work off the Swing thread, write and receive game messages. Do not add a listener list, file watcher, thread or executor, page loading path or message route beside them; a need they do not meet changes the system, with a decision recorded in that document. Fix a problem of timing, staleness or reading twice in the system that owns it, not with a flag in a feature. `SystemsRulesTest` checks the parts it can.
+
 Keep the evaluator and compiled Code mode within their existing responsibilities. Changes to that architecture require an explicit design decision.
 
 Match checks to the changed behavior and affected consumers. Documentation-only changes need link and diff checks. Preserve unrelated working-tree changes and coordinate file ownership when another task is editing the same repository.

companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/CompanionApplication.java

@@ -809,10 +809,10 @@ 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().addListener(() -> {
+        scope.catalog().changed().subscribe(() -> {
             if (currentScope() == scope) onUi(CompanionUi::catalogChanged);
         });
-        scope.changes().addListener(() -> {
+        scope.changes().changed().subscribe(() -> {
             if (currentScope() == scope) onUi(CompanionUi::changesRecorded);
         });
         itemIcons.setItemLookup(itemId -> scope.catalog().index().flatMap(index -> index.itemIcon(itemId)));

companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/catalog/KeyAssignments.java

@@ -1,5 +1,6 @@
 package com.github.minecraft_ta.totalDebugCompanion.catalog;
 
+import com.github.minecraft_ta.totalDebugCompanion.util.Signal;
 import java.io.IOException;
 import java.nio.file.ClosedWatchServiceException;
 import java.nio.file.FileSystems;
@@ -22,20 +23,27 @@
  * screen, Companion or an editor, the listeners hear of it once the assignments differ from those read before. Another
  * option written, such as the volume, tells nobody. The file's folder is watched, so a file replaced by a rename, as the
  * game and Companion write it, is seen; a deleted file assigns nothing. A file that cannot be read keeps the assignments
- * read before, and is read again after 1, 5 and 30 seconds and at its next change.
+ * read before, and is read again after 1, 5 and 30 seconds and at its next change. Companion's own writes are read at once
+ * ({@link #readNow()}), so a page shows them without waiting for the watch.
  */
 public final class KeyAssignments implements AutoCloseable {
     private static final long SETTLE_MILLIS = 300;
     private static final List<Long> RETRY_MILLIS = List.of(1_000L, 5_000L, 30_000L);
+    private static final System.Logger LOGGER = System.getLogger(KeyAssignments.class.getName());
 
     private final Path options;
-    private final List<Runnable> listeners = new CopyOnWriteArrayList<>();
+    private final Signal changed = new Signal();
     private final ScheduledExecutorService timer = Executors.newSingleThreadScheduledExecutor(task -> Thread.ofPlatform()
             .daemon()
             .name("Companion options.txt")
             .unstarted(task));
-    /** Null where the game's folder cannot be watched, as before it exists. */
+    /** Null only where the system has no file watching at all. */
     private final WatchService watcher;
+    /**
+     * Whether the game's folder is watched now: not before it exists, or after it was removed; then its watch is tried
+     * again after 1 and 5 seconds and every 30 seconds after.
+     */
+    private volatile boolean watching;
     private ScheduledFuture<?> pending;
     /** Counts writes seen; only a read for the latest may tell, so a retry of an earlier one cannot read a write in parts. */
     private long generation;
@@ -44,40 +52,81 @@ public final class KeyAssignments implements AutoCloseable {
     /** Whether the last read failed: a page may show that failure, so the next read that succeeds is told. */
     private boolean unreadable;
 
-    /** Watches {@code options}; where its folder, the game's, cannot be watched, nothing is told. */
+    /** Watches {@code options}, whose folder, the game's, may not exist yet: it is watched once it does. */
     public KeyAssignments(Path options) {
         this.options = Objects.requireNonNull(options, "options").toAbsolutePath().normalize();
-        this.watcher = watcher(this.options.getParent());
-        if (this.watcher == null) return;
-        Thread.ofPlatform().daemon().name("Companion options.txt watcher").start(this::watch);
+        this.watcher = watchService();
+        if (this.watcher != null) {
+            Thread.ofPlatform().daemon().name("Companion options.txt watcher").start(this::watch);
+            if (!register()) registerLater(0);
+        }
         this.timer.execute(() -> read(0, 0));
     }
 
-    private static WatchService watcher(Path folder) {
-        WatchService watcher = null;
+    /**
+     * The keys the file assigns: as last read while its folder is watched, and read now where it has not been read yet
+     * or is not watched, since then a change would go unseen. The first read, whoever makes it, is what the next change
+     * is told against, so a follower that read before this owner did still hears of a later change; after it, only
+     * this owner's own reads are, so every follower hears of a change it has not seen. Blocking where it reads.
+     */
+    public Map<String, KeyBindings.Assignment> assignments() throws IOException {
+        synchronized (this) {
+            if (this.read != null && this.watching) return this.read;
+        }
+        Map<String, KeyBindings.Assignment> now = KeyBindings.readOptions(this.options);
+        synchronized (this) {
+            if (this.read == null) this.read = now;
+        }
+        return now;
+    }
+
+    private static WatchService watchService() {
         try {
-            watcher = FileSystems.getDefault().newWatchService();
-            folder.register(watcher, StandardWatchEventKinds.ENTRY_CREATE, StandardWatchEventKinds.ENTRY_MODIFY,
-                    StandardWatchEventKinds.ENTRY_DELETE);
-            return watcher;
-        } catch (IOException | RuntimeException unwatchable) {
-            System.getLogger(KeyAssignments.class.getName()).log(System.Logger.Level.WARNING,
-                    "Key assignments in " + folder + " are read when a page is shown, not watched: " + unwatchable.getMessage());
-            if (watcher != null) {
-                try {
-                    watcher.close();
-                } catch (IOException ignored) {
-                    // Nothing was registered with it.
-                }
-            }
+            return FileSystems.getDefault().newWatchService();
+        } catch (IOException | RuntimeException unavailable) {
+            LOGGER.log(System.Logger.Level.WARNING, "Key assignments are read when asked, not watched: " + unavailable.getMessage());
             return null;
         }
     }
 
-    /** Runs {@code listener} on a Companion thread after the assignments changed; returns its removal. */
-    public Runnable addListener(Runnable listener) {
-        this.listeners.add(Objects.requireNonNull(listener, "listener"));
-        return () -> this.listeners.remove(listener);
+    /** Watches the game's folder; false where it cannot be, as before it exists. */
+    private boolean register() {
+        try {
+            this.options.getParent().register(this.watcher, StandardWatchEventKinds.ENTRY_CREATE,
+                    StandardWatchEventKinds.ENTRY_MODIFY, StandardWatchEventKinds.ENTRY_DELETE);
+            this.watching = true;
+            return true;
+        } catch (IOException | RuntimeException unwatchable) {
+            return false;
+        }
+    }
+
+    /**
+     * Tries to watch the game's folder again after a while, and reads what the file assigns by then either way: while it
+     * cannot be watched, a key rebound in the game is found by these reads.
+     */
+    private void registerLater(int attempt) {
+        long delay = RETRY_MILLIS.get(Math.min(attempt, RETRY_MILLIS.size() - 1));
+        try {
+            this.timer.schedule(() -> {
+                boolean watchingNow = register();
+                written();
+                if (!watchingNow) registerLater(attempt + 1);
+            }, delay, TimeUnit.MILLISECONDS);
+        } catch (RuntimeException closed) {
+            // Closed with the project.
+        }
+    }
+
+    /** Fires on a Companion thread after the assignments changed. */
+    public Signal changed() {
+        return this.changed;
+    }
+
+    /** Reads the file now rather than once a watch saw it settle, as after Companion wrote it. */
+    public synchronized void readNow() {
+        long generation = ++this.generation;
+        schedule(() -> read(0, generation), 0);
     }
 
     private void watch() {
@@ -91,7 +140,12 @@ private void watch() {
                             || event.context() instanceof Path name && name.equals(this.options.getFileName());
                 }
                 if (ours) written();
-                if (!key.reset()) return;
+                if (!key.reset()) {
+                    // The folder was removed: what the file assigns is read now, and the folder watched again once it is back.
+                    this.watching = false;
+                    written();
+                    registerLater(0);
+                }
             }
         } catch (InterruptedException | ClosedWatchServiceException closed) {
             // Closed with the project.
@@ -130,12 +184,16 @@ private void read(int attempt, long generation) {
         }
         boolean changed;
         synchronized (this) {
-            if (generation != this.generation) return;
+            // A newer write is read after this one; what this one read is still what the next compares with.
+            if (generation != this.generation) {
+                if (this.read == null) this.read = now;
+                return;
+            }
             changed = this.unreadable || this.read != null && !this.read.equals(now);
             this.read = now;
             this.unreadable = false;
         }
-        if (changed) this.listeners.forEach(Runnable::run);
+        if (changed) this.changed.fire();
     }
 
     /** Stops watching. It never fails, so a project's shutdown goes on to its queued writes after it. */
@@ -146,8 +204,7 @@ public void close() {
         try {
             this.watcher.close();
         } catch (IOException failure) {
-            System.getLogger(KeyAssignments.class.getName()).log(System.Logger.Level.WARNING,
-                    "The watcher of " + this.options + " did not close: " + failure.getMessage());
+            LOGGER.log(System.Logger.Level.WARNING, "The watcher of " + this.options + " did not close: " + failure.getMessage());
         }
     }
 }

companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/catalog/KeyBindingControl.java

@@ -1,5 +1,6 @@
 package com.github.minecraft_ta.totalDebugCompanion.catalog;
 
+import com.github.minecraft_ta.totalDebugCompanion.util.Signal;
 import com.github.minecraft_ta.totalDebugCompanion.change.ChangeCategory;
 import com.github.minecraft_ta.totalDebugCompanion.change.ChangePipeline;
 import com.github.minecraft_ta.totalDebugCompanion.game.Access;
@@ -20,7 +21,6 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.function.Consumer;
-import java.util.function.Function;
 
 /**
  * Puts key bindings on keys, as a category of the {@link ChangePipeline}: a binding is named as in {@code options.txt},
@@ -36,24 +36,26 @@ public record Change(String name, KeyBindings.Assignment shown, KeyBindings.Assi
 
     private final ChangePipeline pipeline;
     private final Path options;
-    private final Function<Runnable, Runnable> assignmentsChanged;
+    private final KeyAssignments assignments;
 
     /**
-     * Changes the keys of the pipeline's game, in its {@code options.txt}. {@code assignmentsChanged} adds a listener for
-     * the keys that file assigns changing, whoever wrote it ({@link KeyAssignments}), and returns its removal.
+     * Changes the keys of the pipeline's game, in its {@code options.txt}, whose keys {@code assignments} follows, whoever
+     * writes the file.
      */
-    public KeyBindingControl(ChangePipeline pipeline, Function<Runnable, Runnable> assignmentsChanged) {
+    public KeyBindingControl(ChangePipeline pipeline, KeyAssignments assignments) {
         this.pipeline = Objects.requireNonNull(pipeline, "pipeline");
         this.options = pipeline.location().workspace().resolve("options.txt");
-        this.assignmentsChanged = Objects.requireNonNull(assignmentsChanged, "assignmentsChanged");
+        this.assignments = Objects.requireNonNull(assignments, "assignments");
     }
 
-    /**
-     * Runs {@code listener} when the keys {@code options.txt} assigns changed, such as a key rebound in the game's
-     * controls screen; returns its removal.
-     */
-    public Runnable addAssignmentListener(Runnable listener) {
-        return this.assignmentsChanged.apply(listener);
+    /** Fires when the keys {@code options.txt} assigns changed, such as a key rebound in the game's controls screen. */
+    public Signal assignmentsChanged() {
+        return this.assignments.changed();
+    }
+
+    /** The keys {@code options.txt} assigns now ({@link KeyAssignments#assignments()}). Blocking. */
+    public Map<String, KeyBindings.Assignment> assignments() throws IOException {
+        return this.assignments.assignments();
     }
 
     /** The change record the bindings' changes are entered in. */
@@ -73,7 +75,9 @@ public Path options() {
     public CompletableFuture<String> set(List<Change> changes) {
         List<ChangePipeline.Edit<ChangeRecord.KeyBinding, String>> edits = changes.stream().map(change -> new ChangePipeline.Edit<>(
                 new ChangeRecord.KeyBinding(change.name()), change.shown().encode(), change.assignment().encode())).toList();
-        return this.pipeline.change(this, edits).handle((done, failure) -> failure == null ? "" : message(failure));
+        // The game saved options.txt before it answered, or Companion wrote it: the owner of the keys reads what changed.
+        return this.pipeline.change(this, edits).whenComplete((done, failure) -> this.assignments.readNow())
+                .handle((done, failure) -> failure == null ? "" : message(failure));
     }
 
     private static String message(Throwable failure) {

companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/catalog/KeyBindingLabels.java

@@ -23,7 +23,7 @@ public final class KeyBindingLabels implements ChangeLabels {
     /** A key rebound outside Companion, such as in the game, changes the key a row shows. */
     @Override
     public Runnable follow(Runnable listener) {
-        return this.keys.addAssignmentListener(listener);
+        return this.keys.assignmentsChanged().subscribe(listener);
     }
 
     public KeyBindingLabels(KeyBindingControl keys) {

companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/catalog/PackCatalogService.java

@@ -1,5 +1,6 @@
 package com.github.minecraft_ta.totalDebugCompanion.catalog;
 
+import com.github.minecraft_ta.totalDebugCompanion.util.Signal;
 import com.github.minecraft_ta.totaldebug.storage.InstancePaths;
 import com.github.minecraft_ta.totaldebug.storage.PackCatalog;
 import com.github.minecraft_ta.totaldebug.storage.RuntimeInventory;
@@ -44,7 +45,7 @@ public record Failed(String detail) implements State {
     }
 
     private final InstancePaths paths;
-    private final List<Runnable> listeners = new CopyOnWriteArrayList<>();
+    private final Signal changed = new Signal();
     private State state = new None();
     private long generation;
 
@@ -61,10 +62,9 @@ public Optional<CatalogIndex> index() {
         return Optional.ofNullable(shown(state()));
     }
 
-    /** Listeners run on the Swing thread after the state changed. */
-    public Runnable addListener(Runnable listener) {
-        this.listeners.add(Objects.requireNonNull(listener, "listener"));
-        return () -> this.listeners.remove(listener);
+    /** Fires on the Swing thread after the state changed. */
+    public Signal changed() {
+        return this.changed;
     }
 
     /**
@@ -108,7 +108,7 @@ public void capturing() {
             this.state = new Capturing(shown);
         }
         // Pages that show the catalog go on showing it; only one without a catalog says it is being captured.
-        if (!shownBefore) SwingUtilities.invokeLater(() -> this.listeners.forEach(Runnable::run));
+        if (!shownBefore) SwingUtilities.invokeLater(this.changed::fire);
     }
 
     /**
@@ -183,7 +183,7 @@ private void apply(long expectedGeneration, State state) {
             }
             this.state = state;
         }
-        SwingUtilities.invokeLater(() -> this.listeners.forEach(Runnable::run));
+        SwingUtilities.invokeLater(this.changed::fire);
     }
 
     /** The catalog {@code state} shows: the one ready, or while Minecraft captures it again, the one before; or null. */

companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/model/DefinitionView.java

@@ -1,5 +1,6 @@
 package com.github.minecraft_ta.totalDebugCompanion.model;
 
+import com.github.minecraft_ta.totalDebugCompanion.storage.ChangeRecord;
 import com.github.minecraft_ta.totalDebugCompanion.navigation.NavigationTarget;
 import com.github.minecraft_ta.totalDebugCompanion.ui.EditorContext;
 import com.github.minecraft_ta.totalDebugCompanion.ui.components.catalog.DefinitionDetails;
@@ -18,7 +19,7 @@ public DefinitionView(EditorContext context, SubjectRef.Definition subject) {
         this.subject = subject;
         this.panel = SubjectPanel.definition(subject, new DefinitionDetails.Services(context.project().catalog(),
                 () -> context.project().sources(), context.itemIcons(), context.navigation()::navigate,
-                context.project().packs()::addResourcePackListener));
+                context.project().packs().changed(ChangeRecord.PackSide.RESOURCES)::subscribe));
     }
 
     public SubjectRef.Definition subject() {

companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/model/InspectionView.java

@@ -1,5 +1,6 @@
 package com.github.minecraft_ta.totalDebugCompanion.model;
 
+import com.github.minecraft_ta.totalDebugCompanion.storage.ChangeRecord;
 import com.github.minecraft_ta.totalDebugCompanion.navigation.NavigationTarget;
 import com.github.minecraft_ta.totalDebugCompanion.runtime.RuntimeBinding;
 import com.github.minecraft_ta.totalDebugCompanion.ui.EditorContext;
@@ -22,7 +23,7 @@ public InspectionView(EditorContext context, InspectSubjectPayload subject, Runt
         this.subject = Objects.requireNonNull(subject, "subject");
         this.panel = SubjectPanel.occurrence(subject, context.snippets(), () -> context.project().scriptFiles(),
                 new DefinitionDetails.Services(context.project().catalog(), () -> context.project().sources(),
-                        context.itemIcons(), context.navigation()::navigate, context.project().packs()::addResourcePackListener));
+                        context.itemIcons(), context.navigation()::navigate, context.project().packs().changed(ChangeRecord.PackSide.RESOURCES)::subscribe));
     }
 
     public InspectSubjectPayload subject() {

companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/model/KeyBindingsView.java

@@ -17,11 +17,6 @@ public KeyBindingsView(EditorContext context) {
                 context.navigation()::navigate);
     }
 
-    /** Reads the keys again. */
-    public void refresh() {
-        this.panel.load();
-    }
-
     /** Shows a binding, such as {@code key.jump}; an empty name shows none. */
     public void show(String binding) {
         if (!binding.isEmpty()) this.panel.select(binding);