Skip to content

Commit

Permalink
fix identity bug, add drag example
Browse files Browse the repository at this point in the history
  • Loading branch information
jbakse committed Nov 17, 2021
1 parent adf64ff commit 345ffec
Show file tree
Hide file tree
Showing 5 changed files with 241 additions and 27 deletions.
2 changes: 1 addition & 1 deletion .vscode/launch.json
Expand Up @@ -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"
}
]
Expand Down
105 changes: 105 additions & 0 deletions 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];
}
```
59 changes: 59 additions & 0 deletions 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?
32 changes: 12 additions & 20 deletions public/examples/drag/index.js
Expand Up @@ -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",
Expand All @@ -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() {
Expand All @@ -71,6 +65,7 @@ function initSprite(rect = new Rect(), color = "red") {
const s = {};
s.rect = rect;
s.color = color;
s.touch = 0;
return s;
}

Expand All @@ -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;
Expand Down
70 changes: 64 additions & 6 deletions src/Record.js
Expand Up @@ -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];
}
}
}

0 comments on commit 345ffec

Please sign in to comment.