Skip to content

Commit

Permalink
notes cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
jbakse committed Jan 12, 2022
1 parent ecfa0ca commit 71fb149
Show file tree
Hide file tree
Showing 6 changed files with 179 additions and 136 deletions.
71 changes: 71 additions & 0 deletions notes/bugs.md
@@ -0,0 +1,71 @@
# Invalid State transition in participant code

This is an old note, might not be an issue anymore. Keeping it around a while to see if it shows back up.

Sometimes getting a Invalid state transition

```
Invalid state transition.
Details: {"transition":13,"state":"DELETING"}
History:
From - to LOADING_OFFLINE via -
From LOADING_OFFLINE to SUBSCRIBING via 23
From SUBSCRIBING to READY via 4
From READY to DELETING via 9
```

Review the code paths for getMyShared and getParticipantShared

Do they create and delete things correctly?
Do they wait FULLY for the records in participants to be ready?
All shared objects SHOULD be loaded and initialized BEFORE setup is called, but i don't think participant array currently is fully ready.

# Shortcut Bug

These are notes from a possibly outdated bug. Should test this to see what happens...

Demo the outdated shorcut problem:

```javascript
const test = {
animals: {
bird: {
legs: 2,
},
cat: {
legs: 4,
},
},
};

console.log("test", test);

const watchedTest = onChange(test, (path, newValue, oldValue) =>
console.log(path, newValue, oldValue)
);

console.log("wt", watchedTest);

const watchedCat = watchedTest.animals.cat;

watchedCat.teeth = "3";

delete test.animals.cat;

console.log("t", test);

watchedCat.teeth = "5";

console.log(JSON.stringify(test));
```

# Unexpected Disconnect

Sometimes a client "unexpectedly leaves" because ds reconnects them (not sure why).
They get removed from the room and then we don't know they are there, but they are still there because they auto reconnect.
possible fixes

- mark them as missing and reconnect them if they reapear.
- can we have a client readd _themselves_ to room on autoreconnect
- don't remove participants on unexpected leave?
- remove them, but after time delay?
10 changes: 0 additions & 10 deletions notes/design.md

This file was deleted.

39 changes: 23 additions & 16 deletions notes/identity_bug.md
@@ -1,6 +1,6 @@
The identity bug. AKA The not-equal bug.
Fixed as of 2021.11.19

# The Bug.
# The identity bug. AKA The not-equal 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.

Expand Down Expand Up @@ -58,8 +58,8 @@ This isn't just two different proxy objects being made of the same data. There a
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
// shared.sprites[0] is replaced by new clone with the new data. s1 is now an "old" clone
// s1.animal doesn't get updates anymore. future assignments to shared.sprites[0] won't show upon s1 though!
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)
```
Expand All @@ -73,9 +73,22 @@ https://github.com/Two-Screen/symmetry/
https://github.com/AsyncBanana/microdiff
https://news.ycombinator.com/item?id=29130661

# Original

This was the original code for updating Record.#shared when the bug above was identified. 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];
}
```

# 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.
This was the second version, using the symmetry package to generate and apply a patch. It showed promise, but to get everything working using symmetry, i had to start chaning its internals. Symmetry has a copy-on-write approach and we need a patch-in-place and ultimately it looked like a hand rolled solution would be better.

```javascript
console.log("this: shared", this.#shared);
Expand All @@ -90,16 +103,10 @@ if (p === "reset")
if (typeof p === "object") applyPatch(this.#shared, p, true);
```

# Original
# Third try

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.
As of 2021.11.19, Record isusing hand rolled object merge and it addresses the bug pretty well.

```javascript
for (const key in this.#shared) {
delete this.#shared[key];
}
for (const key in data) {
this.#shared[key] = data[key];
}
```
# Future

I'm considering replacing the hand rolled one with lodashs merge, on the theory that they are better programmers than me. :)
33 changes: 0 additions & 33 deletions notes/scraps.js
Expand Up @@ -5,36 +5,3 @@ function delay(ms) {
setTimeout(r, ms);
});
}

// demo the outdated shorcut problem

const test = {
animals: {
bird: {
legs: 2,
},
cat: {
legs: 4,
},
},
};

console.log("test", test);

const watchedTest = onChange(test, (path, newValue, oldValue) =>
console.log(path, newValue, oldValue)
);

console.log("wt", watchedTest);

const watchedCat = watchedTest.animals.cat;

watchedCat.teeth = "3";

delete test.animals.cat;

console.log("t", test);

watchedCat.teeth = "5";

console.log(JSON.stringify(test));
8 changes: 5 additions & 3 deletions notes/symmetry_notes.md
@@ -1,6 +1,8 @@
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.
2021.11.19

You can probably delete this note after a while, if no reason to go back to symetry crops up.

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

Expand Down
154 changes: 80 additions & 74 deletions notes/todo.todo
@@ -1,111 +1,117 @@
I wrote a custome merge function in Record, but maybe it would be better to use a library?

[Lodash.merge](https://lodash.com/docs/#merge)
Feature Requests / Enhancements

mergeWidth should be flexible enough to add debug reporting and customize behavior
[Lodash.mergeWidth](https://lodash.com/docs/#mergeWith)
- Room
-- make debug view opt in
-- add ping to debug view
-- add a way to clear all the shared records
more broadly, a full reset for rooms or apps would be good
during development having stale data stick around can be confusing
reloading a js program usually restarts it from 0, but not when you have data hanging out on the back end.
it could be a common pattern to have `setup->if host->reset room->init room`
might even automatically reset the room if its empty (not the persistant worlds question)

- Record

might be worth compairing the merge function to
https://github.com/deepstreamIO/deepstream.io/blob/892c0fea1b348cc5152e3b75cf19e3241ece3edc/src/utils/utils.ts#L77
- Client
- reconnect after page reload
consider cacheing random id in _session_ storage so reloads can reconnect as same client

to see how the approaches differ, if one is better


# NOW
In the participant code...
- Branch - Push Improvements
-- config auto push or manual
I'm not sure if this would be useful at all?
-- config record push debounce
Again, not sure if this would work?
The thought here is it might be good to let code make lots of little changes, but not send them until the work is done.
Especially in a case where the same value might get updated many times in a row. e.g. incrementing the score could happen a lot in a single draw, but we only need to send the final version


Sometimes getting a Invalid state transition
```
Invalid state transition.
Details: {"transition":13,"state":"DELETING"}
History:
From - to LOADING_OFFLINE via -
From LOADING_OFFLINE to SUBSCRIBING via 23
From SUBSCRIBING to READY via 4
From READY to DELETING via 9
```
- Branch - Participants
-- expose participant count
-- expose participant id list
-- expose my id



# Contribution Project Ideas
-- Server Admin Panel. Show connected apps/rooms/guests + stats.
-- Client Admin Panel. Improve
-- Example Games + Apps
-- Video Tutorials
-- Dorkshop
-- Eleventy Docs. Migrate docs to .md/11ty. Visual design.



# API Changes
-- rename? partyGetParticipantShared() -> partyGetMyShared()
-- rename? participants -> guests


# Notes from from code-review
-- Why does the room create a participant record automatically?
- :Could this partyGetMyShared() just be a call like partyGetShared(myID)




# Improvements

-- merging incoming data with shared object
: I wrote a custom merge function in Record, but maybe it would be better to use a library?
[Lodash.merge](https://lodash.com/docs/#merge)
mergeWidth should be flexible enough to add debug reporting and customize behavior
[Lodash.mergeWidth](https://lodash.com/docs/#mergeWith)
might be good to just study their merge and compare to the current party implementation
might be worth also studying the merge function in deepstream
https://github.com/deepstreamIO/deepstream.io/blob/892c0fea1b348cc5152e3b75cf19e3241ece3edc/src/utils/utils.ts#L77
to see how the approaches differ, if one is better

Get you head back in the game and review the code paths for getMyShared and getParticipanShared

Do they create and delete things correctly?
Do they wait FULLY for the records in participants to be ready?
I THINK NO.
All shared objects SHOULD be loaded and initialized BEFORE setup is called, but i don't think participant array currently is fully ready.





# Tooling

- consider providing min and unmin versions
-- dist min and non-min versions?
https://stackoverflow.com/questions/25956937/how-to-build-minified-and-uncompressed-bundle-with-webpack
- compare to other p5 libraries
compare to other p5 libraries

- can bake package version into code somehow?
-- can bake package version into code somehow?
I frequently think it would be nice if dist/lib.js had a comment at the top like /* p5.party v1.2.3 */
When you copy a dep into a project (rather than using npm or something) its easy forget what version it is

.- why is version on npm up to date, only 50k, and missing css? also dist was empty

# Bugs
- you currently need to call getParticipantShared every time you use it to ensure you have an update to date list
- make this work more like shared where i replace contents keeping references pointting the right spot


# Style / Naming

# Enhancements

- Record

- Client
- consider cacheing random id in _session_ storage so reloads can reconnect as same client

- Room
- make debug view opt in
- add ping to debug view?


# Docs
- also, while possibly this same library could be used for long term server persistance, for now limit the scope to session-longevity multiplayer
- merging_is_hard
and party can't do it for you
create conceptual doc on why merging is hard, strategies to avoid conflicts
started in merging_is_hard.md


# Questions
# Design Questions
- Persistant Worlds?
deeptstream stores state locally in process memory, and can be connected to data store
currently it does not connect to a data store, and data will be lost on server/process restart
theoretically, this library could be used for prototyping persitent worlds right now, and with a data store would be even better, but the as a design decision we limit the scope to single-session-multiplayer for now


# Tips

# Examples

# Requests
- add a way to clear all the shared records in a room

# Branch - Subscribe

--- get shared object changed messages

# Branch - Push Improvements

- config auto push or not on records
- config record push debounce

# Branch - Participants

--- expose participant count
--- expose participant list
--- expose a record of info for each participant

# Branch - Info
- using vue?
- debug view?
- room view?
- show room records
- show room participants
- dashboard view?
- show apps, rooms, records, participants

# Branch - Deep Update
- check this out, in order to make record support sharing any property of the record (not just .shared)
-- Doesn't work because of the `shortcut reference` problem
- 1) watch the whole record instead of shared
- 2) return the part of the whole record you want to expose, or that is requested
- 3) done!
- ?Do a deep nested update of .shared (or whole record) so that shortcut references will still work?
-- would solve the `shortcut reference` problem

0 comments on commit 71fb149

Please sign in to comment.