MapSchema: dissallow delete if key doesn't exist

Author: endel

.github/workflows/test-suit.yml

@@ -11,7 +11,7 @@ jobs:
 
     strategy:
       matrix:
-        node-version: [16]
+        node-version: [20]
 
     steps:
       - uses: actions/checkout@v2

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@colyseus/schema",
-  "version": "3.0.61",
+  "version": "3.0.62",
   "description": "Binary state serializer with delta encoding for games",
   "bin": {
     "schema-codegen": "bin/schema-codegen",

package.json

@@ -151,6 +151,10 @@ export class MapSchema<V=any, K extends string = string> implements Map<K, V>, C
     }
 
     delete(key: K) {
+        if (!this.$items.has(key)) {
+            return false;
+        }
+
         const index = this[$changes].indexes[key];
 
         this.deletedItems[index] = this[$changes].delete(index);

src/types/custom/MapSchema.ts

@@ -1,7 +1,7 @@
 import * as assert from "assert";
 
 import { State, Player, getCallbacks, getEncoder, createInstanceFromReflection, getDecoder, assertDeepStrictEqualEncodeAll, assertRefIdCounts } from "./Schema";
-import { ArraySchema, Schema, type, Reflection, $changes, MapSchema, ChangeTree, schema } from "../src";
+import { ArraySchema, Schema, type, $changes, MapSchema, ChangeTree, schema } from "../src";
 
 describe("ArraySchema Tests", () => {
 

test/ArraySchema.test.ts

@@ -661,6 +661,25 @@ describe("Type: MapSchema", () => {
         assertDeepStrictEqualEncodeAll(state);
     });
 
+    it("should allow removing the same key twice", () => {
+        class Item extends Schema {
+            @type("number") damage: number;
+        }
+        class State extends Schema {
+            @type({ map: Item }) items = new MapSchema<Item>();
+        }
+
+        const state = new State();
+        state.items.set("one", new Item().assign({ damage: 10 }));
+        state.items.set("two", new Item().assign({ damage: 20 }));
+
+        assertDeepStrictEqualEncodeAll(state);
+
+        state.items.delete("one");
+        state.items.delete("one");
+        assertDeepStrictEqualEncodeAll(state);
+    });
+
     it("should allow to move a key from one map to another", () => {
         class Entity extends Schema {
             @type("number") id: number;

test/MapSchema.test.ts

@@ -1,8 +1,9 @@
+import * as assert from "assert";
+
 import { Schema, type, ArraySchema, MapSchema, Reflection, Iterator, StateView } from "../src";
 import { Decoder } from "../src/decoder/Decoder";
 import { Encoder } from "../src/encoder/Encoder";
 import { CallbackProxy, getDecoderStateCallbacks, SchemaCallbackProxy } from "../src/decoder/strategy/StateCallbacks";
-import assert = require("assert");
 
 // augment Schema to add encode/decode methods
 // (workaround to keep tests working while we don't migrate the tests to the new API)