Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cursors Based On Connections Instead Of Users. #4794

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

seanparsons
Copy link
Contributor

Problem:
We want to see multiplayer cursors for our user if they're on a different instance of the editor.

Fix:
There's two main changes:

  • Attaching the colorIndex to the connection.
  • Creating cursors based off of individual connections and not excluding those which are present for another connection for the same user.
    A lot of the work was mostly just making sure those requirements were satisfied. However one outcome was that as the colorIndex value was previously connected to the user for the comment listing it wouldn't make sense to have a colour for the connection and for the user, as they would likely be inconsistent. So the decision was made to remove the colours for comments and their avatars.

Commit Details:

  • Comments now lose their user specific colours as the colours are now attached to connections, which would result in some inconsistent colours against comments versus the cursor for them.
  • MultiplayerCursors now uses excludeMyConnection to get everything but the current connection for the current user.
  • When calling multiplayerColorFromIndex, the getConnectionById function is used to obtain the colorIndex for the connection if it exists.
  • MultiplayerUserBar now uses excludeMyConnection to get the connections excluding the current connection used by the current user.
  • Added some utility functions related to connections in comment-hooks.tsx.
  • Moved colorIndex from UserMeta to ConnectionInfo.

- Comments now lose their user specific colours as the colours are
  now attached to connections, which would result in some inconsistent
  colours against comments versus the cursor for them.
- `MultiplayerCursors` now uses `excludeMyConnection` to get everything
  but the current connection for the current user.
- When calling `multiplayerColorFromIndex`, the `getConnectionById` function
  is used to obtain the `colorIndex` for the connection if it exists.
- `MultiplayerUserBar` now uses `excludeMyConnection` to get the connections
  excluding the current connection used by the current user.
- Added some utility functions related to connections in `comment-hooks.tsx`.
- Moved `colorIndex` from `UserMeta` to `ConnectionInfo`.
Copy link
Contributor

github-actions bot commented Jan 24, 2024

Try me

Copy link

relativeci bot commented Jan 24, 2024

Job #10045: Bundle Size — 59.82MiB (+0.03%).

1ef4b31(current) vs 8f5e3ac master#10040(baseline)

Warning

Bundle contains 66 duplicate packages – View duplicate packages

Bundle metrics  Change 5 changes Regression 3 regressions
                 Current
Job #10045
     Baseline
Job #10040
Regression  Initial JS 45.64MiB(+0.03%) 45.63MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 23.79% 20.42%
No change  Chunks 27 27
No change  Assets 31 31
Change  Modules 4500(+0.04%) 4498
Regression  Duplicate Modules 581(+0.35%) 579
Regression  Duplicate Code 27.55%(+0.07%) 27.53%
No change  Packages 462 462
No change  Duplicate Packages 65 65
Bundle size by type  Change 1 change Regression 1 regression
                 Current
Job #10045
     Baseline
Job #10040
Regression  JS 59.81MiB (+0.03%) 59.79MiB
Not changed  HTML 11.87KiB 11.87KiB

View job #10045 reportView feature/multiplayer-per-connecti... branch activityView project dashboard

@seanparsons seanparsons merged commit 25260fa into master Jan 25, 2024
13 of 15 checks passed
@seanparsons seanparsons deleted the feature/multiplayer-per-connection branch January 25, 2024 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants