diff --git a/editor/src/components/canvas/multiplayer-presence.tsx b/editor/src/components/canvas/multiplayer-presence.tsx index 122364f57f54..a18785fd82a5 100644 --- a/editor/src/components/canvas/multiplayer-presence.tsx +++ b/editor/src/components/canvas/multiplayer-presence.tsx @@ -338,7 +338,9 @@ const FollowingOverlay = React.memo(() => { const isFollowTarget = React.useCallback( (other: User): boolean => { - return isFollowMode(mode) && other.id === mode.playerId + return ( + isFollowMode(mode) && other.id === mode.playerId && other.connectionId === mode.connectionId + ) }, [mode], ) diff --git a/editor/src/components/editor/editor-modes.ts b/editor/src/components/editor/editor-modes.ts index 3a7266b5bf62..2b14ebc6182f 100644 --- a/editor/src/components/editor/editor-modes.ts +++ b/editor/src/components/editor/editor-modes.ts @@ -191,6 +191,7 @@ export interface LiveCanvasMode { export interface FollowMode { type: 'follow' playerId: string // the ID of the followed player + connectionId: number // the connection ID of the followed player } export type Mode = @@ -248,10 +249,11 @@ export const EditorModes = { isDragging: isDragging, } }, - followMode: function (playerId: string): FollowMode { + followMode: function (playerId: string, connectionId: number): FollowMode { return { type: 'follow', playerId: playerId, + connectionId: connectionId, } }, } diff --git a/editor/src/components/editor/store/store-deep-equality-instances.ts b/editor/src/components/editor/store/store-deep-equality-instances.ts index 518a421a58d6..4c122576260e 100644 --- a/editor/src/components/editor/store/store-deep-equality-instances.ts +++ b/editor/src/components/editor/store/store-deep-equality-instances.ts @@ -3295,9 +3295,11 @@ export const CommentModeKeepDeepEquality: KeepDeepEqualityCall = co EditorModes.commentMode, ) -export const FollowModeKeepDeepEquality: KeepDeepEqualityCall = combine1EqualityCall( +export const FollowModeKeepDeepEquality: KeepDeepEqualityCall = combine2EqualityCalls( (mode) => mode.playerId, StringKeepDeepEquality, + (mode) => mode.connectionId, + NumberKeepDeepEquality, EditorModes.followMode, ) diff --git a/editor/src/components/user-bar.tsx b/editor/src/components/user-bar.tsx index d9516555a4ac..56cd8fadb4df 100644 --- a/editor/src/components/user-bar.tsx +++ b/editor/src/components/user-bar.tsx @@ -1,19 +1,19 @@ /** @jsxRuntime classic */ /** @jsx jsx */ import React from 'react' -import { css, jsx } from '@emotion/react' +import { jsx } from '@emotion/react' import type { CSSProperties } from 'react' import { useOthers, useStatus, useStorage } from '../../liveblocks.config' import { getUserPicture, isLoggedIn } from '../common/user' import { getCollaborator, useMyUserAndPresence } from '../core/commenting/comment-hooks' -import type { MultiplayerColor } from '../core/shared/multiplayer' +import type { FollowTarget, MultiplayerColor } from '../core/shared/multiplayer' import { canFollowTarget, + followTarget, isDefaultAuth0AvatarURL, multiplayerColorFromIndex, multiplayerInitialsFromName, normalizeMultiplayerName, - normalizeOthersList, } from '../core/shared/multiplayer' import { MultiplayerWrapper } from '../utils/multiplayer-wrapper' import { unless, when } from '../utils/react-conditionals' @@ -133,6 +133,7 @@ const MultiplayerUserBar = React.memo(() => { return { ...getCollaborator(collabs, other), following: other.presence.following, + connectionId: other.connectionId, } }), ) @@ -165,9 +166,13 @@ const MultiplayerUserBar = React.memo(() => { }, [ownerId, myUser]) const toggleFollowing = React.useCallback( - (targetId: string) => () => { + (target: FollowTarget) => () => { let actions: EditorAction[] = [] - const canFollow = canFollowTarget(myUser.id, targetId, others) + const canFollow = canFollowTarget( + followTarget(myUser.id, myPresence.connectionId), + target, + others, + ) if (!canFollow) { actions.push( showToast( @@ -181,14 +186,16 @@ const MultiplayerUserBar = React.memo(() => { ) } else { const newMode = - isFollowMode(mode) && mode.playerId === targetId + isFollowMode(mode) && + mode.playerId === target.playerId && + mode.connectionId === target.connectionId ? EditorModes.selectMode(null, false, 'none') - : EditorModes.followMode(targetId) + : EditorModes.followMode(target.playerId, target.connectionId) actions.push(switchEditorMode(newMode)) } dispatch(actions) }, - [dispatch, mode, myUser, others], + [dispatch, mode, others, myUser, myPresence], ) if (myUser.name == null) { @@ -218,8 +225,8 @@ const MultiplayerUserBar = React.memo(() => { tooltip={{ text: name, colored: true }} color={multiplayerColorFromIndex(other.colorIndex)} picture={other.avatar} - onClick={toggleFollowing(other.id)} - isBeingFollowed={isFollowMode(mode) && mode.playerId === other.id} + onClick={toggleFollowing(followTarget(other.id, other.connectionId))} + isBeingFollowed={isFollowMode(mode) && mode.connectionId === other.connectionId} follower={other.following === myUser.id} isOwner={isOwner} /> diff --git a/editor/src/core/shared/multiplayer.spec.ts b/editor/src/core/shared/multiplayer.spec.ts index 2867c699a49e..82a65724c502 100644 --- a/editor/src/core/shared/multiplayer.spec.ts +++ b/editor/src/core/shared/multiplayer.spec.ts @@ -1,4 +1,4 @@ -import { canFollowTarget, multiplayerInitialsFromName } from './multiplayer' +import { canFollowTarget, followTarget, multiplayerInitialsFromName } from './multiplayer' describe('multiplayer', () => { describe('multiplayerInitialsFromName', () => { @@ -21,34 +21,56 @@ describe('multiplayer', () => { describe('canFollowTarget', () => { it('can follow a single player', () => { - expect(canFollowTarget('foo', 'bar', [{ id: 'bar', following: null }])).toBe(true) - }) - it('can follow a player that follows another player', () => { expect( - canFollowTarget('foo', 'bar', [ - { id: 'bar', following: 'baz' }, - { id: 'baz', following: null }, + canFollowTarget(followTarget('foo', 0), followTarget('bar', 0), [ + { id: 'bar', connectionId: 0, following: null }, ]), ).toBe(true) }) - it('can follow a player that follows another player indirectly', () => { + it('cannot follow self', () => { + expect( + canFollowTarget(followTarget('foo', 0), followTarget('foo', 0), [ + { id: 'foo', connectionId: 1, following: null }, + ]), + ).toBe(false) + }) + it('can follow a single player with the same ID but on another connection', () => { expect( - canFollowTarget('foo', 'bar', [ - { id: 'bar', following: 'baz' }, - { id: 'baz', following: 'qux' }, - { id: 'qux', following: null }, + canFollowTarget(followTarget('foo', 0), followTarget('foo', 1), [ + { id: 'foo', connectionId: 1, following: null }, ]), ).toBe(true) }) + it('cannot follow a player that follows another player', () => { + expect( + canFollowTarget(followTarget('foo', 0), followTarget('bar', 0), [ + { id: 'bar', connectionId: 0, following: 'baz' }, + { id: 'baz', connectionId: 0, following: null }, + ]), + ).toBe(false) + }) + it('cannot follow a player that follows another player indirectly', () => { + expect( + canFollowTarget(followTarget('foo', 0), followTarget('bar', 0), [ + { id: 'bar', connectionId: 0, following: 'baz' }, + { id: 'baz', connectionId: 0, following: 'qux' }, + { id: 'qux', connectionId: 0, following: null }, + ]), + ).toBe(false) + }) it('cannot follow a player back', () => { - expect(canFollowTarget('foo', 'bar', [{ id: 'bar', following: 'foo' }])).toBe(false) + expect( + canFollowTarget(followTarget('foo', 0), followTarget('bar', 0), [ + { id: 'bar', connectionId: 0, following: 'foo' }, + ]), + ).toBe(false) }) it('cannot follow a player that has an indirect loop', () => { expect( - canFollowTarget('foo', 'bar', [ - { id: 'bar', following: 'baz' }, - { id: 'baz', following: 'qux' }, - { id: 'qux', following: 'foo' }, + canFollowTarget(followTarget('foo', 0), followTarget('bar', 0), [ + { id: 'bar', connectionId: 0, following: 'baz' }, + { id: 'baz', connectionId: 0, following: 'qux' }, + { id: 'qux', connectionId: 0, following: 'foo' }, ]), ).toBe(false) }) diff --git a/editor/src/core/shared/multiplayer.ts b/editor/src/core/shared/multiplayer.ts index f43c4aa08fef..74cd2b635245 100644 --- a/editor/src/core/shared/multiplayer.ts +++ b/editor/src/core/shared/multiplayer.ts @@ -1,20 +1,20 @@ import { LiveObject, type CommentData, type ThreadData, type User } from '@liveblocks/client' import { useMutation, + useStorage, type Presence, type ThreadMetadata, type UserMeta, - useStorage, } from '../../../liveblocks.config' -import { possiblyUniqueInArray, safeIndex, stripNulls, uniqBy } from './array-utils' -import { colorTheme } from '../../uuiui' -import type { ElementPath } from './project-file-types' import { setHighlightedView, switchEditorMode, } from '../../components/editor/actions/action-creators' import { EditorModes, existingComment } from '../../components/editor/editor-modes' +import { colorTheme } from '../../uuiui' +import { possiblyUniqueInArray, safeIndex, stripNulls, uniqBy } from './array-utils' import { useMyUserId } from './multiplayer-hooks' +import type { ElementPath } from './project-file-types' export type MultiplayerColor = { background: string @@ -105,25 +105,29 @@ export function isDefaultAuth0AvatarURL(s: string | null): boolean { ) } -export function canFollowTarget( - selfId: string, - targetId: string | null, - others: { id: string; following: string | null }[], -): boolean { - let followChain: Set = new Set() - - let id = targetId - while (id != null) { - if (followChain.has(id)) { - return false - } - followChain.add(id) +export type FollowTarget = { + playerId: string + connectionId: number +} - const target = others.find((o) => o.id === id) - id = target?.following ?? null +export function followTarget(playerId: string, connectionId: number): FollowTarget { + return { + playerId: playerId, + connectionId: connectionId, } +} - return !followChain.has(selfId) +export function canFollowTarget( + from: FollowTarget, + to: FollowTarget, + others: { id: string; following: string | null; connectionId: number }[], +): boolean { + if (from.playerId === to.playerId && from.connectionId === to.connectionId) { + return false + } + return !others.some( + (o) => o.id === to.playerId && o.connectionId === to.connectionId && o.following != null, + ) } const roomIdPrefix = `project-room-`