reworked container access (never throw, multi-field access, return values, operator), added package storage

src/main/java/carpet/CarpetServer.java

@@ -111,6 +111,7 @@ public class CarpetServer // static for now - easier to handle all around the co
 
     public static void onServerClosed(MinecraftServer server)
     {
+        scriptServer.onClose();
         LoggerRegistry.stopLoggers();
         extensions.forEach(e -> e.onServerClosed(server));
     }

src/main/java/carpet/script/CarpetExpression.java

@@ -50,6 +50,7 @@ import net.minecraft.item.BlockItem;
 import net.minecraft.item.ItemStack;
 import net.minecraft.item.Items;
 import net.minecraft.nbt.CompoundTag;
+import net.minecraft.nbt.Tag;
 import net.minecraft.particle.ParticleEffect;
 import net.minecraft.predicate.entity.EntityPredicates;
 import net.minecraft.server.MinecraftServer;
@@ -2873,6 +2874,34 @@ public class CarpetExpression
             CarpetServer.scriptServer.events.scheduleCall(cc, funname, args, delay);
             return (c_, t_) -> Value.TRUE;
         });
+
+        this.expr.addLazyFunction("get_global_state", -1, (c, t, lv) ->
+        {
+            String file = null;
+            if (lv.size()>0)
+                file = lv.get(0).evalValue(c).getString();
+            Tag state = ((CarpetContext)c).host.getGlobalState(file);
+            if (state == null)
+                return (cc, tt) -> Value.NULL;
+            Value retVal = new NBTSerializableValue(state);
+            return (cc, tt) -> retVal;
+        });
+
+        this.expr.addLazyFunction("set_global_state", -1, (c, t, lv) ->
+        {
+            if (lv.size() == 0)
+                throw new InternalExpressionException("save_state needs nbt tag and optional file");
+            Value val = lv.get(0).evalValue(c);
+            String file = null;
+            if (lv.size()>1)
+                file = lv.get(1).evalValue(c).getString();
+            NBTSerializableValue tagValue =  (val instanceof NBTSerializableValue)
+                    ? (NBTSerializableValue) val
+                    : new NBTSerializableValue(val.getString());
+            Tag tag = tagValue.getTag();
+            ((CarpetContext)c).host.setGlobalState(tag, file);
+            return (cc, tt) -> Value.NULL;
+        });
     }
 
     /**

src/main/java/carpet/script/CarpetScriptServer.java

@@ -126,7 +126,7 @@ public class CarpetScriptServer
 
     private ScriptHost createMinecraftScriptHost(String name, ModuleInterface module, boolean perPlayer, ServerCommandSource source)
     {
-        ScriptHost host = new ScriptHost(name, module, perPlayer );
+        ScriptHost host = new ScriptHost(name, module, perPlayer, null );
         host.globalVariables.put("_x", (c, t) -> Value.ZERO);
         host.globalVariables.put("_y", (c, t) -> Value.ZERO);
         host.globalVariables.put("_z", (c, t) -> Value.ZERO);
@@ -360,5 +360,17 @@ public class CarpetScriptServer
     public void tick()
     {
         events.tick();
+        for (ScriptHost host : modules.values())
+        {
+            host.tick();
+        }
+    }
+
+    public void onClose()
+    {
+        for (ScriptHost host : modules.values())
+        {
+            host.onClose();
+        }
     }
 }

src/main/java/carpet/script/Expression.java

@@ -1243,29 +1243,6 @@ public class Expression implements Cloneable
      */
     public void Operators()
     {
-        addLazyBinaryOperator(":", precedence.get("attribute~:"),true, (c, t, container_lv, key_lv) ->
-        {
-            Value container = container_lv.evalValue(c);
-            if (container instanceof LContainerValue)
-            {
-                ContainerValueInterface outerContainer = ((LContainerValue) container).getContainer();
-                Value outerAddress = ((LContainerValue) container).getAddress();
-                Value innerContainer = outerContainer.get(outerAddress);
-                if (!(innerContainer instanceof  ContainerValueInterface))
-                    throw new InternalExpressionException("Cannot access elements of a non-container");
-                Value innerLValue = new LContainerValue((ContainerValueInterface) innerContainer, key_lv.evalValue(c));
-                return (cc, tt) -> innerLValue;
-            }
-            if (!(container instanceof ContainerValueInterface))
-                throw new InternalExpressionException("Cannot access elements of a non-container");
-            Value address = key_lv.evalValue(c);
-            if (t != Context.LVALUE)
-            {
-                Value retVal = ((ContainerValueInterface) container).get(address);
-                return (cc, ct) -> retVal;
-            }
-            return (cc, ct) -> new LContainerValue((ContainerValueInterface) container, address);
-        });
         addBinaryOperator("+", precedence.get("addition+-"), true, Value::add);
         addBinaryOperator("-", precedence.get("addition+-"), true, Value::subtract);
         addBinaryOperator("*", precedence.get("multiplication*/%"), true, Value::multiply);
@@ -1329,8 +1306,12 @@ public class Expression implements Cloneable
             }
             if (v1 instanceof LContainerValue)
             {
-                Value ret = ((LContainerValue) v1).getContainer().put(((LContainerValue) v1).getAddress(), v2);
+                ContainerValueInterface container = ((LContainerValue) v1).getContainer();
-                return (cc, tt) -> ret;
+                if (container == null)
+                    return (cc, tt) -> Value.NULL;
+                Value address = ((LContainerValue) v1).getAddress();
+                if (!(container.put(address, v2))) return (cc, tt) -> Value.NULL;
+                return (cc, tt) -> v2;
             }
             v1.assertAssignable();
             String varname = v1.getVariable();
@@ -2341,13 +2322,13 @@ public class Expression implements Cloneable
         addUnaryFunction("keys", v -> {
             if (v instanceof MapValue)
                 return new ListValue(((MapValue) v).getMap().keySet());
-            throw new InternalExpressionException("keys applies only to maps");
+            return Value.NULL;
         });
 
         addUnaryFunction("values", v -> {
             if (v instanceof MapValue)
                 return new ListValue(((MapValue) v).getMap().values());
-            throw new InternalExpressionException("keys applies only to maps");
+            return Value.NULL;
         });
 
         addUnaryFunction("pairs", v -> {
@@ -2355,21 +2336,93 @@ public class Expression implements Cloneable
                 return ListValue.wrap(((MapValue) v).getMap().entrySet().stream().map(
                         (p) -> ListValue.of(p.getKey(), p.getValue())
                 ).collect(Collectors.toList()));
-            throw new InternalExpressionException("pairs applies only to maps");
+            return Value.NULL;
         });
 
-        addBinaryFunction("get", (container, key) ->
+        addLazyBinaryOperator(":", precedence.get("attribute~:"),true, (c, t, container_lv, key_lv) ->
         {
-            if (container instanceof ContainerValueInterface)
+            Value container = container_lv.evalValue(c);
-                return ((ContainerValueInterface) container).get(key);
+            if (container instanceof LContainerValue)
-            throw new InternalExpressionException("Cannot access elements of a non-container");
+            {
+                ContainerValueInterface outerContainer = ((LContainerValue) container).getContainer();
+                if (outerContainer == null)
+                {
+                    Value innerLValue = new LContainerValue(null, null);
+                    return (cc, tt) -> innerLValue;
+                }
+                Value innerContainer = outerContainer.get(((LContainerValue) container).getAddress());
+                if (!(innerContainer instanceof  ContainerValueInterface))
+                {
+                    Value innerLValue = new LContainerValue(null, null);
+                    return (cc, tt) -> innerLValue;
+                }
+                Value innerLValue = new LContainerValue((ContainerValueInterface) innerContainer, key_lv.evalValue(c));
+                return (cc, tt) -> innerLValue;
+            }
+            if (!(container instanceof ContainerValueInterface))
+                return (cc, tt) -> Value.NULL;
+            Value address = key_lv.evalValue(c);
+            if (t != Context.LVALUE)
+            {
+                Value retVal = ((ContainerValueInterface) container).get(address);
+                return (cc, ct) -> retVal;
+            }
+            Value retVal = new LContainerValue((ContainerValueInterface) container, address);
+            return (cc, ct) -> retVal;
         });
 
-        addBinaryFunction("has", (container, key) ->
+        addLazyFunction("get", -1, (c, t, lv) ->
         {
-            if (container instanceof ContainerValueInterface)
+            if (lv.size() == 0)
-                return new NumericValue(((ContainerValueInterface) container).has(key));
+                throw new InternalExpressionException("get requires parameters");
-            throw new InternalExpressionException("Cannot access elements of a non-container");
+            if (lv.size() == 1)
+            {
+                Value v = lv.get(0).evalValue(c, Context.LVALUE);
+                if (!(v  instanceof LContainerValue))
+                    return (cc, tt) -> Value.NULL;
+                ContainerValueInterface container = ((LContainerValue) v).getContainer();
+                if (container == null)
+                    return (cc, tt) -> Value.NULL;
+                Value ret = container.get(((LContainerValue) v).getAddress());
+                return (cc, tt) -> ret;
+            }
+            Value container = lv.get(0).evalValue(c);
+            for (int i = 1; i < lv.size(); i++)
+            {
+                if (!(container instanceof ContainerValueInterface)) return (cc, tt) -> Value.NULL;
+                container = ((ContainerValueInterface) container).get(lv.get(i).evalValue(c));
+            }
+            if (container == null)
+                return (cc, tt) -> Value.NULL;
+            Value finalContainer = container;
+            return (cc, tt) -> finalContainer;
+        });
+
+        addLazyFunction("has", -1, (c, t, lv) ->
+        {
+            if (lv.size() == 0)
+                throw new InternalExpressionException("get requires parameters");
+            if (lv.size() == 1)
+            {
+                Value v = lv.get(0).evalValue(c, Context.LVALUE);
+                if (!(v  instanceof LContainerValue))
+                    return (cc, tt) -> Value.NULL;
+                ContainerValueInterface container = ((LContainerValue) v).getContainer();
+                if (container == null)
+                    return (cc, tt) -> Value.NULL;
+                Value ret = new NumericValue(container.has(((LContainerValue) v).getAddress()));
+                return (cc, tt) -> ret;
+            }
+            Value container = lv.get(0).evalValue(c);
+            for (int i = 1; i < lv.size()-1; i++)
+            {
+                if (!(container instanceof ContainerValueInterface)) return (cc, tt) -> Value.NULL;
+                container = ((ContainerValueInterface) container).get(lv.get(i).evalValue(c));
+            }
+            if (!(container instanceof ContainerValueInterface))
+                return (cc, tt) -> Value.NULL;
+            Value ret = new NumericValue(((ContainerValueInterface) container).has(lv.get(lv.size()-1).evalValue(c)));
+            return (cc, tt) -> ret;
         });
 
         //Deprecated, use "get" instead, or . operator
@@ -2386,53 +2439,68 @@ public class Expression implements Cloneable
             return items.get((int)index);
         });
 
-        addFunction("put", (lv) ->
+        addLazyFunction("put", -1, (c, t, lv) ->
         {
+            if(lv.size()<2)
+            {
+                throw new InternalExpressionException("put takes at least three arguments, a container, address, and values to insert at that index");
+            }
+            Value container = lv.get(0).evalValue(c, Context.LVALUE);
+            if (container instanceof LContainerValue)
+            {
+                ContainerValueInterface internalContainer = ((LContainerValue) container).getContainer();
+                if (internalContainer == null)
+                {
+                    return (cc, tt) -> Value.NULL;
+                }
+                Value address = ((LContainerValue) container).getAddress();
+                Value what = lv.get(1).evalValue(c);
+                Value retVal = new NumericValue( (lv.size() > 2)
+                        ? internalContainer.put(address, what, lv.get(3).evalValue(c))
+                        : internalContainer.put(address, what));
+                return (cc, tt) -> retVal;
+
+            }
             if(lv.size()<3)
             {
                 throw new InternalExpressionException("put takes at least three arguments, a container, address, and values to insert at that index");
             }
-            Value container = lv.get(0);
             if (!(container instanceof ContainerValueInterface))
             {
-                throw new InternalExpressionException("First argument of put should be a container: list, map or nbt");
+                return (cc, tt) -> Value.NULL;
-            }
-            Value where = lv.get(1);
-            Value what = lv.get(2);
-            if (lv.size()>3)
-            {
-                return ((ContainerValueInterface) container).put(where, what, lv.get(3));
-            }
-            else
-            {
-                return ((ContainerValueInterface) container).put(where, what);
             }
+            Value where = lv.get(1).evalValue(c);
+            Value what = lv.get(2).evalValue(c);
+            Value retVal = new NumericValue( (lv.size()>3)
+                    ? ((ContainerValueInterface) container).put(where, what, lv.get(3).evalValue(c))
+                    : ((ContainerValueInterface) container).put(where, what));
+            return (cc, tt) -> retVal;
         });
 
         addLazyFunction("delete", -1, (c, t, lv) ->
         {
-            if (lv.size() < 1)
+            if (lv.size() == 0)
-                throw new InternalExpressionException("delete requires arguments");
+                throw new InternalExpressionException("delete requires parameters");
-            Value containerOrLValue = lv.get(0).evalValue(c, Context.LVALUE);
+            if (lv.size() == 1)
-            ContainerValueInterface container;
-            Value address;
-            if (containerOrLValue instanceof LContainerValue)
             {
-                container = ((LContainerValue) containerOrLValue).getContainer();
+                Value v = lv.get(0).evalValue(c, Context.LVALUE);
-                address = ((LContainerValue) containerOrLValue).getAddress();
+                if (!(v  instanceof LContainerValue))
+                    return (cc, tt) -> Value.NULL;
+                ContainerValueInterface container = ((LContainerValue) v).getContainer();
+                if (container == null)
+                    return (cc, tt) -> Value.NULL;
+                Value ret = new NumericValue(container.delete(((LContainerValue) v).getAddress()));
+                return (cc, tt) -> ret;
             }
-            else if (containerOrLValue instanceof ContainerValueInterface)
+            Value container = lv.get(0).evalValue(c);
+            for (int i = 1; i < lv.size()-1; i++)
             {
-                container = (ContainerValueInterface) containerOrLValue;
+                if (!(container instanceof ContainerValueInterface)) return (cc, tt) -> Value.NULL;
-                if (lv.size() < 2)
+                container = ((ContainerValueInterface) container).get(lv.get(i).evalValue(c));
-                    throw new InternalExpressionException("delete requires two arguments, container (list, map) and address");
-                address = lv.get(1).evalValue(c);
             }
-            else
+            if (!(container instanceof ContainerValueInterface))
-            {
+                return (cc, tt) -> Value.NULL;
-                throw new InternalExpressionException("First argument of delete should be a container: list, map or nbt");
+            Value ret = new NumericValue(((ContainerValueInterface) container).delete(lv.get(lv.size()-1).evalValue(c)));
-            }
-            Value ret = container.delete(address);
             return (cc, tt) -> ret;
         });
     }

src/main/java/carpet/script/ScriptHost.java

@@ -9,6 +9,7 @@ import carpet.script.value.NumericValue;
 import carpet.script.value.StringValue;
 import carpet.script.value.Value;
 import com.mojang.brigadier.exceptions.CommandSyntaxException;
+import net.minecraft.nbt.Tag;
 import net.minecraft.server.command.ServerCommandSource;
 import net.minecraft.server.network.ServerPlayerEntity;
 import net.minecraft.util.math.BlockPos;
@@ -25,18 +26,25 @@ import static java.lang.Math.max;
 
 public class ScriptHost
 {
-    public final Map<String, ScriptHost> userHosts = new HashMap<>();
+    private final Map<String, ScriptHost> userHosts = new HashMap<>();
     public Map<String, UserDefinedFunction> globalFunctions = new HashMap<>();
     public Map<String, LazyValue> globalVariables = new HashMap<>();
 
+    private Tag globalState;
+    private int saveTimeout;
+
+    private ScriptHost parent;
+
     private String name;
     public String getName() {return name;}
 
     private final ModuleInterface myCode;
-    public final boolean perUser;
+    private final boolean perUser;
 
-    ScriptHost(String name, ModuleInterface code, boolean perUser)
+    ScriptHost(String name, ModuleInterface code, boolean perUser, ScriptHost parent)
     {
+        this.saveTimeout = 0;
+        this.parent = parent;
         this.name = name;
         this.myCode = code;
         this.perUser = perUser;
@@ -50,16 +58,19 @@ public class ScriptHost
         globalVariables.put("_", (c, t) -> Value.ZERO);
         globalVariables.put("_i", (c, t) -> Value.ZERO);
         globalVariables.put("_a", (c, t) -> Value.ZERO);
+
+        if (parent == null && name != null)
+            globalState = loadState();
     }
 
-    public ScriptHost retrieveForExecution(String /*Nullable*/ user)
+    private ScriptHost retrieveForExecution(String /*Nullable*/ user)
     {
         if (!perUser)
             return this;
         ScriptHost userHost = userHosts.get(user);
         if (userHost != null)
             return userHost;
-        userHost = new ScriptHost(this.name, myCode, false);
+        userHost = new ScriptHost(this.name, myCode, false, this);
         userHost.globalVariables.putAll(this.globalVariables);
         userHost.globalFunctions.putAll(this.globalFunctions);
         userHosts.put(user, userHost);
@@ -221,5 +232,60 @@ public class ScriptHost
         }
     }
 
+    private void dumpState()
+    {
+        myCode.saveData(null, globalState);
+    }
 
+    private Tag loadState()
+    {
+        return myCode.getData(null);
+    }
+
+    public Tag getGlobalState(String file)
+    {
+        if (name == null || myCode.isInternal()) return null;
+        if (file != null)
+            myCode.getData(file);
+        if (parent == null)
+            return globalState;
+        return parent.globalState;
+    }
+
+    public void setGlobalState(Tag tag, String file)
+    {
+        if (name == null || myCode.isInternal()) return;
+
+        if (file!= null)
+        {
+            myCode.saveData(file, tag);
+            return;
+        }
+
+        ScriptHost responsibleHost = (parent != null)?parent:this;
+        responsibleHost.globalState = tag;
+        if (responsibleHost.saveTimeout == 0)
+        {
+            responsibleHost.dumpState();
+            responsibleHost.saveTimeout = 200;
+        }
+    }
+
+    public void tick()
+    {
+        if (this.saveTimeout > 0)
+        {
+            this.saveTimeout --;
+            if (this.saveTimeout == 0)
+            {
+                dumpState();
+            }
+        }
+    }
+
+    public void onClose()
+    {
+        if (this.saveTimeout > 0)
+            dumpState();
+    }
 }

src/main/java/carpet/script/bundled/FileModule.java

@@ -1,9 +1,15 @@
 package carpet.script.bundled;
 
 import carpet.CarpetServer;
+import net.minecraft.nbt.EndTag;
+import net.minecraft.nbt.PositionTracker;
+import net.minecraft.nbt.Tag;
 
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
 import java.io.File;
-import java.io.FileReader;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.util.Locale;
@@ -42,4 +48,76 @@ public class FileModule implements ModuleInterface
     {
         return code;
     }
+
+    @Override
+    public boolean isInternal()
+    {
+        return false;
+    }
+
+    //copied private method from net.minecraft.nbt.NbtIo.read()
+    private static Tag read(File file)
+    {
+        try (DataInputStream dataInput_1 = new DataInputStream(new FileInputStream(file)))
+        {
+            byte byte_1 = dataInput_1.readByte();
+            if (byte_1 == 0)
+            {
+                return new EndTag();
+            }
+            else
+            {
+                dataInput_1.readUTF();
+                Tag tag_1 = Tag.createTag(byte_1);
+                tag_1.read(dataInput_1, 0, PositionTracker.DEFAULT);
+                return tag_1;
+            }
+        }
+        catch (IOException ignored)
+        {
+        }
+        return null;
+    }
+
+    //copied private method from net.minecraft.nbt.NbtIo.write() and client method safe_write
+    private static void write(Tag tag_1, File file)
+    {
+        File file_2 = new File(file.getAbsolutePath() + "_tmp");
+        if (file_2.exists()) file_2.delete();
+
+        try(DataOutputStream dataOutputStream_1 = new DataOutputStream(new FileOutputStream(file_2)))
+        {
+            dataOutputStream_1.writeByte(tag_1.getType());
+            if (tag_1.getType() != 0)
+            {
+                dataOutputStream_1.writeUTF("");
+                tag_1.write(dataOutputStream_1);
+            }
+        }
+        catch (IOException e)
+        {
+            return;
+        }
+        if (file.exists()) file.delete();
+        if (!file.exists()) file_2.renameTo(file);
+    }
+
+    @Override
+    public Tag getData(String file)
+    {
+        File dataFile =CarpetServer.minecraft_server.getLevelStorage().resolveFile(
+                CarpetServer.minecraft_server.getLevelName(), "scripts/"+name+".data"+(file==null?"":"."+file)+".nbt");
+        if (!(dataFile.isFile()))
+            return null;
+        return read(dataFile);
+    }
+
+    @Override
+    public void saveData(String file, Tag globalState)
+    {
+        File dataFile =CarpetServer.minecraft_server.getLevelStorage().resolveFile(
+                CarpetServer.minecraft_server.getLevelName(), "scripts/"+name+".data"+(file==null?"":"."+file)+".nbt");
+        write(globalState, dataFile);
+
+    }
 }

src/main/java/carpet/script/bundled/ModuleInterface.java

@@ -1,7 +1,12 @@
 package carpet.script.bundled;
 
+import net.minecraft.nbt.Tag;
+
 public interface ModuleInterface
 {
     String getName();
     String getCode();
+    default boolean isInternal() {return true; }
+    default Tag getData(String file) {return null; }
+    default void saveData(String file, Tag globalState) {}
 }

src/main/java/carpet/script/value/ContainerValueInterface.java

@@ -2,12 +2,12 @@ package carpet.script.value;
 
 public interface ContainerValueInterface
 {
-    public Value put(Value where, Value value);
+    boolean put(Value where, Value value);
-    default Value put(Value where, Value value, Value conditions)
+    default boolean put(Value where, Value value, Value conditions)
     {
         return put(where, value);
     }
-    public Value get(Value where);
+    Value get(Value where);
-    public boolean has(Value where);
+    boolean has(Value where);
-    public Value delete(Value where);
+    boolean delete(Value where);
 }

src/main/java/carpet/script/value/ListValue.java

@@ -306,7 +306,7 @@ public class ListValue extends AbstractListValue implements ContainerValueInterf
     }
 
     @Override
-    public Value put(Value where, Value value, Value conditionValue)
+    public boolean put(Value where, Value value, Value conditionValue)
     {
         String condition = conditionValue.getString();
         if (condition.equalsIgnoreCase("insert"))
@@ -319,11 +319,12 @@ public class ListValue extends AbstractListValue implements ContainerValueInterf
     }
 
     @Override
-    public Value put(Value ind, Value value)
+    public boolean put(Value ind, Value value)
     {
         return put(ind, value, true, false);
     }
-    private Value  put(Value ind, Value value, boolean replace, boolean extend)
+
+    private boolean put(Value ind, Value value, boolean replace, boolean extend)
     {
         if (ind == Value.NULL)
         {
@@ -335,12 +336,14 @@ public class ListValue extends AbstractListValue implements ContainerValueInterf
             {
                 items.add(value);
             }
-            return items.get(items.size()-1);
+            return true;
         }
         else
         {
             int numitems = items.size();
-            int index = (int) NumericValue.asNumber(ind).getLong();
+            if (!(ind instanceof NumericValue))
+                return false;
+            int index = (int)((NumericValue) ind).getLong();
             if (index < 0)
             {
                 long range = abs(index) / numitems;
@@ -350,7 +353,8 @@ public class ListValue extends AbstractListValue implements ContainerValueInterf
             if (replace)
             {
                 while (index >= items.size()) items.add(Value.NULL);
-                return items.set(index, value);
+                items.set(index, value);
+                return true;
             }
             while (index > items.size()) items.add(Value.NULL);
 
@@ -359,11 +363,10 @@ public class ListValue extends AbstractListValue implements ContainerValueInterf
                 Iterable<Value> iterable = ((AbstractListValue) value)::iterator;
                 List<Value> appendix = StreamSupport.stream( iterable.spliterator(), false).collect(Collectors.toList());
                 items.addAll(index, appendix );
-                return items.get(index+appendix.size()-1);
+                return true;
             }
             items.add(index, value);
-            return value;
+            return true;
-
         }
     }
 
@@ -386,14 +389,16 @@ public class ListValue extends AbstractListValue implements ContainerValueInterf
     }
 
     @Override
-    public Value delete(Value where)
+    public boolean delete(Value where)
     {
-        long index = NumericValue.asNumber(where).getLong();
+        if (!(where instanceof NumericValue) || items.isEmpty()) return false;
+        long index = ((NumericValue) where).getLong();
         int numitems = items.size();
         long range = abs(index)/numitems;
         index += (range+2)*numitems;
         index = index % numitems;
-        return items.remove((int)index);
+        items.remove((int)index);
+        return true;
     }
 
     @Override

src/main/java/carpet/script/value/MapValue.java

@@ -177,21 +177,17 @@ public class MapValue extends AbstractListValue implements ContainerValueInterfa
     }
 
     @Override
-    public Value delete(Value where)
+    public boolean delete(Value where)
     {
         Value ret = map.remove(where);
-        if (ret == null)
+        return ret != null;
-            return Value.NULL;
-        return ret;
     }
 
     @Override
-    public Value put(Value key, Value value)
+    public boolean put(Value key, Value value)
     {
         Value ret = map.put(key, value);
-        if (ret == null)
+        return ret != null;
-            return Value.NULL;
-        return ret;
     }
 
     @Override

src/main/java/carpet/script/value/NBTSerializableValue.java

@@ -191,7 +191,7 @@ public class NBTSerializableValue extends Value implements ContainerValueInterfa
         return new StringValue(t.asString());
     }
 
-    private Tag getTag()
+    public Tag getTag()
     {
         if (nbtTag == null)
             nbtTag = nbtSupplier.get();
@@ -210,7 +210,7 @@ public class NBTSerializableValue extends Value implements ContainerValueInterfa
     {
         if (o instanceof NBTSerializableValue)
             return getTag().equals(((NBTSerializableValue) o).getTag());
-        return super.equals((Value)o);
+        return super.equals(o);
     }
 
     @Override
@@ -240,13 +240,13 @@ public class NBTSerializableValue extends Value implements ContainerValueInterfa
     }
 
     @Override
-    public Value put(Value where, Value value)
+    public boolean put(Value where, Value value)
     {
         return put(where, value, new StringValue("replace"));
     }
 
     @Override
-    public Value put(Value where, Value value, Value conditions)
+    public boolean put(Value where, Value value, Value conditions)
     {
         /// WIP
 
@@ -272,14 +272,15 @@ public class NBTSerializableValue extends Value implements ContainerValueInterfa
             }
             else
             {
-                return this;
+                return false;
             }
         }
         if (modifiedTag != null)
         {
             replaceTag(modifiedTag);
+            return  true;
         }
-        return this;
+        return false;
     }
 
 
@@ -412,13 +413,13 @@ public class NBTSerializableValue extends Value implements ContainerValueInterfa
     }
 
     @Override
-    public Value delete(Value where)
+    public boolean delete(Value where)
     {
         NbtPathArgumentType.NbtPath path = cachePath(where.getString());
         Tag tag = getTag().copy();
         int removed = path.remove(tag);
         if (removed > 0) replaceTag(tag);
-        return new NumericValue(removed);
+        return removed > 0;
     }
 
     public static class InventoryLocator

src/main/java/carpet/script/value/Value.java

@@ -116,7 +116,7 @@ public abstract class Value implements Comparable<Value>, Cloneable
         return getString().compareTo(o.getString());
     }
 
-    @Override // for hashmap key access
+    @Override // for hashmap key access, and == operator
     public boolean equals(final Object o)
     {
         if (o instanceof Value)
@@ -124,12 +124,6 @@ public abstract class Value implements Comparable<Value>, Cloneable
         return false;
     }
 
-    // for general use
-    public boolean equals(final Value o)
-    {
-        return this.compareTo(o)==0;
-    }
-
     public void assertAssignable()
     {
         if (boundVariable == null || boundVariable.startsWith("_"))