tldraw / tldraw-v1

A tiny little drawing app. This is the original 2021-2022 version, released under MIT.
https://old.tldraw.com
MIT License
64 stars 35 forks source link

[bug] Too many cursor show up in Multiplayer mode. #245

Closed lichin-lin closed 2 years ago

lichin-lin commented 2 years ago

First of all, thanks for making the multiplayer feature, it works in most cases!

Here is one bug I found: If you try to refresh a tab with an element selected, somehow the removeuser function in useMultiplayerState.ts will not be triggered, so in other user's views, they will see more and more cursors show up (but actually, they all indicate the same user.)

I am not sure if this has to do with how Liveblock sends the event to a room, or, we didn't really clean up the userID in our storage so that we can't show the correct amount of cursor in a room 🙌

https://user-images.githubusercontent.com/10290616/154104177-8a26a71b-d30f-48c0-bd48-afdd73ed907f.mov

a-type commented 2 years ago

If you're using Liveblocks I'd recommend this improvement we made in our app vs the example code for tracking active users:

liveBlocks.subscribe('others', (others, event) => {
  if (event.type === 'leave') {
    if (event.user.presence) {
      app.removeUser(event.user.presence.id);
    }
  } else {
    app.updateUsers(
      others.map((p) => p.presence)
    );
  }
})

Assuming you're storing TLDraw user data directly in Liveblocks presence, presence.id will be the TLDraw user ID.

By listening explicitly for Liveblocks user leave events, which are delivered reliably, you can drop the beforeunload handlers and the use of broadcast events to tell peers you're leaving.

judicaelandria commented 2 years ago

Hi @lichin-lin , this seems to be fixed, could you test it again please, if it's working for you, please close the issue! thanks

lichin-lin commented 2 years ago

I think this issue still occurs, and it's quite easily to reproduce:

  1. open a multiplayer room
  2. open two tabs
  3. keep refreshing on one of the tab
  4. you will get the following result on the other tab (where too many cursor stay on the page)

https://user-images.githubusercontent.com/10290616/169863224-d7b6c9ee-ac7e-4eb7-bdb7-9765cca85600.mov

judicaelandria commented 2 years ago

It appears because you're on the same browser, I tried it on a different browser

lichin-lin commented 2 years ago

I've also tested with different browsers, but still get the same issue:

https://user-images.githubusercontent.com/10290616/169864221-105c0be7-9ee1-474f-adf0-fc33ebfcc587.mov

judicaelandria commented 2 years ago

that's weird, can you try deleting the cache? and the retry please, let me know if the bugs still occur

lichin-lin commented 2 years ago

Yes, I've tried cleaning the cache, and use incognito to test the behavior, sadly, this issue still happens. 😢

judicaelandria commented 2 years ago

okay, I can reproduce it now! I'll try to fix it

judicaelandria commented 2 years ago

the problem was the user presence was never deleted when they exit or leave so that's why there are many cursor because the user presence before he leaves is never deleted

judicaelandria commented 2 years ago

@lichin-lin can you try with this link if it's working, please? link preview tldraw

lichin-lin commented 2 years ago

yes!! it's working perfectly!

judicaelandria commented 2 years ago

okay it's merged, can you close this issue now @lichin-lin please ? thanks