From 27b78cc84e7d84130221636e84f5320d56a35026 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 18 Oct 2026 19:19:09 +0000 Subject: [PATCH 1/2] Fix display panels not recovering from script errors after external edits Script changes made by an external editor were applied on the file watcher thread. Resolving the cached error there could not find the part's network, as the world can not be accessed off-thread, so the variable kept its error. Externally changed disks are now reloaded on the server tick. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018MRyTdV3fSV5Q5NPGep4q9 --- .../core/network/ScriptingData.java | 15 +++++++-- .../gametest/GameTestsScripts.java | 33 +++++++++++++++++++ .../core/network/ScriptingDataTest.java | 23 +++++++------ 3 files changed, 58 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java b/src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java index eea57b951..2a1d12da6 100644 --- a/src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java +++ b/src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java @@ -60,6 +60,9 @@ public class ScriptingData implements IScriptingData { private final Map pathWatchers = new ConcurrentHashMap<>(); private final Map pathWatchersReverse = new ConcurrentHashMap<>(); + // Disks changed on the file system, reloaded on the server thread + private final Set pendingDiskReloads = ConcurrentHashMap.newKeySet(); + private boolean initialized = false; private WatchService watchService; @@ -78,6 +81,13 @@ public void tick() { this.initialize(); } + // Reload externally changed disks here, as change listeners may access the world + for (Iterator 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 entry : dirtyPaths) { @@ -146,7 +156,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 +216,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 } diff --git a/src/main/java/org/cyclops/integratedscripting/gametest/GameTestsScripts.java b/src/main/java/org/cyclops/integratedscripting/gametest/GameTestsScripts.java index b7c2186e9..bb61adbdd 100644 --- a/src/main/java/org/cyclops/integratedscripting/gametest/GameTestsScripts.java +++ b/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 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(); + } + } diff --git a/src/test/java/org/cyclops/integratedscripting/core/network/ScriptingDataTest.java b/src/test/java/org/cyclops/integratedscripting/core/network/ScriptingDataTest.java index 5ab3cf714..0851c1f15 100644 --- a/src/test/java/org/cyclops/integratedscripting/core/network/ScriptingDataTest.java +++ b/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; } From 7f740cc003fa9b7c0dfbd39e4de81cdf827ef29f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 15:17:58 +0000 Subject: [PATCH 2/2] Don't start a second file watcher when scripting data is initialized directly Initializing the scripting data before its first tick, as tests do, made the tick initialize it again. This replaced the watch service and started a second watcher thread, so changes in some directories could be missed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018MRyTdV3fSV5Q5NPGep4q9 --- .../integratedscripting/core/network/ScriptingData.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java b/src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java index 2a1d12da6..7eb4fd8ac 100644 --- a/src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java +++ b/src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java @@ -77,7 +77,6 @@ public Path getRootPath() { public void tick() { // Initialize once if (!this.initialized) { - this.initialized = true; this.initialize(); } @@ -116,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();