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

feat: SDK7 implement ADR-245 (player components) #5724

Draft
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

leanmendoza
Copy link
Contributor

@leanmendoza leanmendoza commented Sep 20, 2023

This is a PoC to demonstrate the implementation of ADR-245 is possible with current state of renderer.

What?

Implements https://adr.decentraland.org/adr/ADR-245

Implemented and tested:

  • Unique Entity each player
  • Recycle entity numbers in the range
  • Pointer Events
  • Emotes
  • Transform updates only in the scene where they are (otherwise null/delete component)
  • Profile changed
    • implemented
    • fix bug: isn't working on preview

Copy link
Contributor

@gonpombo8 gonpombo8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about the perf of this when we have many users on multiple scenes ?
Does it affects in something ? It seems costly but im not sure

@@ -0,0 +1,6 @@
export * from './deleteComponent'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cant we use @dcl/sdk here so we avoid duplicating all these files ?
And use the ones that are already tested ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely we can, I didn't try to iterate fast, this is not going to be implemented by me, only giving a PoC to show it could work

@leanmendoza leanmendoza changed the title wip feat: SDK7 implement ADR-245 (player components) Sep 21, 2023
export { PBPointerEventsResult } from '../../../../protocol/decentraland/sdk/components/pointer_events_result.gen'

const MAX_ENTITY_VERSION = 0xffff
const AVATAR_RESERVED_ENTITY_NUMBER = { from: 10, to: 200 }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is kind of a limitation right ? We can't support more that 190 users 🤔 ?
What when we have the sdk7 avatar scene and foreing entities ? How this two things are going to live together ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the range defined in the ADR is from 32 to 256 (224 players limit). AFAIK, the current limit in the Foundation client is 100 players. I'm not sure how realistic it is to think about more than 224 players but the ADR-245 is in Review state and comments and modifications are welcome.
Regarding the foreign entities, I'm not sure about how they and this are going to live together as I don't know what is the ETA to get the foreign entities a fact. The first (and original) idea was to do this with the Foreign entities (in the Protocol Squad), but due to the complexity and the scope of this, we decided to move with this approach.

Bevy and Godot implementation doesn't have a JavaScript Scene for avatars, and probably Explorer-Alpha goes with that approach, it'd be embedded in the explorer itself.

@leanmendoza
Copy link
Contributor Author

what about the perf of this when we have many users on multiple scenes ? Does it affects in something ? It seems costly but im not sure

Each player is only in one Scene, so the Transform is only sent to it. I don't see any perf issue but we'd need to do some tests.

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.

2 participants