From 7dea53dadf94db1dab641422bd28f139bbea0629 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 18:21:56 +0000 Subject: [PATCH 1/6] Add undo and redo to the script editor Scripts are saved on every edit and right-click selected the whole script, so a right-click followed by a single key press replaced and saved the entire script, without any way to get it back. Add a per-file undo history (ctrl+z, and ctrl+y or ctrl+shift+z to redo) that groups consecutive typing, and stop selecting everything on right-click. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018MRyTdV3fSV5Q5NPGep4q9 --- .../input/TextFieldHelperExtended.java | 5 +- .../gui/component/input/TextHistory.java | 75 +++++++++++++++ .../gui/component/input/WidgetTextArea.java | 56 ++++++++++- .../ContainerScreenTerminalScripting.java | 4 + .../gui/component/input/TextHistoryTest.java | 93 +++++++++++++++++++ 5 files changed, 228 insertions(+), 5 deletions(-) create mode 100644 src/main/java/org/cyclops/integratedscripting/client/gui/component/input/TextHistory.java create mode 100644 src/test/java/org/cyclops/integratedscripting/client/gui/component/input/TextHistoryTest.java diff --git a/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/TextFieldHelperExtended.java b/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/TextFieldHelperExtended.java index 2fca066bd..2cc00c2cc 100644 --- a/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/TextFieldHelperExtended.java +++ b/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/TextFieldHelperExtended.java @@ -27,8 +27,9 @@ public TextFieldHelperExtended(Supplier getMessageFn, Consumer s public String getSelected() { String value = this.getMessageFn.get(); - int i = Math.min(this.getCursorPos(), this.getSelectionPos()); - int j = Math.max(this.getCursorPos(), this.getSelectionPos()); + // Positions may still refer to a previous, longer value + int i = Math.min(Math.min(this.getCursorPos(), this.getSelectionPos()), value.length()); + int j = Math.min(Math.max(this.getCursorPos(), this.getSelectionPos()), value.length()); return value.substring(i, j); } diff --git a/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/TextHistory.java b/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/TextHistory.java new file mode 100644 index 000000000..bf5cefa30 --- /dev/null +++ b/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/TextHistory.java @@ -0,0 +1,75 @@ +package org.cyclops.integratedscripting.client.gui.component.input; + +import javax.annotation.Nullable; +import java.util.ArrayDeque; +import java.util.Deque; + +/** + * Undo and redo history of a text area. + * @author rubensworks + */ +public class TextHistory { + + public static final int MAX_SIZE = 500; + public static final long TYPING_GROUP_MILLIS = 1000; + + private final Deque undoStack = new ArrayDeque<>(); + private final Deque redoStack = new ArrayDeque<>(); + private boolean lastWasTyping; + private long lastRecordTime; + private int lastTypingEnd = -1; + + /** + * Record the state before an edit. + * Consecutive typing at the same position within a short time is grouped into a single undo step. + * @param before The state before the edit. + * @param typing If the edit was caused by typing a single character. + * @param time The current time in milliseconds. + */ + public void record(State before, boolean typing, long time) { + boolean group = typing && this.lastWasTyping && !this.undoStack.isEmpty() + && time - this.lastRecordTime < TYPING_GROUP_MILLIS + && Math.min(before.cursorPos(), before.selectionPos()) == this.lastTypingEnd; + if (!group) { + this.undoStack.push(before); + if (this.undoStack.size() > MAX_SIZE) { + this.undoStack.removeLast(); + } + } + this.redoStack.clear(); + this.lastWasTyping = typing; + this.lastRecordTime = time; + this.lastTypingEnd = typing ? Math.min(before.cursorPos(), before.selectionPos()) + 1 : -1; + } + + /** + * @param current The current state, which will become redoable. + * @return The state to restore, or null if there is nothing to undo. + */ + @Nullable + public State undo(State current) { + if (this.undoStack.isEmpty()) { + return null; + } + this.redoStack.push(current); + this.lastWasTyping = false; + return this.undoStack.pop(); + } + + /** + * @param current The current state, which will become undoable. + * @return The state to restore, or null if there is nothing to redo. + */ + @Nullable + public State redo(State current) { + if (this.redoStack.isEmpty()) { + return null; + } + this.undoStack.push(current); + this.lastWasTyping = false; + return this.redoStack.pop(); + } + + public static record State(String value, int cursorPos, int selectionPos) {} + +} diff --git a/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/WidgetTextArea.java b/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/WidgetTextArea.java index 6cf30e0d7..728caa959 100644 --- a/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/WidgetTextArea.java +++ b/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/WidgetTextArea.java @@ -91,6 +91,7 @@ public class WidgetTextArea extends EditBox implements GuiEventListener { @Nullable private WidgetScrollBar scrollBar; private int firstRow; + private TextHistory history = new TextHistory(); public WidgetTextArea(Font font, int x, int y, int width, int height, Component narrationMessage, boolean scrollBar, boolean showLineNumbers) { super(font, x, y, width, height, narrationMessage); @@ -152,6 +153,10 @@ public void setListenerCursor(@Nullable IInputListener listenerCursor) { this.listenerCursor = listenerCursor; } + public void setHistory(TextHistory history) { + this.history = history; + } + public void setMarkupProvider(@Nullable IMarkupProvider markupProvider) { this.markupProvider = markupProvider; } @@ -236,9 +241,8 @@ public void tick() { public boolean mouseClicked(double mouseX, double mouseY, int mouseButton) { if (mouseButton == 1 && mouseX >= this.getX() && mouseX < this.getX() + this.width && mouseY >= this.getY() && mouseY < this.getY() + this.height) { - // Select everything + // Don't select everything on right-click, as typing would then replace the whole script this.setFocused(true); - textFieldHelper.selectAll(); return true; } else { if (mouseButton == 0 && mouseX >= this.getX() && mouseX < this.getX() + this.width @@ -312,8 +316,13 @@ public boolean mouseDragged(double mouseX, double mouseY, int mouseButton, doubl @Override public boolean keyPressed(int typedChar, int keyCode, int modifiers) { - boolean flag = this.textFieldKeyPressed(typedChar, keyCode, modifiers); + TextHistory.State before = this.getState(); + boolean history = this.historyKeyPressed(typedChar, modifiers); + boolean flag = history || this.textFieldKeyPressed(typedChar, keyCode, modifiers); if (flag) { + if (!history) { + this.recordHistory(before, false); + } this.clearDisplayCache(); this.setFocused(true); this.ensureCursorVisible(); @@ -331,7 +340,9 @@ public boolean keyPressed(int typedChar, int keyCode, int modifiers) { public boolean charTyped(char typedChar, int keyCode) { // Don't delegate to EditBox, as that would consume the character into its own (unused) value if (StringUtil.isAllowedChatCharacter(typedChar)) { + TextHistory.State before = this.getState(); this.textFieldHelper.insertText(Character.toString(typedChar)); + this.recordHistory(before, true); this.clearDisplayCache(); this.setFocused(true); this.ensureCursorVisible(); @@ -341,6 +352,45 @@ public boolean charTyped(char typedChar, int keyCode) { } } + private TextHistory.State getState() { + return new TextHistory.State(this.getValue(), this.textFieldHelper.getCursorPos(), this.textFieldHelper.getSelectionPos()); + } + + private void setState(TextHistory.State state) { + this.setValuePassive(state.value()); + this.textFieldHelper.setSelectionRange(state.cursorPos(), state.selectionPos()); + } + + private void recordHistory(TextHistory.State before, boolean typing) { + if (!before.value().equals(this.getValue())) { + this.history.record(before, typing, Util.getMillis()); + } + } + + // Modifiers are read from the key event instead of the global keyboard state + private static boolean hasControlDown(int modifiers) { + return (modifiers & (Minecraft.ON_OSX ? GLFW.GLFW_MOD_SUPER : GLFW.GLFW_MOD_CONTROL)) != 0; + } + + private boolean historyKeyPressed(int key, int modifiers) { + if (!hasControlDown(modifiers)) { + return false; + } + boolean shift = (modifiers & GLFW.GLFW_MOD_SHIFT) != 0; + TextHistory.State state; + if (key == GLFW.GLFW_KEY_Z && !shift) { + state = this.history.undo(this.getState()); + } else if (key == GLFW.GLFW_KEY_Y || (key == GLFW.GLFW_KEY_Z && shift)) { + state = this.history.redo(this.getState()); + } else { + return false; + } + if (state != null) { + this.setState(state); + } + return true; + } + private boolean textFieldKeyPressed(int p_98153_, int p_98154_, int p_98155_) { if (Screen.isSelectAll(p_98153_)) { this.textFieldHelper.selectAll(); diff --git a/src/main/java/org/cyclops/integratedscripting/client/gui/container/ContainerScreenTerminalScripting.java b/src/main/java/org/cyclops/integratedscripting/client/gui/container/ContainerScreenTerminalScripting.java index 2f64ceb31..fc51a8bff 100644 --- a/src/main/java/org/cyclops/integratedscripting/client/gui/container/ContainerScreenTerminalScripting.java +++ b/src/main/java/org/cyclops/integratedscripting/client/gui/container/ContainerScreenTerminalScripting.java @@ -30,6 +30,7 @@ import org.cyclops.integratedscripting.IntegratedScripting; import org.cyclops.integratedscripting.Reference; import org.cyclops.integratedscripting.api.language.ILanguageHandler; +import org.cyclops.integratedscripting.client.gui.component.input.TextHistory; import org.cyclops.integratedscripting.client.gui.component.input.WidgetDialog; import org.cyclops.integratedscripting.client.gui.component.input.WidgetTextArea; import org.cyclops.integratedscripting.client.gui.image.ScriptImages; @@ -81,6 +82,7 @@ public class ContainerScreenTerminalScripting extends ContainerScreenExtended lastScriptCursorPos = Maps.newHashMap(); private final Map lastScriptSelectionPos = Maps.newHashMap(); private final Int2ObjectMap lastDiskScript = new Int2ObjectAVLTreeMap<>(); + private final Map, TextHistory> scriptHistories = Maps.newHashMap(); public ContainerScreenTerminalScripting(ContainerTerminalScripting container, Inventory inventory, Component title) { super(container, inventory, title); @@ -499,6 +501,8 @@ public void setFirstRow(int firstRow) { private void onActiveScriptSelected() { String script = getMenu().getActiveScript(); this.textArea.setValue(script == null ? "" : script); + Path path = getMenu().getActiveScriptPath(); + this.textArea.setHistory(path == null ? new TextHistory() : scriptHistories.computeIfAbsent(Pair.of(getMenu().getActiveDisk(), path), k -> new TextHistory())); } private void onActiveScriptModified() { diff --git a/src/test/java/org/cyclops/integratedscripting/client/gui/component/input/TextHistoryTest.java b/src/test/java/org/cyclops/integratedscripting/client/gui/component/input/TextHistoryTest.java new file mode 100644 index 000000000..eeb523565 --- /dev/null +++ b/src/test/java/org/cyclops/integratedscripting/client/gui/component/input/TextHistoryTest.java @@ -0,0 +1,93 @@ +package org.cyclops.integratedscripting.client.gui.component.input; + +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.junit.Assert.assertThat; + +public class TextHistoryTest { + + private static TextHistory.State s(String value, int cursor) { + return new TextHistory.State(value, cursor, cursor); + } + + @Test + public void testEmpty() { + TextHistory history = new TextHistory(); + assertThat(history.undo(s("a", 1)), nullValue()); + assertThat(history.redo(s("a", 1)), nullValue()); + } + + @Test + public void testUndoRedo() { + TextHistory history = new TextHistory(); + history.record(s("", 0), false, 0); + assertThat(history.undo(s("pasted", 6)), equalTo(s("", 0))); + assertThat(history.undo(s("", 0)), nullValue()); + assertThat(history.redo(s("", 0)), equalTo(s("pasted", 6))); + assertThat(history.redo(s("pasted", 6)), nullValue()); + } + + @Test + public void testGroupsConsecutiveTyping() { + TextHistory history = new TextHistory(); + history.record(s("", 0), true, 0); + history.record(s("a", 1), true, 100); + history.record(s("ab", 2), true, 200); + assertThat(history.undo(s("abc", 3)), equalTo(s("", 0))); + assertThat(history.undo(s("", 0)), nullValue()); + } + + @Test + public void testDoesNotGroupTypingAfterPause() { + TextHistory history = new TextHistory(); + history.record(s("", 0), true, 0); + history.record(s("a", 1), true, 5000); + assertThat(history.undo(s("ab", 2)), equalTo(s("a", 1))); + assertThat(history.undo(s("a", 1)), equalTo(s("", 0))); + } + + @Test + public void testDoesNotGroupTypingAtOtherPosition() { + TextHistory history = new TextHistory(); + history.record(s("xy", 0), true, 0); + history.record(s("axy", 3), true, 100); + assertThat(history.undo(s("axyb", 4)), equalTo(s("axy", 3))); + } + + @Test + public void testDoesNotGroupTypingWithOtherEdits() { + TextHistory history = new TextHistory(); + history.record(s("", 0), true, 0); + history.record(s("a", 1), false, 100); + history.record(s("a\n", 2), true, 200); + assertThat(history.undo(s("a\nb", 3)), equalTo(s("a\n", 2))); + assertThat(history.undo(s("a\n", 2)), equalTo(s("a", 1))); + assertThat(history.undo(s("a", 1)), equalTo(s("", 0))); + } + + @Test + public void testNewEditClearsRedo() { + TextHistory history = new TextHistory(); + history.record(s("", 0), false, 0); + history.undo(s("a", 1)); + history.record(s("", 0), false, 100); + assertThat(history.redo(s("b", 1)), nullValue()); + } + + @Test + public void testMaxSize() { + TextHistory history = new TextHistory(); + for (int i = 0; i < TextHistory.MAX_SIZE + 10; i++) { + history.record(s(String.valueOf(i), 0), false, i * 10000L); + } + TextHistory.State last = null; + TextHistory.State state; + while ((state = history.undo(s("current", 0))) != null) { + last = state; + } + assertThat(last, equalTo(s("10", 0))); + } + +} From 1ffe4be9efecaab65e09833aa91ebed3fc6a92bc Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 18:22:09 +0000 Subject: [PATCH 2/6] Don't echo synced scripts back to the client When the client synced a script, the server sent the same version back. If the player typed in the meantime, this echo overwrote the newer client version, which was then synced instead, losing the last keystrokes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018MRyTdV3fSV5Q5NPGep4q9 --- .../inventory/container/ContainerTerminalScripting.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/org/cyclops/integratedscripting/inventory/container/ContainerTerminalScripting.java b/src/main/java/org/cyclops/integratedscripting/inventory/container/ContainerTerminalScripting.java index 268e0c1ba..8eddf5294 100644 --- a/src/main/java/org/cyclops/integratedscripting/inventory/container/ContainerTerminalScripting.java +++ b/src/main/java/org/cyclops/integratedscripting/inventory/container/ContainerTerminalScripting.java @@ -250,6 +250,9 @@ public Int2ObjectMap> getLastScripts() { } public void setServerScript(int disk, Path path, @Nullable String script) { + // The client already has this version, so don't send it back in broadcastChanges. + // Otherwise, this echo could overwrite newer client changes that were not synced yet. + setLastScript(disk, path, script); ScriptingNetworkHelpers.getScriptingData().setScript(disk, path, script, IScriptingData.ChangeLocation.MEMORY); } From 89a13424dd544642e65683287614085f25eae98f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 18:59:59 +0000 Subject: [PATCH 3/6] Write scripts to disk atomically Scripts were flushed by truncating and rewriting the file, so the file watcher could reload a partially written or empty script into memory and send it to clients. Write to a temporary file outside the watched directories and move it into place instead, keeping the permissions of existing files and the default permissions for new ones. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018MRyTdV3fSV5Q5NPGep4q9 --- .../core/network/ScriptingData.java | 28 +++++++++- .../core/network/ScriptingDataTest.java | 55 +++++++++++++++++++ 2 files changed, 82 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 5df42670a..9ee70381b 100644 --- a/src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java +++ b/src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java @@ -18,14 +18,18 @@ import java.io.IOException; import java.io.OutputStream; import java.nio.charset.StandardCharsets; +import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.ClosedWatchServiceException; import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.nio.file.StandardWatchEventKinds; import java.nio.file.WatchEvent; import java.nio.file.WatchKey; import java.nio.file.WatchService; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFilePermissions; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -390,6 +394,28 @@ public Pair getOutputStreams(int disk, Path scriptPa ); } + /** + * Write a file without exposing partially written contents to the file watcher. + * The temporary file is created outside the watched disk directories. + */ + protected void writeAtomically(Path path, String contents) throws IOException { + Path tempFile = Files.createTempFile(rootPath, "flush", ".tmp"); + try { + // Temporary files are only accessible by their owner, so use the permissions of a regular file instead + if (Files.getFileStore(tempFile).supportsFileAttributeView(PosixFileAttributeView.class)) { + Files.setPosixFilePermissions(tempFile, Files.exists(path) ? Files.getPosixFilePermissions(path) : PosixFilePermissions.fromString("rw-r--r--")); + } + Files.writeString(tempFile, contents, StandardCharsets.UTF_8); + try { + Files.move(tempFile, path, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); + } catch (AtomicMoveNotSupportedException e) { + Files.move(tempFile, path, StandardCopyOption.REPLACE_EXISTING); + } + } finally { + Files.deleteIfExists(tempFile); + } + } + protected void flushScript(int disk, Path scriptPathRelative) { Map scripts = getScripts(disk); String script = scripts.get(scriptPathRelative); @@ -399,7 +425,7 @@ protected void flushScript(int disk, Path scriptPathRelative) { FileUtils.delete(scriptPathAbsolute.toFile()); } else { scriptPathAbsolute.getParent().toFile().mkdirs(); - FileUtils.write(scriptPathAbsolute.toFile(), script, StandardCharsets.UTF_8); + writeAtomically(scriptPathAbsolute, script); this.registerPathWatcher(disk, null); this.registerPathWatcher(disk, scriptPathRelative.getParent()); } 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 eb09f4ae9..0935f5786 100644 --- a/src/test/java/org/cyclops/integratedscripting/core/network/ScriptingDataTest.java +++ b/src/test/java/org/cyclops/integratedscripting/core/network/ScriptingDataTest.java @@ -171,6 +171,61 @@ public void testExternalUpdatesOnRuntimeFirstUsedDiskSynced() throws IOException } } + @Test + public void testFlushDoesNotExposePartialContents() throws IOException, InterruptedException { + Path rootPath = Files.createTempDirectory("integratedscripting-test"); + ScriptingData scriptingData = new ScriptingData(rootPath); + try { + scriptingData.initialize(); + Path scriptPath = Path.of("main.js"); + String body = "x".repeat(1_000_000); + + // The file watcher reloads the disk after each flush, which must never see a partially written file + for (int i = 0; i < 20; i++) { + scriptingData.setScript(789, scriptPath, body + i + "//end", IScriptingData.ChangeLocation.MEMORY); + scriptingData.tick(); + long deadline = System.currentTimeMillis() + 100; + while (System.currentTimeMillis() < deadline) { + String script = scriptingData.getScripts(789).get(scriptPath); + assertTrue("Observed partial script contents", script != null && script.endsWith("//end")); + } + } + + try (var files = Files.list(rootPath)) { + assertTrue("Temporary files were left behind", files.noneMatch(f -> f.toString().endsWith(".tmp"))); + } + } finally { + scriptingData.close(); + FileUtils.deleteDirectory(rootPath.toFile()); + } + } + + @Test + public void testFlushKeepsFilePermissions() throws IOException { + Path rootPath = Files.createTempDirectory("integratedscripting-test"); + ScriptingData scriptingData = new ScriptingData(rootPath); + try { + org.junit.Assume.assumeTrue(Files.getFileStore(rootPath).supportsFileAttributeView(java.nio.file.attribute.PosixFileAttributeView.class)); + Path scriptPath = Path.of("main.js"); + Path scriptPathAbsolute = rootPath.resolve("scripting-disks").resolve("321").resolve(scriptPath); + + // New files are readable by others + scriptingData.setScript(321, scriptPath, scriptValue(1), IScriptingData.ChangeLocation.MEMORY); + scriptingData.tick(); + assertThat(java.nio.file.attribute.PosixFilePermissions.toString(Files.getPosixFilePermissions(scriptPathAbsolute)), equalTo("rw-r--r--")); + + // Existing permissions are kept + Files.setPosixFilePermissions(scriptPathAbsolute, java.nio.file.attribute.PosixFilePermissions.fromString("rw-rw-r--")); + scriptingData.setScript(321, scriptPath, scriptValue(2), IScriptingData.ChangeLocation.MEMORY); + scriptingData.tick(); + assertThat(java.nio.file.attribute.PosixFilePermissions.toString(Files.getPosixFilePermissions(scriptPathAbsolute)), equalTo("rw-rw-r--")); + assertThat(Files.readString(scriptPathAbsolute), equalTo(scriptValue(2))); + } 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)); } From 9622668b2e40c7df1b7d36723ed2643db9ae8f03 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 18:23:06 +0000 Subject: [PATCH 4/6] Show external script changes in the open editor When a script changed outside the editor, for example by editing the file on disk, the open editor kept showing the old text, and the next key press synced that old text back, discarding the external change. Reload the active script when the server sends a newer version, as an undoable edit that keeps the cursor position. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018MRyTdV3fSV5Q5NPGep4q9 --- .../gui/component/input/WidgetTextArea.java | 11 +++++++++++ .../ContainerScreenTerminalScripting.java | 11 +++++++++++ .../container/ContainerTerminalScripting.java | 18 ++++++++++++++++++ .../TerminalScriptingModifiedScriptPacket.java | 1 + 4 files changed, 41 insertions(+) diff --git a/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/WidgetTextArea.java b/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/WidgetTextArea.java index 728caa959..86da7a15c 100644 --- a/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/WidgetTextArea.java +++ b/src/main/java/org/cyclops/integratedscripting/client/gui/component/input/WidgetTextArea.java @@ -367,6 +367,17 @@ private void recordHistory(TextHistory.State before, boolean typing) { } } + /** + * Replace the value as an undoable edit, keeping the cursor position where possible. + * @param value The new value. + */ + public void replaceValue(String value) { + TextHistory.State before = this.getState(); + this.setState(new TextHistory.State(value, before.cursorPos(), before.selectionPos())); + this.recordHistory(before, false); + this.clearDisplayCache(); + } + // Modifiers are read from the key event instead of the global keyboard state private static boolean hasControlDown(int modifiers) { return (modifiers & (Minecraft.ON_OSX ? GLFW.GLFW_MOD_SUPER : GLFW.GLFW_MOD_CONTROL)) != 0; diff --git a/src/main/java/org/cyclops/integratedscripting/client/gui/container/ContainerScreenTerminalScripting.java b/src/main/java/org/cyclops/integratedscripting/client/gui/container/ContainerScreenTerminalScripting.java index fc51a8bff..84eef1b32 100644 --- a/src/main/java/org/cyclops/integratedscripting/client/gui/container/ContainerScreenTerminalScripting.java +++ b/src/main/java/org/cyclops/integratedscripting/client/gui/container/ContainerScreenTerminalScripting.java @@ -127,6 +127,7 @@ public int getTotalRows() { textArea = new WidgetTextArea(Minecraft.getInstance().font, this.leftPos + SCRIPT_X + 1, this.topPos + SCRIPT_Y + 1, SCRIPT_WIDTH, SCRIPT_HEIGHT, Component.translatable("gui.integratedscripting.script"), true, true); textArea.setListener(this::onActiveScriptModified); + getMenu().setRemoteScriptChangeListener(this::onRemoteScriptChanged); textArea.setListenerSelection(this::onSelectionModified); textArea.setListenerCursor(this::onSelectionModified); textArea.setMarkupProvider((style, line) -> { @@ -505,6 +506,16 @@ private void onActiveScriptSelected() { this.textArea.setHistory(path == null ? new TextHistory() : scriptHistories.computeIfAbsent(Pair.of(getMenu().getActiveDisk(), path), k -> new TextHistory())); } + private void onRemoteScriptChanged(int disk, Path path) { + // Show changes from outside this editor, such as external file edits, instead of overwriting them on the next edit + if (disk == getMenu().getActiveDisk() && path.equals(getMenu().getActiveScriptPath())) { + String script = getMenu().getActiveScript(); + if (script != null && !script.equals(this.textArea.getValue())) { + this.textArea.replaceValue(script); + } + } + } + private void onActiveScriptModified() { getMenu().setActiveScript(this.textArea.getValue()); } diff --git a/src/main/java/org/cyclops/integratedscripting/inventory/container/ContainerTerminalScripting.java b/src/main/java/org/cyclops/integratedscripting/inventory/container/ContainerTerminalScripting.java index 8eddf5294..e41ac3169 100644 --- a/src/main/java/org/cyclops/integratedscripting/inventory/container/ContainerTerminalScripting.java +++ b/src/main/java/org/cyclops/integratedscripting/inventory/container/ContainerTerminalScripting.java @@ -50,6 +50,7 @@ import javax.annotation.Nullable; import java.nio.file.Path; import java.util.*; +import java.util.function.BiConsumer; import java.util.regex.Pattern; /** @@ -74,6 +75,8 @@ public class ContainerTerminalScripting extends InventoryContainer implements ID private final int activeDiskId; private final int activeScriptPathId; private final int selectionId; + @Nullable + private BiConsumer remoteScriptChangeListener; public ContainerTerminalScripting(int id, Inventory playerInventory, RegistryFriendlyByteBuf packetBuffer) { this(id, playerInventory, PartHelpers.readPartTarget(packetBuffer), Optional.empty(), @@ -245,6 +248,21 @@ public void setLastScript(int disk, Path path, @Nullable String script) { } } + public void setRemoteScriptChangeListener(@Nullable BiConsumer remoteScriptChangeListener) { + this.remoteScriptChangeListener = remoteScriptChangeListener; + } + + /** + * Called on the client when a script was changed by the server. + * @param disk The disk id. + * @param path The script path. + */ + public void onRemoteScriptChange(int disk, Path path) { + if (this.remoteScriptChangeListener != null) { + this.remoteScriptChangeListener.accept(disk, path); + } + } + public Int2ObjectMap> getLastScripts() { return lastScripts; } diff --git a/src/main/java/org/cyclops/integratedscripting/network/packet/TerminalScriptingModifiedScriptPacket.java b/src/main/java/org/cyclops/integratedscripting/network/packet/TerminalScriptingModifiedScriptPacket.java index ae34d68f2..e35b6ff48 100644 --- a/src/main/java/org/cyclops/integratedscripting/network/packet/TerminalScriptingModifiedScriptPacket.java +++ b/src/main/java/org/cyclops/integratedscripting/network/packet/TerminalScriptingModifiedScriptPacket.java @@ -59,6 +59,7 @@ public boolean isAsync() { public void actionClient(Level world, Player player) { if(player.containerMenu instanceof ContainerTerminalScripting container) { container.setLastScript(disk, Path.of(path), getScript()); + container.onRemoteScriptChange(disk, Path.of(path)); } } From 63e7aa2f69336e531533a05a32e66b7e2da0a5ac Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 18 Oct 2026 05:24:19 +0000 Subject: [PATCH 5/6] Only apply external changes when the file watcher reloads scripts When the file watcher reloaded a disk directory, it replaced the scripts in memory with whatever was on disk at that moment. As it also triggers on the server's own writes, it could run while a script was not written yet, for example right after creating the directory of a new disk, which made that script disappear. Concurrent reloads could also overwrite each other. Remember the contents last written or read for each file, only apply files that changed outside of the mod, and don't let reloads, flushes and script changes run at the same time. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018MRyTdV3fSV5Q5NPGep4q9 --- .../core/network/ScriptingData.java | 40 ++++++++++++++--- .../core/network/ScriptingDataTest.java | 44 +++++++++++++++++++ 2 files changed, 78 insertions(+), 6 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 9ee70381b..ece96edc9 100644 --- a/src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java +++ b/src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java @@ -38,6 +38,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; /** @@ -48,6 +49,8 @@ public class ScriptingData implements IScriptingData { public static final LevelResource LEVEL_RESOURCE = new LevelResource("integratedscripting"); private final Int2ObjectMap> diskScripts = new Int2ObjectAVLTreeMap<>(); + // The file contents as last written or read by this class, to detect changes made outside of it + private final Map> knownDiskContents = new ConcurrentHashMap<>(); private final Path rootPath; private final Set> dirtyPaths = Sets.newHashSet(); private final Int2ObjectMap> scriptChangeListeners = new Int2ObjectAVLTreeMap<>(); @@ -140,8 +143,9 @@ public void initialize() { watchThread.start(); } - protected void initializeDiskScripts(int diskId) { - Map scripts = Maps.newHashMap(); + // Synchronized, as this is also called from the file watcher thread + protected synchronized void initializeDiskScripts(int diskId) { + Map scriptsDisk = Maps.newHashMap(); Path diskDir = getDiskPath(diskId); Iterator filesIt = FileUtils.iterateFilesAndDirs(diskDir.toFile(), FileFilterUtils.trueFileFilter(), FileFilterUtils.trueFileFilter()); while (filesIt.hasNext()) { @@ -149,12 +153,33 @@ protected void initializeDiskScripts(int diskId) { if (file.isFile()) { Path filePathRelative = diskDir.relativize(file.toPath()); try { - scripts.put(filePathRelative, FileUtils.readFileToString(file, StandardCharsets.UTF_8)); + scriptsDisk.put(filePathRelative, FileUtils.readFileToString(file, StandardCharsets.UTF_8)); } catch (IOException e) { e.printStackTrace(); } } } + + // Only apply changes made outside of this class. + // The file watcher may also trigger while scripts are being written here, + // in which case the disk contents are outdated compared to the scripts in memory. + Map known = knownDiskContents.computeIfAbsent(diskId, k -> new ConcurrentHashMap<>()); + Map scriptsMemory = diskScripts.get(diskId); + Map scripts = scriptsMemory == null ? Maps.newHashMap() : Maps.newHashMap(scriptsMemory); + Set paths = Sets.newHashSet(scriptsDisk.keySet()); + paths.addAll(known.keySet()); + for (Path path : paths) { + String scriptDisk = scriptsDisk.get(path); + if (!Objects.equals(scriptDisk, known.get(path))) { + if (scriptDisk == null) { + scripts.remove(path); + known.remove(path); + } else { + scripts.put(path, scriptDisk); + known.put(path, scriptDisk); + } + } + } this.setScripts(diskId, scripts, ChangeLocation.DISK); } @@ -207,7 +232,7 @@ public Map getScripts(int disk) { } @Override - public void setScripts(int disk, Map scripts, ChangeLocation changeLocation) { + public synchronized void setScripts(int disk, Map scripts, ChangeLocation changeLocation) { // Update script data Map oldScripts = diskScripts.put(disk, scripts); @@ -291,7 +316,7 @@ boolean isScriptPathSafe(int disk, Path supplied) { } @Override - public void setScript(int disk, Path scriptPathRelative, @Nullable String script, ChangeLocation changeLocation) { + public synchronized void setScript(int disk, Path scriptPathRelative, @Nullable String script, ChangeLocation changeLocation) { // Validate that the path is safe and contained within the disk root if (!isScriptPathSafe(disk, scriptPathRelative)) { return; @@ -416,16 +441,19 @@ protected void writeAtomically(Path path, String contents) throws IOException { } } - protected void flushScript(int disk, Path scriptPathRelative) { + protected synchronized void flushScript(int disk, Path scriptPathRelative) { Map scripts = getScripts(disk); String script = scripts.get(scriptPathRelative); Path scriptPathAbsolute = getDiskPath(disk).resolve(scriptPathRelative); try { + Map known = knownDiskContents.computeIfAbsent(disk, k -> new ConcurrentHashMap<>()); if (script == null) { FileUtils.delete(scriptPathAbsolute.toFile()); + known.remove(scriptPathRelative); } else { scriptPathAbsolute.getParent().toFile().mkdirs(); writeAtomically(scriptPathAbsolute, script); + known.put(scriptPathRelative, script); this.registerPathWatcher(disk, null); this.registerPathWatcher(disk, scriptPathRelative.getParent()); } 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 0935f5786..3d40ac31c 100644 --- a/src/test/java/org/cyclops/integratedscripting/core/network/ScriptingDataTest.java +++ b/src/test/java/org/cyclops/integratedscripting/core/network/ScriptingDataTest.java @@ -226,6 +226,50 @@ public void testFlushKeepsFilePermissions() throws IOException { } } + @Test + public void testDiskReloadDuringFirstFlushKeepsScript() throws IOException { + Path rootPath = Files.createTempDirectory("integratedscripting-test"); + ScriptingData scriptingData = new ScriptingData(rootPath); + try { + scriptingData.initialize(); + scriptingData.setScript(777, Path.of("main.js"), "value", IScriptingData.ChangeLocation.MEMORY); + + // The file watcher reloads the disk once its directory is created, possibly before the script is written + Files.createDirectories(rootPath.resolve("scripting-disks").resolve("777")); + scriptingData.initializeDiskScripts(777); + + assertThat(scriptingData.getScripts(777).get(Path.of("main.js")), equalTo("value")); + } finally { + scriptingData.close(); + FileUtils.deleteDirectory(rootPath.toFile()); + } + } + + @Test + public void testDiskReloadAppliesExternalChanges() throws IOException { + Path rootPath = Files.createTempDirectory("integratedscripting-test"); + ScriptingData scriptingData = new ScriptingData(rootPath); + try { + Path diskPath = rootPath.resolve("scripting-disks").resolve("778"); + Files.createDirectories(diskPath); + Files.writeString(diskPath.resolve("a.js"), "a1"); + Files.writeString(diskPath.resolve("b.js"), "b1"); + scriptingData.initialize(); + assertThat(scriptingData.getScripts(778).get(Path.of("a.js")), equalTo("a1")); + + // Externally modify one file and delete another + Files.writeString(diskPath.resolve("a.js"), "a2"); + Files.delete(diskPath.resolve("b.js")); + scriptingData.initializeDiskScripts(778); + + assertThat(scriptingData.getScripts(778).get(Path.of("a.js")), equalTo("a2")); + assertThat(scriptingData.getScripts(778).get(Path.of("b.js")), nullValue()); + } 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)); } From 926fd6503bf8d3bb9279e52dacc1ccf193297078 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 18 Oct 2026 05:59:11 +0000 Subject: [PATCH 6/6] Notice all externally created script files Files created outside of the game did not always show up until another file changed. Every reload cancelled and re-registered the directory watchers, losing events that happened in between, and only directories that already contained scripts were watched, so files in new, empty or intermediate directories were missed. Watch all directories of a disk before reading its files, keep existing watchers registered, and clean up watchers of deleted directories. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018MRyTdV3fSV5Q5NPGep4q9 --- .../core/network/ScriptingData.java | 63 ++++++++++--------- .../core/network/ScriptingDataTest.java | 60 ++++++++++++++++++ 2 files changed, 95 insertions(+), 28 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 ece96edc9..eea57b951 100644 --- a/src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java +++ b/src/main/java/org/cyclops/integratedscripting/core/network/ScriptingData.java @@ -17,6 +17,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.ClosedWatchServiceException; @@ -40,6 +41,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; +import java.util.stream.Stream; /** * @author rubensworks @@ -54,8 +56,9 @@ public class ScriptingData implements IScriptingData { private final Path rootPath; private final Set> dirtyPaths = Sets.newHashSet(); private final Int2ObjectMap> scriptChangeListeners = new Int2ObjectAVLTreeMap<>(); - private final Map pathWatchers = Maps.newHashMap(); - private final Map pathWatchersReverse = Maps.newHashMap(); + // Accessed by both the server thread and the file watcher thread + private final Map pathWatchers = new ConcurrentHashMap<>(); + private final Map pathWatchersReverse = new ConcurrentHashMap<>(); private boolean initialized = false; private WatchService watchService; @@ -145,6 +148,9 @@ public void initialize() { // 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); + Map scriptsDisk = Maps.newHashMap(); Path diskDir = getDiskPath(diskId); Iterator filesIt = FileUtils.iterateFilesAndDirs(diskDir.toFile(), FileFilterUtils.trueFileFilter(), FileFilterUtils.trueFileFilter()); @@ -210,6 +216,13 @@ protected void watchFiles() { } } poll = key.reset(); + if (!poll) { + // The directory was deleted + Path path = pathWatchersReverse.remove(key); + if (path != null) { + pathWatchers.remove(path); + } + } Thread.yield(); } } catch (InterruptedException e) { @@ -273,18 +286,8 @@ public synchronized void setScripts(int disk, Map scripts, ChangeL } } - // Unregister old watchers - if (oldScripts != null) { - for (Path path : oldScripts.keySet()) { - this.unregisterPathWatcher(disk, path.getParent()); - } - } - - // Register watchers for all directories - this.registerPathWatcher(disk, null); - for (Path path : scripts.keySet()) { - this.registerPathWatcher(disk, path.getParent()); - } + // Watchers are not re-registered, as events that happen in between would be lost + this.registerDiskWatchers(disk); } // Invoke listeners @@ -340,7 +343,7 @@ public synchronized void setScript(int disk, Path scriptPathRelative, @Nullable } if (changeLocation == ChangeLocation.DISK) { // Register watcher - this.registerPathWatcher(disk, scriptPathRelative.getParent()); + this.registerDiskWatchers(disk); } // Invoke listeners @@ -352,6 +355,21 @@ public synchronized void setScript(int disk, Path scriptPathRelative, @Nullable } } + /** + * Watch all directories of a disk, including empty ones, so that new files in any of them are noticed. + */ + protected void registerDiskWatchers(int diskId) { + Path diskPath = getDiskPath(diskId); + if (!Files.isDirectory(diskPath)) { + return; + } + try (Stream paths = Files.walk(diskPath)) { + paths.filter(Files::isDirectory).forEach(this::registerAbsolutePathWatcher); + } catch (IOException | UncheckedIOException e) { + // A directory may have been deleted while walking, which will trigger another reload + } + } + protected void registerPathWatcher(int diskId, @Nullable Path pathRelative) { Path diskPath = getDiskPath(diskId); Path pathAbsolute = pathRelative == null ? diskPath : diskPath.resolve(pathRelative); @@ -373,17 +391,6 @@ protected void registerAbsolutePathWatcher(Path pathAbsolute) { } } - protected void unregisterPathWatcher(int diskId, @Nullable Path pathRelative) { - Path diskPath = getDiskPath(diskId); - Path pathAbsolute = pathRelative == null ? diskPath : diskPath.resolve(pathRelative); - WatchKey watchKey = pathWatchers.get(pathAbsolute); - if (watchKey != null) { - pathWatchers.remove(pathAbsolute); - pathWatchersReverse.remove(watchKey); - watchKey.cancel(); - } - } - @Override public void markDirty(int disk, Path scriptPathRelative) { dirtyPaths.add(Pair.of(disk, scriptPathRelative)); @@ -454,8 +461,8 @@ protected synchronized void flushScript(int disk, Path scriptPathRelative) { scriptPathAbsolute.getParent().toFile().mkdirs(); writeAtomically(scriptPathAbsolute, script); known.put(scriptPathRelative, script); - this.registerPathWatcher(disk, null); - this.registerPathWatcher(disk, scriptPathRelative.getParent()); + // Also watch intermediate directories that were created + this.registerDiskWatchers(disk); } } catch (IOException e) { e.printStackTrace(); 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 3d40ac31c..5ab3cf714 100644 --- a/src/test/java/org/cyclops/integratedscripting/core/network/ScriptingDataTest.java +++ b/src/test/java/org/cyclops/integratedscripting/core/network/ScriptingDataTest.java @@ -1,5 +1,6 @@ package org.cyclops.integratedscripting.core.network; +import com.google.common.collect.Sets; import org.apache.commons.io.FileUtils; import org.cyclops.integratedscripting.api.network.IScriptingData; import org.junit.Test; @@ -270,6 +271,65 @@ public void testDiskReloadAppliesExternalChanges() throws IOException { } } + @Test + public void testExternalFilesCreatedAfterDeletionsSynced() throws IOException, InterruptedException { + Path rootPath = Files.createTempDirectory("integratedscripting-test"); + ScriptingData scriptingData = new ScriptingData(rootPath); + try { + Path diskPath = rootPath.resolve("scripting-disks").resolve("901"); + Files.createDirectories(diskPath); + for (int i = 0; i < 4; i++) { + Files.writeString(diskPath.resolve("old" + i + ".js"), "old"); + } + scriptingData.initialize(); + + // Delete and create files in quick succession, as a file manager or script would + for (int i = 0; i < 4; i++) { + Files.delete(diskPath.resolve("old" + i + ".js")); + } + for (int i = 0; i < 4; i++) { + Files.writeString(diskPath.resolve("new" + i + ".js"), "new"); + } + + awaitCondition(() -> 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(); + FileUtils.deleteDirectory(rootPath.toFile()); + } + } + + @Test + public void testExternalFilesInNewSubdirectoriesSynced() throws IOException, InterruptedException { + Path rootPath = Files.createTempDirectory("integratedscripting-test"); + ScriptingData scriptingData = new ScriptingData(rootPath); + try { + Path diskPath = rootPath.resolve("scripting-disks").resolve("902"); + Files.createDirectories(diskPath); + Files.writeString(diskPath.resolve("main.js"), "main"); + scriptingData.initialize(); + + // 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")))); + + // 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")))); + + // 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")))); + Files.writeString(diskPath.resolve("empty").resolve("c.js"), "c"); + awaitCondition(() -> "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)); }