diff --git a/.vscode/launch.json b/.vscode/launch.json index d3917b2..f08481d 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -8,7 +8,7 @@ "name": "Launch localhost", "type": "chrome", "request": "launch", - "url": "http://localhost:8080/", + "url": "http://localhost:8080/show_example.html?drag", "webRoot": "${workspaceFolder}/public" } ] diff --git a/notes/identity_bug.md b/notes/identity_bug.md new file mode 100644 index 0000000..f0e4865 --- /dev/null +++ b/notes/identity_bug.md @@ -0,0 +1,105 @@ +The identity bug. AKA The not-equal bug. + +# The Bug. + +When you change a property of an object in the shared object, it is often (always?) replaced with another object with the same values. + +this seems like a kind of big deal problem... + +```javascript +const s1 = shared.sprites[0]; +const s2 = shared.sprites[0]; + +console.log("match before", s1 === shared.sprites[0], s1 === s2); // true, true +console.log(s1.touch++); +const s3 = shared.sprites[0]; +console.log("match after", s1 === s3, s1 === s2); // false, true +``` + +# Why? + +Changing a property on s1 triggers the following: + +1. on-change observes the change, tells p5.party +2. p5.party tells local deepstream client +3. deepstreem client updates the data locally (and starts async remote broadcast process) +4. deepstream client tells p5.party (local) data has changed +5. p5.party overwrites the shared object's properties with the new values from deepstreem + +At this point, the first item in the array (and maybe every thing in shared!) is a different object than before (with the same values). The values in the objects match, but they are different objects. === returns false. + +# Multiple copies of the data to keep in sync + +client app -reads/writes-> onchange's proxy -wraps-> p5.party's Record.shared -syncsto-> deepstream client data -syncsto-> deepstream server + +# Does changing properties on s1 change s3 also? + +No! + +```javascript +s1.check = random(); +s3.check = random(); +console.log(s1.check === s3.check); // false +``` + +s1 and s2 are references to the same object, both set before any changes to shared data are made. + +s3 is created after the data is changed, and so after the objects are replaced with clones. + +s1 points to the original value, s3 points to a clone + +# Two proxies with two different underlying objects. + +This isn't just two different proxy objects being made of the same data. There are two data objects, and each has a proxy. + +# How does any of this work then? + +```javascript +const s1 = shared.sprites[0]; // get a reference to shared data +s1.animal = "bat"; // mutate shared data +// change is noted and synced +// shared.sprites[0] is replaced by new clone with new data. s1 is now an "old" clone +// s1.animal doesn't get updated, but doesn't NEED to be in the SIMPLEST cases because s1 was updated by the actual assignment before the sync even happened +s1.name = "barry"; // mutate shared data +// change is noted and synced: even though s1 is and old clone, it still sends changes just fine (but won't receive them) +``` + +# Possible solutions? + +maybe `_onServerChangedData(data)` can be changed to very carefully update only specific changes? why don't we get more detailed info to act on from deepstream anyway? some kind of "deep assign?" + +https://github.com/n1ru4l/graphql-live-query/tree/main/packages/json-patch-plus +https://github.com/Two-Screen/symmetry/ +https://github.com/AsyncBanana/microdiff +https://news.ycombinator.com/item?id=29130661 + +# Second Try + +This was the second version, using the symmetry package to generate and apply a patch. to get symmetry to work would take changing its internals to patch-in-place and ultimately it looked like a hand rolled solution would be better. + +```javascript +console.log("this: shared", this.#shared); +const p = createPatch(this.#shared, data); +if (p === "none") return; +if (p === "reset") + log.error( + "Patch is full reset, reset patches not supported", + this.#shared, + data + ); +if (typeof p === "object") applyPatch(this.#shared, p, true); +``` + +# Original + +This was the original code for updating Record.#shared. It wipes out everying in #shared and then copies +everything from data in. This replaced even unchanged things even unchanged things with duplicates. This worked surprisingly well! We built all the initial demos on this. But leads to the idenity bug. + +```javascript +for (const key in this.#shared) { + delete this.#shared[key]; +} +for (const key in data) { + this.#shared[key] = data[key]; +} +``` diff --git a/notes/symmetry_notes.md b/notes/symmetry_notes.md new file mode 100644 index 0000000..04cb1ab --- /dev/null +++ b/notes/symmetry_notes.md @@ -0,0 +1,59 @@ +I considred using the symmetry npm package to patch Record.#shared with incoming data. +After some experimenting, I switched creating a custom function to patch in place. +These notes are thoughts on what changes would have been needed to get symmetry to work for this use case. + +# patch in place + +rather than creating a new object from input and patch, change input by applying patch. + +# document patch format + +explain the format and vocabulary (t, a, o, r, s, p) in the patch format + +# reset patches + +comparing unrelated objects returns "reset" +i'd like it to return a patch that effects the reset + +```javascript +const input = { a: 1 }; +const input = { b: 2 }; +createPatch(input, output); // "reset" +``` + +// adding a common key/value to both objects achieves this + +```javascript +const input = { forcePatch: true, a: 1 }; +const input = { forcePatch: true, b: 2 }; +createPatch(input, output); // {"t":"o","r":["a"],"s":{"b":2}} +``` + +# no-op patches + +comparing two identical objects returns "none" +I'd like it to return a no-op patch + +```javascript +const input = { a: 1 }; +const output = { a: 1 }; +createPatch(input, output); // "none" +``` + +prefer somethig like `{"t": "o"}` that has no r, s, or p + +// started going down the route of adding a forcePatch flag to create Patch +// for this to work it would have to work recursively, so i'd need to +// modify symetry's createPatch (like i did ofr apply patch below) +// before I do that, i should look to see if a "reset" patch would ever actually +// happen in this specific use case +// export function createPatch(left, right, forcePatch = false) { +// if (forcePatch) { +// left.\_forcePatch = true; +// } +// console.log("cp", left, right); +// const p = \_createPatch(left, right); +// return p; +// } + +#shared has a [Symbol.for("Record")] key that points back at the owning record. it is being considerd by symmetery, is that a problem? diff --git a/public/examples/drag/index.js b/public/examples/drag/index.js index d6626c0..db838f7 100644 --- a/public/examples/drag/index.js +++ b/public/examples/drag/index.js @@ -20,14 +20,10 @@ function pointInRect(p, r) { return p.x > r.l && p.x < r.l + r.w && p.y > r.t && p.y < r.t + r.h; } -// function hydrate(data, _class) { -// let o = new window[_class](); -// o = Object.assign(o, data); -// return o; -// } - const my_id = uuidv4(); + let shared; + function preload() { partyConnect( "wss://deepstream-server-1.herokuapp.com", @@ -41,20 +37,18 @@ function setup() { createCanvas(400, 400); noStroke(); - shared.sprites = []; - - shared.sprites.push(initSprite(new Rect(10, 10, 100, 100), "#ffff66")); - shared.sprites.push(initSprite(new Rect(30, 30, 100, 100), "#ff66ff")); - shared.sprites.push(initSprite(new Rect(50, 50, 100, 100), "#66ffff")); - - console.log(shared.sprites); + if (partyIsHost()) { + shared.sprites = []; + shared.sprites.push(initSprite(new Rect(10, 10, 100, 100), "#ffff66")); + shared.sprites.push(initSprite(new Rect(30, 30, 100, 100), "#ff66ff")); + shared.sprites.push(initSprite(new Rect(50, 50, 100, 100), "#66ffff")); + } } function draw() { background("#cc6666"); - - shared.sprites.forEach((s) => stepSprite(s)); - shared.sprites.forEach((s) => drawSprite(s)); + shared.sprites.forEach(stepSprite); + shared.sprites.forEach(drawSprite); } function mousePressed() { @@ -71,6 +65,7 @@ function initSprite(rect = new Rect(), color = "red") { const s = {}; s.rect = rect; s.color = color; + s.touch = 0; return s; } @@ -95,14 +90,11 @@ function drawSprite(s) { function mousePressedSprite(s) { if (pointInRect(new Point(mouseX, mouseY), s.rect)) { - const i = shared.sprites.indexOf(s); - s.inDrag = true; - console.log("changed", shared.sprites.indexOf(s) !== i); - s.owner = my_id; s.dragOffset = new Point(s.rect.l - mouseX, s.rect.t - mouseY); + const i = shared.sprites.indexOf(s); shared.sprites.splice(i, 1); shared.sprites.push(s); return true; diff --git a/src/Record.js b/src/Record.js index b77b243..77a99e4 100644 --- a/src/Record.js +++ b/src/Record.js @@ -105,18 +105,76 @@ export class Record { _onClientChangedData(path, newValue, oldValue) { // on-change alerts us when the value actually changes // so we don't need to test if newValue and oldValue are different - this.#record.set("shared." + path, newValue); } + // called from deepstreem. this is called when the deepstreem data store is + // updated, even if the update was local. If the update is local + // this.#shared === data -> true + // because this.#shared was updated first, triggering this callback + // if the change originated non-locally, than this.#shared does need to be + // updated + _onServerChangedData(data) { - // replace the CONTENTS of this.#shared // don't replace #shared itself as #watchedShared has a reference to it - for (const key in this.#shared) { - delete this.#shared[key]; + // instead patch it to match the incoming data + patchInPlace(this.#shared, data); + } +} + +function getType(value) { + if (value === null) return "null"; + if (typeof value === "object") return "object"; + if (typeof value === "boolean") return "primative"; + if (typeof value === "number") return "primative"; + if (typeof value === "string") return "primative"; + return "unsupported"; +} + +function patchInPlace(_old, _new) { + if (typeof _old !== "object") return log.error("_old is not an object"); + if (typeof _new !== "object") return log.error("_new is not an object"); + + const oldKeys = Object.keys(_old); + const newKeys = Object.keys(_new); + + // remove old keys not in new + for (const key of oldKeys) { + if (!Object.prototype.hasOwnProperty.call(_new, key)) { + log.debug(`remove ${key}`); + delete _old[key]; } - for (const key in data) { - this.#shared[key] = data[key]; + } + + // patch shared object and array keys + for (const key of newKeys) { + if (Object.prototype.hasOwnProperty.call(_old, key)) { + const oldType = getType(_old[key]); + const newType = getType(_new[key]); + if (oldType === "unsupported") { + log.error(`_old[${key}] is unsupported type: ${typeof _new[key]}`); + continue; + } + if (newType === "unsupported") { + log.error(`_new[${key}] is unsupported type: ${typeof _new[key]}`); + continue; + } + if (oldType != "object" || newType != "object") { + if (_old[key] !== _new[key]) { + log.debug(`update ${key}`); + _old[key] = _new[key]; + } + continue; + } + patchInPlace(_old[key], _new[key]); + } + } + + // add new keys not in old + for (const key of newKeys) { + if (!Object.prototype.hasOwnProperty.call(_old, key)) { + log.debug(`add ${key}`); + _old[key] = _new[key]; } } }