Fix display panels not recovering from script errors after external edits

src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java

@@ -60,6 +60,9 @@ public class ScriptingData implements IScriptingData {
     private final Map<Path, WatchKey> pathWatchers = new ConcurrentHashMap<>();
     private final Map<WatchKey, Path> pathWatchersReverse = new ConcurrentHashMap<>();
 
+    // Disks changed on the file system, reloaded on the server thread
+    private final Set<Integer> pendingDiskReloads = ConcurrentHashMap.newKeySet();
+
     private boolean initialized = false;
     private WatchService watchService;
 
@@ -74,10 +77,16 @@ public Path getRootPath() {
     public void tick() {
         // Initialize once
         if (!this.initialized) {
-            this.initialized = true;
             this.initialize();
         }
 
+        // Reload externally changed disks here, as change listeners may access the world
+        for (Iterator<Integer> it = pendingDiskReloads.iterator(); it.hasNext(); ) {
+            int diskId = it.next();
+            it.remove();
+            this.initializeDiskScripts(diskId);
+        }
+
         // Flush changes to disk if needed
         if (!dirtyPaths.isEmpty()) {
             for (Pair<Integer, Path> entry : dirtyPaths) {
@@ -106,6 +115,8 @@ protected Path getDiskPath(int id) {
     }
 
     public void initialize() {
+        // Also when called directly, so a later tick doesn't start a second file watcher
+        this.initialized = true;
         Path disksPath = getDisksPath();
         disksPath.toFile().mkdirs();
 
@@ -146,7 +157,6 @@ public void initialize() {
         watchThread.start();
     }
 
-    // Synchronized, as this is also called from the file watcher thread
     protected synchronized void initializeDiskScripts(int diskId) {
         // Watch all directories before reading files, so files created during reading still trigger a reload
         this.registerDiskWatchers(diskId);
@@ -207,8 +217,8 @@ protected void watchFiles() {
                             if (changedPathAbsolute.startsWith(disksPath)) {
                                 try {
                                     int diskId = Integer.parseInt(changedPathAbsolute.getName(disksPath.getNameCount()).toString());
-                                    // Re-initialize the full directory
-                                    this.initializeDiskScripts(diskId);
+                                    // Re-initialize the full directory on the next tick
+                                    this.pendingDiskReloads.add(diskId);
                                 } catch (NumberFormatException e) {
                                     // Ignore non-disk directories
                                 }

src/main/java/org/cyclops/integratedscripting/gametest/GameTestsScripts.java

@@ -18,10 +18,14 @@
 import org.cyclops.integrateddynamics.core.evaluate.variable.ValueTypeString;
 import org.cyclops.integrateddynamics.core.evaluate.variable.ValueTypes;
 import org.cyclops.integrateddynamics.part.PartTypePanelDisplay;
+import org.cyclops.integratedscripting.IntegratedScripting;
 import org.cyclops.integratedscripting.Reference;
 import org.cyclops.integratedscripting.api.network.IScriptingData;
 import org.cyclops.integratedscripting.core.network.ScriptingNetworkHelpers;
 
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 
 import static org.cyclops.integrateddynamics.gametest.GameTestHelpersIntegratedDynamics.*;
@@ -188,4 +192,33 @@ public void testScriptsDisplayScriptInfiniteLoop(GameTestHelper helper) {
         });
     }
 
+    @GameTest(template = TEMPLATE_EMPTY, timeoutTicks = TIMEOUT)
+    public void testScriptsDisplayScriptErrorResolvedAfterExternalFix(GameTestHelper helper) {
+        GameTestHelpersIntegratedScripting.NetworkPositions positions = createBasicNetwork(helper, POS);
+
+        // Write a script that fails when evaluated
+        ScriptingNetworkHelpers.getScriptingData().setScript(positions.diskId(), Path.of("script0.js"), "const abc = doesNotExist();", IScriptingData.ChangeLocation.MEMORY);
+
+        // Create variable from script and place it in a display
+        ItemStack variableScript = createVariableForScript(helper.getLevel(), positions.diskId(), Path.of("script0.js"), "abc");
+        Pair<PartTypePanelDisplay, PartTypePanelDisplay.State> partAndState = placeVariableInDisplayPanel(helper.getLevel(), positions.displayPanel(), variableScript);
+
+        helper.startSequence()
+                .thenWaitUntil(() -> helper.assertFalse(partAndState.getRight().getGlobalErrors().isEmpty(), "Display panel must show the script error"))
+                // Let the network settle, so that only the script change can make the display update
+                .thenIdle(40)
+                // Fix the script externally, so the file watcher picks it up
+                .thenExecute(() -> {
+                    Path scriptFile = IntegratedScripting._instance.scriptingData.getRootPath()
+                            .resolve("scripting-disks").resolve(String.valueOf(positions.diskId())).resolve("script0.js");
+                    try {
+                        Files.writeString(scriptFile, "const abc = 3;");
+                    } catch (IOException e) {
+                        throw new UncheckedIOException(e);
+                    }
+                })
+                .thenWaitUntil(() -> assertValueEqual(partAndState.getRight().getDisplayValue(), ValueTypeInteger.ValueInteger.of(3)))
+                .thenSucceed();
+    }
+
 }

src/test/java/org/cyclops/integratedscripting/core/network/ScriptingDataTest.java

@@ -136,7 +136,7 @@ public void testExternalUpdatesOnRuntimeCreatedDiskFolderSynced() throws IOExcep
             scriptingData.initialize();
             Path diskPath = rootPath.resolve("scripting-disks").resolve("123");
             Files.createDirectories(diskPath);
-            awaitCondition(() -> scriptingData.getDisks().contains(123));
+            awaitCondition(scriptingData, () -> scriptingData.getDisks().contains(123));
 
             Path scriptPath = Path.of("main.js");
             int observedValue = writeScriptValueUntilObserved(
@@ -291,7 +291,7 @@ public void testExternalFilesCreatedAfterDeletionsSynced() throws IOException, I
                 Files.writeString(diskPath.resolve("new" + i + ".js"), "new");
             }
 
-            awaitCondition(() -> scriptingData.getScripts(901).keySet().equals(Sets.newHashSet(
+            awaitCondition(scriptingData, () -> scriptingData.getScripts(901).keySet().equals(Sets.newHashSet(
                     Path.of("new0.js"), Path.of("new1.js"), Path.of("new2.js"), Path.of("new3.js"))));
         } finally {
             scriptingData.close();
@@ -312,36 +312,39 @@ public void testExternalFilesInNewSubdirectoriesSynced() throws IOException, Int
             // A file in a new nested directory
             Files.createDirectories(diskPath.resolve("lib").resolve("helpers"));
             Files.writeString(diskPath.resolve("lib").resolve("helpers").resolve("a.js"), "a");
-            awaitCondition(() -> "a".equals(scriptingData.getScripts(902).get(Path.of("lib/helpers/a.js"))));
+            awaitCondition(scriptingData, () -> "a".equals(scriptingData.getScripts(902).get(Path.of("lib/helpers/a.js"))));
 
             // A file in the intermediate directory, which contains no scripts yet
             Files.writeString(diskPath.resolve("lib").resolve("b.js"), "b");
-            awaitCondition(() -> "b".equals(scriptingData.getScripts(902).get(Path.of("lib/b.js"))));
+            awaitCondition(scriptingData, () -> "b".equals(scriptingData.getScripts(902).get(Path.of("lib/b.js"))));
 
             // A file in an empty directory that already existed
             Files.createDirectories(diskPath.resolve("empty"));
             Files.writeString(diskPath.resolve("main.js"), "main2");
-            awaitCondition(() -> "main2".equals(scriptingData.getScripts(902).get(Path.of("main.js"))));
+            awaitCondition(scriptingData, () -> "main2".equals(scriptingData.getScripts(902).get(Path.of("main.js"))));
             Files.writeString(diskPath.resolve("empty").resolve("c.js"), "c");
-            awaitCondition(() -> "c".equals(scriptingData.getScripts(902).get(Path.of("empty/c.js"))));
+            awaitCondition(scriptingData, () -> "c".equals(scriptingData.getScripts(902).get(Path.of("empty/c.js"))));
         } finally {
             scriptingData.close();
             FileUtils.deleteDirectory(rootPath.toFile());
         }
     }
 
-    private static void awaitCondition(Condition condition) throws InterruptedException {
-        assertTrue("Timed out waiting for condition", awaitCondition(condition, AWAIT_TIMEOUT_MS));
+    private static void awaitCondition(ScriptingData scriptingData, Condition condition) throws InterruptedException {
+        assertTrue("Timed out waiting for condition", awaitCondition(scriptingData, condition, AWAIT_TIMEOUT_MS));
     }
 
-    private static boolean awaitCondition(Condition condition, int timeoutMs) throws InterruptedException {
+    private static boolean awaitCondition(ScriptingData scriptingData, Condition condition, int timeoutMs) throws InterruptedException {
         long deadline = System.currentTimeMillis() + timeoutMs;
         while (System.currentTimeMillis() < deadline) {
+            // External changes are applied on tick
+            scriptingData.tick();
             if (condition.matches()) {
                 return true;
             }
             Thread.sleep(POLLING_INTERVAL_MS);
         }
+        scriptingData.tick();
         return condition.matches();
     }
 
@@ -352,7 +355,7 @@ private static int writeScriptValueUntilObserved(ScriptingData scriptingData, in
         while (System.currentTimeMillis() < deadline) {
             String expectedScriptValue = scriptValue(value);
             Files.writeString(scriptPathAbsolute, expectedScriptValue);
-            if (awaitCondition(() -> expectedScriptValue.equals(scriptingData.getScripts(disk).get(scriptPathRelative)),
+            if (awaitCondition(scriptingData, () -> expectedScriptValue.equals(scriptingData.getScripts(disk).get(scriptPathRelative)),
                     ATTEMPT_TIMEOUT_MS)) {
                 return value;
             }