Skip to content

Commit e530f87

Browse files
committed
fixed all known collaboration bugs
1 parent 9a5fdff commit e530f87

3 files changed

Lines changed: 199 additions & 49 deletions

File tree

src/addons/addons/collaboration/helpers/costumeArtSync.js

Lines changed: 100 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import * as constants from './constants.js';
33
import * as helper from './helper.js';
44
import * as recorder from './recorder.js';
55
import {compose} from 'scratch-paint/src/helper/collab-art.js';
6-
import {applyLive, canApplyLive, replayBitmap, canReplayBitmap, setLiveResync} from 'scratch-paint/src/helper/collab-live.js';
6+
import {applyLive, canApplyLive, replayBitmap, canReplayBitmap, setLiveResync, isRestoringHistory} from 'scratch-paint/src/helper/collab-live.js';
77
import {bitmapDigest, bitmapSnapshot} from 'scratch-paint/src/helper/bit-replay.js';
88

99
const MINT_IDLE_MS = 1000;
@@ -12,6 +12,17 @@ const mintTimers = new Map();
1212

1313
const published = new Map();
1414

15+
const owned = new Map();
16+
17+
function own(key, id) {
18+
let ids = owned.get(key);
19+
if (!ids) {
20+
ids = new Set();
21+
owned.set(key, ids);
22+
}
23+
ids.add(id);
24+
}
25+
1526
let floatingCostume = null;
1627

1728
let recordedFloatId = null;
@@ -73,6 +84,12 @@ export function isDocBacked(costumeId) {
7384

7485
function assignIds(costumeId, shapes, items, seeding) {
7586
const clientId = constants.mutableRefs.ydoc ? constants.mutableRefs.ydoc.clientID : 'x';
87+
const claimed = new Set();
88+
for (const shape of shapes) {
89+
if (!shape.id) continue;
90+
if (claimed.has(shape.id)) shape.id = null;
91+
else claimed.add(shape.id);
92+
}
7693

7794
const existing = artFor(costumeId, false);
7895
if (existing && !seeding) {
@@ -87,17 +104,29 @@ function assignIds(costumeId, shapes, items, seeding) {
87104
}
88105
for (const shape of shapes) {
89106
if (shape.id) continue;
90-
const candidates = byContent.get(contentHash(signature(shape)));
91-
if (candidates && candidates.length) shape.id = candidates.shift();
107+
const candidates = byContent.get(contentHash(signature(shape))) || [];
108+
while (candidates.length) {
109+
const candidate = candidates.shift();
110+
if (claimed.has(candidate)) continue;
111+
shape.id = candidate;
112+
claimed.add(candidate);
113+
break;
114+
}
92115
}
93116
}
94117

118+
const seeded = new Map();
95119
for (let index = 0; index < shapes.length; index++) {
96120
if (!shapes[index].id) {
97-
98-
shapes[index].id = seeding ?
99-
`${costumeId}~${contentHash(signature(shapes[index]))}` :
100-
`${clientId}-${Math.random().toString(36).slice(2, 9)}`;
121+
if (seeding) {
122+
const hash = contentHash(signature(shapes[index]));
123+
const nth = (seeded.get(hash) || 0) + 1;
124+
seeded.set(hash, nth);
125+
shapes[index].id = nth === 1 ? `${costumeId}~${hash}` : `${costumeId}~${hash}~${nth}`;
126+
} else {
127+
shapes[index].id = `${clientId}-${Math.random().toString(36).slice(2, 9)}`;
128+
}
129+
claimed.add(shapes[index].id);
101130
}
102131
const item = items && items[index];
103132
if (item) {
@@ -192,23 +221,30 @@ export function publishLocalArt(art) {
192221
const after = new Map();
193222
for (const shape of art.shapes) after.set(shape.id, signature(shape));
194223

224+
const restoring = isRestoringHistory();
225+
const mine = owned.get(key) || new Set();
226+
const touchable = id => !restoring || mine.has(id) || !before.has(id);
227+
195228
constants.mutableRefs.ydoc.transact(() => {
196229
const store = artFor(costume.id, true);
197230
if (!store) return;
198231

199232
for (const shape of art.shapes) {
200233
if (before.get(shape.id) === after.get(shape.id)) continue;
234+
if (!touchable(shape.id)) continue;
201235
const entry = new Y.Map();
202236
entry.set('tag', shape.tag);
203237
entry.set('attrs', shape.attrs);
204238
if (shape.kids) entry.set('kids', shape.kids);
205239
if (shape.text !== undefined) entry.set('text', shape.text);
206240
if (shape.defs) entry.set('defs', shape.defs);
207241
store.shapes.set(shape.id, entry);
242+
own(key, shape.id);
208243
}
209244

210245
for (const id of before.keys()) {
211-
if (!after.has(id)) store.shapes.delete(id);
246+
if (after.has(id) || !touchable(id)) continue;
247+
store.shapes.delete(id);
212248
}
213249

214250
const wantOrder = art.shapes.map(shape => shape.id);
@@ -219,7 +255,7 @@ export function publishLocalArt(art) {
219255

220256
for (let index = haveOrder.length - 1; index >= 0; index--) {
221257
const id = haveOrder[index];
222-
if (!wantSet.has(id) && before.has(id)) store.order.delete(index, 1);
258+
if (!wantSet.has(id) && before.has(id) && touchable(id)) store.order.delete(index, 1);
223259
}
224260
for (let index = 0; index < wantOrder.length; index++) {
225261
const id = wantOrder[index];
@@ -235,14 +271,27 @@ export function publishLocalArt(art) {
235271
const mutualThere = wantOrder.filter(id => haveSet.has(id));
236272
if (mutualHere.length === mutualThere.length &&
237273
mutualHere.some((id, index) => id !== mutualThere[index])) {
238-
239-
const rebuilt = wantOrder.slice();
240-
settled.forEach((id, index) => {
241-
if (wantSet.has(id)) return;
242-
rebuilt.splice(Math.min(index, rebuilt.length), 0, id);
243-
});
244-
store.order.delete(0, store.order.length);
245-
store.order.insert(0, rebuilt);
274+
const wantAt = new Map(mutualThere.map((id, index) => [id, index]));
275+
const stays = new Set(longestOrdered(mutualHere.map(id => wantAt.get(id)))
276+
.map(index => mutualHere[index]));
277+
const current = store.order.toArray();
278+
for (let index = current.length - 1; index >= 0; index--) {
279+
const id = current[index];
280+
if (wantAt.has(id) && !stays.has(id)) store.order.delete(index, 1);
281+
}
282+
for (const id of mutualThere) {
283+
if (stays.has(id)) continue;
284+
const now = store.order.toArray();
285+
let at = 0;
286+
for (let index = wantAt.get(id) - 1; index >= 0; index--) {
287+
const position = now.indexOf(mutualThere[index]);
288+
if (position !== -1) {
289+
at = position + 1;
290+
break;
291+
}
292+
}
293+
store.order.insert(at, [id]);
294+
}
246295
}
247296

248297
for (const [name, value] of Object.entries(art.view)) {
@@ -252,7 +301,13 @@ export function publishLocalArt(art) {
252301
if (Number.isFinite(art.rotationCenterY)) store.view.set('rotationCenterY', art.rotationCenterY);
253302
}, constants.LOCAL_EVENT_SYNC_ORIGIN);
254303

255-
published.set(key, after);
304+
if (restoring) {
305+
const kept = new Map(after);
306+
for (const [id, sig] of before) if (!touchable(id)) kept.set(id, sig);
307+
published.set(key, kept);
308+
} else {
309+
published.set(key, after);
310+
}
256311
scheduleMint(art.targetId, costume.id);
257312

258313
if (recorder.isRecording()) {
@@ -270,6 +325,24 @@ export function publishLocalArt(art) {
270325
}
271326
}
272327

328+
function longestOrdered(values) {
329+
const best = values.map(() => 1);
330+
const previous = values.map(() => -1);
331+
let end = 0;
332+
for (let index = 0; index < values.length; index++) {
333+
for (let earlier = 0; earlier < index; earlier++) {
334+
if (values[earlier] < values[index] && best[earlier] + 1 > best[index]) {
335+
best[index] = best[earlier] + 1;
336+
previous[index] = earlier;
337+
}
338+
}
339+
if (best[index] > best[end]) end = index;
340+
}
341+
const out = [];
342+
for (let index = values.length ? end : -1; index !== -1; index = previous[index]) out.push(index);
343+
return out.reverse();
344+
}
345+
273346
function scheduleMint(targetId, costumeId) {
274347
const key = artKey(targetId, costumeId);
275348
clearTimeout(mintTimers.get(key));
@@ -339,11 +412,17 @@ export function composeFromDoc(costumeId) {
339412
return {
340413
svg: compose(view, shapes),
341414
ids: shapes.map(shape => shape.id),
415+
baseline: new Map(shapes.map(shape => [shape.id, signature(shape)])),
342416
rotationCenterX: view.rotationCenterX,
343417
rotationCenterY: view.rotationCenterY
344418
};
345419
}
346420

421+
export function noteStamped(token) {
422+
if (!token || !token.key || !token.baseline) return;
423+
published.set(token.key, token.baseline);
424+
}
425+
347426
function isOpenInEditor(target, index) {
348427
if (!canApplyLive()) return false;
349428
const state = constants.mutableRefs.addon?.tab?.redux?.state;
@@ -435,7 +514,8 @@ export function applyRemoteArt(costumeId) {
435514

436515
const index = target ? target.getCostumes().indexOf(costume) : -1;
437516
if (index >= 0 && isOpenInEditor(target, index)) {
438-
applyLive(rebuilt.svg, centerX, centerY, rebuilt.ids);
517+
applyLive(rebuilt.svg, centerX, centerY, rebuilt.ids,
518+
{key: artKey(target.id, costumeId), baseline: rebuilt.baseline});
439519
live = true;
440520
}
441521
} catch (e) {
@@ -814,6 +894,7 @@ export function reset() {
814894
for (const timer of pendingApply.values()) clearTimeout(timer);
815895
pendingApply.clear();
816896
published.clear();
897+
owned.clear();
817898
replayedOps.clear();
818899
for (const timer of digestTimers.values()) clearTimeout(timer);
819900
digestTimers.clear();

src/addons/addons/collaboration/userscript.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import * as collabSnapshot from '../../../lib/collab-snapshot.js';
1818
import {isCursorChatEnabled} from '../../../lib/ct-url-flags.js';
1919
import {isExtensionAllowed, reportRefusedExtension} from '../../../lib/ct-extension-restrictions.js';
2020
import {onArtChanged} from '../../../lib/collab-art-bus.js';
21-
import {setShapeReporting, setLiveResync, setDragReporter} from 'scratch-paint/src/helper/collab-live.js';
21+
import {setShapeReporting, setLiveResync, setDragReporter, setStampReporter} from 'scratch-paint/src/helper/collab-live.js';
2222
import {syncRemoteFloats} from 'scratch-paint/src/helper/bit-replay.js';
2323
import {syncRemoteGhosts} from 'scratch-paint/src/helper/vector-ghost.js';
2424
import {setCursorReporter, setRemoteCursors} from 'scratch-paint/src/helper/collab-cursors.js';
@@ -490,6 +490,7 @@ function attachYjsProvider() {
490490
unsubscribeArt = onArtChanged(costumeArtSync.publishFromEditor);
491491
setShapeReporting(true);
492492
setLiveResync(costumeArtSync.resyncOpenCostume);
493+
setStampReporter(costumeArtSync.noteStamped);
493494

494495
setCursorReporter(point => {
495496
const awareness = constants.mutableRefs.yjsAwarenessInstance;
@@ -1413,6 +1414,7 @@ function attachYjsProvider() {
14131414
constants.mutableRefs.redrawCostumeArt = null;
14141415
setShapeReporting(false);
14151416
setLiveResync(null);
1417+
setStampReporter(null);
14161418
setDragReporter(null);
14171419
syncRemoteFloats([]);
14181420
syncRemoteGhosts([]);

0 commit comments

Comments
 (0)