Avoid broadcasting what was just received (#1116)
Fixes #1115 The issue is that replaceAllElements calls a render synchronously, preventing lastBroadcastedOrReceivedSceneVersion from being set correctly. I tried using batchUpdate but it only takes a single argument ( https://github.com/facebook/react/blob/c5d2fc7127654e43de59fff865b74765a103c4a5/packages/react-reconciler/src/ReactFiberWorkLoop.js#L1088 ) whereas the callback takes two. Test Plan: - Add a console.log before `this.broadcastScene("SCENE_UPDATE");` in App.tsx - Connect a bunch of clients - Have one move a shape - Make sure that this client has the console logged - Make sure the other clients don't have it
This commit is contained in:
committed by
GitHub
parent
763735ac84
commit
a7bd21ccf2
+47
-42
@@ -731,58 +731,63 @@ export class App extends React.Component<any, AppState> {
|
||||
);
|
||||
|
||||
// Reconcile
|
||||
globalSceneState.replaceAllElements(
|
||||
restoredState.elements
|
||||
.reduce((elements, element) => {
|
||||
// if the remote element references one that's currently
|
||||
// edited on local, skip it (it'll be added in the next
|
||||
// step)
|
||||
if (
|
||||
element.id === this.state.editingElement?.id ||
|
||||
element.id === this.state.resizingElement?.id ||
|
||||
element.id === this.state.draggingElement?.id
|
||||
) {
|
||||
return elements;
|
||||
}
|
||||
const newElements = restoredState.elements
|
||||
.reduce((elements, element) => {
|
||||
// if the remote element references one that's currently
|
||||
// edited on local, skip it (it'll be added in the next
|
||||
// step)
|
||||
if (
|
||||
element.id === this.state.editingElement?.id ||
|
||||
element.id === this.state.resizingElement?.id ||
|
||||
element.id === this.state.draggingElement?.id
|
||||
) {
|
||||
return elements;
|
||||
}
|
||||
|
||||
if (
|
||||
localElementMap.hasOwnProperty(element.id) &&
|
||||
localElementMap[element.id].version > element.version
|
||||
) {
|
||||
elements.push(localElementMap[element.id]);
|
||||
delete localElementMap[element.id];
|
||||
} else if (
|
||||
localElementMap.hasOwnProperty(element.id) &&
|
||||
localElementMap[element.id].version === element.version &&
|
||||
localElementMap[element.id].versionNonce !==
|
||||
element.versionNonce
|
||||
) {
|
||||
// resolve conflicting edits deterministically by taking the one with the lowest versionNonce
|
||||
if (
|
||||
localElementMap.hasOwnProperty(element.id) &&
|
||||
localElementMap[element.id].version > element.version
|
||||
localElementMap[element.id].versionNonce <
|
||||
element.versionNonce
|
||||
) {
|
||||
elements.push(localElementMap[element.id]);
|
||||
delete localElementMap[element.id];
|
||||
} else if (
|
||||
localElementMap.hasOwnProperty(element.id) &&
|
||||
localElementMap[element.id].version === element.version &&
|
||||
localElementMap[element.id].versionNonce !==
|
||||
element.versionNonce
|
||||
) {
|
||||
// resolve conflicting edits deterministically by taking the one with the lowest versionNonce
|
||||
if (
|
||||
localElementMap[element.id].versionNonce <
|
||||
element.versionNonce
|
||||
) {
|
||||
elements.push(localElementMap[element.id]);
|
||||
} else {
|
||||
// it should be highly unlikely that the two versionNonces are the same. if we are
|
||||
// really worried about this, we can replace the versionNonce with the socket id.
|
||||
elements.push(element);
|
||||
}
|
||||
delete localElementMap[element.id];
|
||||
} else {
|
||||
// it should be highly unlikely that the two versionNonces are the same. if we are
|
||||
// really worried about this, we can replace the versionNonce with the socket id.
|
||||
elements.push(element);
|
||||
delete localElementMap[element.id];
|
||||
}
|
||||
delete localElementMap[element.id];
|
||||
} else {
|
||||
elements.push(element);
|
||||
delete localElementMap[element.id];
|
||||
}
|
||||
|
||||
return elements;
|
||||
}, [] as Mutable<typeof restoredState.elements>)
|
||||
// add local elements that weren't deleted or on remote
|
||||
.concat(...Object.values(localElementMap)),
|
||||
return elements;
|
||||
}, [] as Mutable<typeof restoredState.elements>)
|
||||
// add local elements that weren't deleted or on remote
|
||||
.concat(...Object.values(localElementMap));
|
||||
|
||||
// Avoid broadcasting to the rest of the collaborators the scene
|
||||
// we just received!
|
||||
// Note: this needs to be set before replaceAllElements as it
|
||||
// syncronously calls render.
|
||||
this.lastBroadcastedOrReceivedSceneVersion = getDrawingVersion(
|
||||
newElements,
|
||||
);
|
||||
|
||||
globalSceneState.replaceAllElements(newElements);
|
||||
}
|
||||
this.lastBroadcastedOrReceivedSceneVersion = getDrawingVersion(
|
||||
globalSceneState.getAllElements(),
|
||||
);
|
||||
|
||||
// We haven't yet implemented multiplayer undo functionality, so we clear the undo stack
|
||||
// when we receive any messages from another peer. This UX can be pretty rough -- if you
|
||||
|
||||
Reference in New Issue
Block a user