Make variable card binding in the scripting terminal visible and safer

src/main/java/org/cyclops/integratedscripting/api/language/ILanguageHandler.java

@@ -8,7 +8,9 @@
 import org.cyclops.integrateddynamics.api.evaluate.EvaluationException;
 import org.cyclops.integratedscripting.api.network.IScriptFactory;
 
+import java.util.Collections;
 import java.util.List;
+import java.util.Set;
 
 /**
  * Logic for handling a specific programming language.
@@ -27,6 +29,13 @@ public interface ILanguageHandler {
 
     public IScriptFactory getScriptFactory() throws EvaluationException;
 
+    /**
+     * @return Words of the language that can not be the name of a script member, such as keywords.
+     */
+    public default Set<String> getReservedWords() {
+        return Collections.emptySet();
+    }
+
     public void onServerStarted();
 
 }

src/main/java/org/cyclops/integratedscripting/client/gui/container/ContainerScreenTerminalScripting.java

@@ -88,8 +88,6 @@ public ContainerScreenTerminalScripting(ContainerTerminalScripting container, In
         this.imageHeight = 240;
         this.titleLabelX = 88;
         this.titleLabelY = 6;
-        this.inventoryLabelX = 88;
-        this.inventoryLabelY = this.imageHeight - 94;
         this.firstRow = 0;
         this.lastClientSyncTick = 0;
     }
@@ -306,14 +304,48 @@ protected Path getHoveredScriptPath(double mouseX, double mouseY) {
 
     @Override
     protected void renderLabels(GuiGraphics guiGraphics, int mouseX, int mouseY) {
-        super.renderLabels(guiGraphics, mouseX, mouseY);
+        // The inventory label is left out, so its row can show the card binding
+        guiGraphics.drawString(this.font, this.title, this.titleLabelX, this.titleLabelY, 4210752, false);
 
         // Draw disk label
         guiGraphics.drawString(font, L10NHelpers.localize("gui.integratedscripting.disk") + ":", 8, 6, 16777215);
 
+        // Draw the member the card is bound to, or will be bound to, right-aligned next to the card slot
+        Component bindingLabel = getBindingLabel();
+        if (bindingLabel != null) {
+            int right = getErrorX() - 2;
+            guiGraphics.drawString(font, bindingLabel, right - font.width(bindingLabel), getErrorY() + 2, 4210752, false);
+        }
+
         displayErrors.drawForeground(guiGraphics.pose(), getMenu().getReadErrors(), getErrorX(), getErrorY(), mouseX, mouseY, this, this.leftPos, this.topPos);
     }
 
+    @Nullable
+    protected Component getBindingLabel() {
+        if (getMenu().getActiveScriptPath() == null) {
+            return null;
+        }
+        String cardMember = getMenu().getCardMember();
+        if (cardMember != null) {
+            return Component.translatable("gui.integratedscripting.binding.card", formatBindingMember(cardMember, "gui.integratedscripting.binding.card"));
+        }
+        if (getMenu().isMemberSelected()) {
+            // Not bound yet, so shown less prominently
+            return Component.translatable("gui.integratedscripting.binding.next", formatBindingMember(getMenu().getSelectedMember(), "gui.integratedscripting.binding.next"))
+                    .withStyle(style -> style.withItalic(true).withColor(0x707070));
+        }
+        return null;
+    }
+
+    protected Component formatBindingMember(String member, String translationKey) {
+        // Truncate the member so the label fits between the editor's left border and the status icon
+        int maxWidth = getErrorX() - 2 - (SCRIPT_X_INNER - 6) - font.width(L10NHelpers.localize(translationKey, ""));
+        if (font.width(member) > maxWidth) {
+            member = font.plainSubstrByWidth(member, maxWidth - font.width("\u2026")) + "\u2026";
+        }
+        return Component.literal(member).withStyle(style -> style.withColor(0x1D4F91));
+    }
+
     @Override
     public boolean mouseClicked(double mouseX, double mouseY, int mouseButton) {
         // Make active dialog consume all input

src/main/java/org/cyclops/integratedscripting/core/language/LanguageHandlerJavaScript.java

@@ -22,6 +22,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * @author rubensworks
@@ -35,6 +36,14 @@ public class LanguageHandlerJavaScript implements ILanguageHandler {
     public static final Style CONSTANT = Style.EMPTY.withColor(TextColor.fromRgb(Helpers.RGBToInt(0, 134, 179))); // Blue
     public static final Style KEYWORD = Style.EMPTY.withColor(TextColor.fromRgb(Helpers.RGBToInt(167, 29, 93))); // Red-purple
 
+    // Keywords and literals, which can never be the name of a member in a (non-strict) script
+    public static final Set<String> RESERVED_WORDS = Set.of(
+            "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", "else",
+            "enum", "export", "extends", "false", "finally", "for", "function", "if", "import", "in", "instanceof",
+            "new", "null", "return", "super", "switch", "this", "throw", "true", "try", "typeof", "var", "void",
+            "while", "with"
+    );
+
     public final Map<String, Style> tokenStyles;
     private final IScriptFactory scriptFactory;
 
@@ -110,6 +119,11 @@ public List<Pair<Style, String>> markupLine(String line) {
         return segments;
     }
 
+    @Override
+    public Set<String> getReservedWords() {
+        return RESERVED_WORDS;
+    }
+
     @Override
     public IScriptFactory getScriptFactory() throws EvaluationException {
         return this.scriptFactory;

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

@@ -0,0 +1,64 @@
+package org.cyclops.integratedscripting.gametest;
+
+import it.unimi.dsi.fastutil.ints.IntList;
+import net.minecraft.gametest.framework.GameTest;
+import net.minecraft.gametest.framework.GameTestHelper;
+import net.minecraft.nbt.CompoundTag;
+import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.level.GameType;
+import net.neoforged.neoforge.gametest.GameTestHolder;
+import net.neoforged.neoforge.gametest.PrefixGameTestTemplate;
+import org.cyclops.integrateddynamics.RegistryEntries;
+import org.cyclops.integrateddynamics.api.part.PartTarget;
+import org.cyclops.integrateddynamics.core.helper.PartHelpers;
+import org.cyclops.integratedscripting.Reference;
+import org.cyclops.integratedscripting.api.network.IScriptingData;
+import org.cyclops.integratedscripting.core.network.ScriptingNetworkHelpers;
+import org.cyclops.integratedscripting.inventory.container.ContainerTerminalScripting;
+import org.cyclops.integratedscripting.part.PartTypes;
+
+import java.nio.file.Path;
+
+import static org.cyclops.integratedscripting.gametest.GameTestHelpersIntegratedScripting.createBasicNetwork;
+
+@GameTestHolder(Reference.MOD_ID)
+@PrefixGameTestTemplate(false)
+public class GameTestsTerminalScripting {
+
+    public static final String TEMPLATE_EMPTY = "empty10";
+
+    @GameTest(template = TEMPLATE_EMPTY)
+    public void testTerminalScriptingKeywordDoesNotRebindCard(GameTestHelper helper) {
+        GameTestHelpersIntegratedScripting.NetworkPositions positions = createBasicNetwork(helper, GameTestsScripts.POS);
+        Path path = Path.of("script0.js");
+        ScriptingNetworkHelpers.getScriptingData().setScript(positions.diskId(), path, "function abc() {}\nconst def = 1;", IScriptingData.ChangeLocation.MEMORY);
+
+        ContainerTerminalScripting container = new ContainerTerminalScripting(0, helper.makeMockPlayer(GameType.CREATIVE).getInventory(),
+                PartTarget.fromCenter(positions.terminal()), PartHelpers.getPartContainer(positions.terminal().getPos(), positions.terminal().getSide()),
+                PartTypes.TERMINAL_SCRIPTING, new ContainerTerminalScripting.InitData(IntList.of(positions.diskId()))) {
+            // Handle values as if they were sent by the client, as the mock player has no connection
+            @Override
+            public void setValue(int valueId, CompoundTag value) {
+                onUpdate(valueId, value);
+            }
+        };
+        container.setActiveScriptPath(path);
+
+        // Inserting a card binds it to the selected member
+        container.setSelection("abc");
+        container.getContainerInventory().setItem(0, new ItemStack(RegistryEntries.ITEM_VARIABLE));
+        helper.assertValueEqual(container.getCardMember(), "abc", "card member");
+
+        // Selecting a keyword keeps the card as is
+        container.setSelection("const");
+        helper.assertFalse(container.isMemberSelected(), "Keyword must not be a selected member");
+        helper.assertValueEqual(container.getCardMember(), "abc", "card member");
+
+        // Selecting another member rebinds the card
+        container.setSelection("def");
+        helper.assertValueEqual(container.getCardMember(), "def", "card member");
+
+        helper.succeed();
+    }
+
+}

src/main/java/org/cyclops/integratedscripting/inventory/container/ContainerTerminalScripting.java

@@ -23,6 +23,8 @@
 import org.cyclops.cyclopscore.inventory.container.InventoryContainer;
 import org.cyclops.cyclopscore.persist.IDirtyMarkListener;
 import org.cyclops.integrateddynamics.IntegratedDynamics;
+import org.cyclops.integrateddynamics.api.evaluate.variable.ValueDeseralizationContext;
+import org.cyclops.integrateddynamics.api.item.IVariableFacade;
 import org.cyclops.integrateddynamics.api.item.IVariableFacadeHandlerRegistry;
 import org.cyclops.integrateddynamics.api.network.INetwork;
 import org.cyclops.integrateddynamics.api.part.IPartContainer;
@@ -33,10 +35,12 @@
 import org.cyclops.integratedscripting.IntegratedScripting;
 import org.cyclops.integratedscripting.RegistryEntries;
 import org.cyclops.integratedscripting.api.item.IScriptVariableFacade;
+import org.cyclops.integratedscripting.api.language.ILanguageHandler;
 import org.cyclops.integratedscripting.api.network.IScriptingData;
 import org.cyclops.integratedscripting.api.network.IScriptingNetwork;
 import org.cyclops.integratedscripting.core.evaluate.ScriptVariableFacadeHandler;
 import org.cyclops.integratedscripting.core.item.ScriptVariableFacade;
+import org.cyclops.integratedscripting.core.language.LanguageHandlers;
 import org.cyclops.integratedscripting.core.network.ScriptingNetworkHelpers;
 import org.cyclops.integratedscripting.network.packet.TerminalScriptingCreateNewScriptPacket;
 import org.cyclops.integratedscripting.network.packet.TerminalScriptingDeleteScriptPacket;
@@ -291,7 +295,31 @@ public void setSelection(String selection) {
     }
 
     public boolean isMemberSelected() {
-        return VALID_MEMBER_NAME.matcher(getSelectedMember()).matches();
+        String member = getSelectedMember();
+        return VALID_MEMBER_NAME.matcher(member).matches() && !isReservedWord(member);
+    }
+
+    /**
+     * @return If the given word is reserved in the language of the active script, such as a keyword.
+     */
+    public boolean isReservedWord(String word) {
+        Path path = getActiveScriptPath();
+        ILanguageHandler languageHandler = path == null ? null : LanguageHandlers.REGISTRY.getProvider(path);
+        return languageHandler != null && languageHandler.getReservedWords().contains(word);
+    }
+
+    /**
+     * @return The member the variable card in the slot is bound to, or null if there is no bound card.
+     */
+    @Nullable
+    public String getCardMember() {
+        ItemStack itemStack = getContainerInventory().getItem(0);
+        if (itemStack.isEmpty()) {
+            return null;
+        }
+        IVariableFacadeHandlerRegistry registry = IntegratedDynamics._instance.getRegistryManager().getRegistry(IVariableFacadeHandlerRegistry.class);
+        IVariableFacade variableFacade = registry.handle(ValueDeseralizationContext.of(getLevel()), itemStack);
+        return variableFacade instanceof IScriptVariableFacade scriptVariableFacade ? scriptVariableFacade.getMember() : null;
     }
 
     @Nullable
@@ -332,8 +360,12 @@ public List<MutableComponent> getReadErrors() {
         List<MutableComponent> list = Lists.newArrayList();
         if (canWriteScriptToVariable()) {
             if (!isMemberSelected()) {
-                list.add(Component.translatable("gui.integratedscripting.error.invalid_member"));
-                list.add(Component.translatable("gui.integratedscripting.error.invalid_member.current", getSelectedMember()));
+                if (isReservedWord(getSelectedMember())) {
+                    list.add(Component.translatable("gui.integratedscripting.error.reserved_member", getSelectedMember()));
+                } else {
+                    list.add(Component.translatable("gui.integratedscripting.error.invalid_member"));
+                    list.add(Component.translatable("gui.integratedscripting.error.invalid_member.current", getSelectedMember()));
+                }
             }
         }
         return list;

src/main/resources/assets/integratedscripting/lang/en_us.json

@@ -32,6 +32,9 @@
   "gui.integratedscripting.removal_dialog.keep": "Keep",
   "gui.integratedscripting.error.invalid_member": "Select a valid function name or variable name inside the script editor to bind with.",
   "gui.integratedscripting.error.invalid_member.current": "Current selection: \"%s\"",
+  "gui.integratedscripting.error.reserved_member": "\"%s\" is a keyword, so it can not be bound. Select a function name or variable name instead.",
+  "gui.integratedscripting.binding.next": "Binds to: %s",
+  "gui.integratedscripting.binding.card": "Bound to: %s",
 
   "_comment": "Value Type Translation",
   "valuetype.integratedscripting.error.translation.unknown_to_graal": "Could not find a translator from %s values to Graal.",

src/test/java/org/cyclops/integratedscripting/core/language/LanguageHandlerJavaScriptTest.java

@@ -0,0 +1,60 @@
+package org.cyclops.integratedscripting.core.language;
+
+import org.graalvm.polyglot.Context;
+import org.graalvm.polyglot.PolyglotException;
+import org.graalvm.polyglot.Source;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * @author rubensworks
+ */
+public class LanguageHandlerJavaScriptTest {
+
+    private static Context CTX;
+
+    @BeforeClass
+    public static void beforeClass() {
+        CTX = Context.newBuilder("js").build();
+    }
+
+    @AfterClass
+    public static void afterClass() {
+        CTX.close();
+    }
+
+    private static boolean isValidVariableName(String name) {
+        try {
+            CTX.parse(Source.create("js", "var " + name + " = 1;"));
+            return true;
+        } catch (PolyglotException e) {
+            if (e.isSyntaxError()) {
+                return false;
+            }
+            throw e;
+        }
+    }
+
+    @Test
+    public void testReservedWordsAreNoValidNames() {
+        for (String word : LanguageHandlerJavaScript.RESERVED_WORDS) {
+            if (isValidVariableName(word)) {
+                fail("\"" + word + "\" is a valid name in a script, so it must not be reserved");
+            }
+        }
+    }
+
+    @Test
+    public void testValidNamesAreNotReserved() {
+        for (String name : new String[]{"double", "offset", "of", "async", "get", "set", "let", "static", "yield", "await"}) {
+            assertTrue(name, isValidVariableName(name));
+            assertFalse(name, LanguageHandlerJavaScript.RESERVED_WORDS.contains(name));
+        }
+    }
+
+}