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.

Sign up for GitHub

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

Jump to bottom

Allow networked entities to become children of a non-networked parent #401

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Allow networked entities to become children of a non-networked parent #401

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1ffa782
Allow networked entities to become childeren of a non-networked parent
Agupta00 Mar 12, 2023
15e3a8c
update basic example with commennts
Agupta00 Mar 12, 2023
de3b374
require # to be in param
Agupta00 Mar 12, 2023
48011a5
add platform example and update to use syncData.parent field
Agupta00 Mar 30, 2023
0e1fe09
quality updates
Agupta00 Mar 30, 2023
c89b885
quality
Agupta00 Mar 30, 2023
af42b9b
fix css rule
Agupta00 Mar 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion examples/basic.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@
<a-entity environment="preset:starry;groundColor:#000000;"></a-entity>
<a-entity light="type:ambient;intensity:0.5"></a-entity>

<!-- Networked entities will be added as children of the platform -->
<a-box id="platform" color="tomato" depth="10" height=".5" width="10" position="0 10 0">

<!-- Here we declare only the local user's avatar, which we then broadcast to other users -->
<!-- The 'spawn-in-circle' component will set the position and rotation of #rig;
because this entity also has the networked component, and position and rotation are tracked by default,
Expand All @@ -108,7 +111,8 @@
including all applicable child elements. However, because we don't need to see our own avatar, we use the
`attachTemplateToLocal:false` option. This makes our local copies invisible on our machine, but visible on everyone else's.
-->
<a-entity id="rig" movement-controls="fly:true;" spawn-in-circle="radius:3" networked="template:#rig-template;">
<a-entity id="rig" movement-controls="fly:true;" spawn-in-circle="radius:3" networked="template:#rig-template; attachToParentId:#platform;">
Copy link
Member

Choose a reason for hiding this comment

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

Instead of modifying the basic example. Can you create a new example where you absolutely need your changes.
For this modified example, instead of attachToParentId:#platform;" you can really just use position="0 10 0" on rig and it will be absolutely the same.

Copy link
Author

@Agupta00 Agupta00 Mar 14, 2023 •
edited

Choose a reason for hiding this comment

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

Yeah sure, did you have an idea of what example would be good? Perhaps I can change it so the client can manually set the hight of the platform local to his own computer. Then I think you would absolutely need this change. Does that work?

Copy link
Member

Choose a reason for hiding this comment

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

I'm asking you because I don't know which use case those changes would really be useful. If I merge changes, I want an example where the changes are necessary and the example can't be achieved in another way.
Having the height of the platform configurable doesn't change anything, you change both position y on rig and platform and that would do it without the changes.
Didn't you have a real use case for those changes?
Your modified example seems the wrong use case to me. What about a second platform, where the avatars can go, you will need to change the parent of your avatar, how would it work? Generally you would not put your avatar rig as a parent of another entity but use a root entity, you would then update the avatar rig according to where the avatar is. This is how it works with the two different navmesh components we have.

Copy link
Author

Choose a reason for hiding this comment

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

Apologies in advance if I misunderstood anything.., here is why I think my example works.

Example: Player A's platform's y=10, Player B's platform is at y=20.

On Player B's local client, how would you ensure that Player A ends up on my platform? If you set Player A rig entity position to y=20, then the next time Player A moves, the rig's position would get updated back to the original y height since its rig is networked entity right? If it is still unclear I can make the example and perhaps you can try it.

(side note): I tried setting the position of a networked entity (from another client) and it did not update, maybe there is logic that prevents this anyways?

Real Life Example

Here is a video of a real use case, I can attach the scene code if it helps, but in summary, each client has a sort of "platform" whose position is local the client. In the video the "platform" is the green outline, which is tracked and changed locally on the client. A different phone viewing the same target can have a different position for the "platform".

The thing I care about syncing is the robot's positions, but only relative to the "platform". You can think of the "platform" as the root of my networked scene.

example.mp4 
  <a-scene>
    <a-entity id="platform"> //local client can set this to anything. The position here only makes sense to the client so it should not be networked.
      <a-entity id="avatar" networked=""template:#avatar-template> // The robot's position in the scene should make sense relative to other players in the scene as well. 
      </a-entity>
    </a-entity>
  </a-scene>

Copy link
Member

Choose a reason for hiding this comment

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

For your side note, you need to call NAF.utils.takeOwnership(yourEntity) to be the owner of it so the changes propagate to the other users.
Ok if you have a different platform height for each user, your example indeed is fine, and of course you don't want it networked.
What I wanted to understand is why you need a platform height different for each user.
From your video I really don't know what I'm seeing.
But I can see that it can probably be useful if you set the platform height corresponding to an anchor on a table in AR for example, although you could just set the position on a-scene I think.
It would be interesting to have an example with one user on desktop and another with Chrome Android with AR on the phone moving a character like your video.

Copy link
Author

@Agupta00 Agupta00 Mar 16, 2023 •
edited

Choose a reason for hiding this comment

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

Great, so just to confirm, I will plan to make the example with platforms of different heights then if that's okay with you?

You can play around with it after and try out your suggestions if you still have any doubts 😄 .

Copy link
Member

Choose a reason for hiding this comment

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

Yes please go ahead.
I don't know if you have an ARCore compatible Android phone. If you do, you can look at creating an experience in AR to place the platform on a table, see https://github.com/AdaRoseCannon/aframe-xr-boilerplate for an example. Chrome Android is only implementing hittest, that's enough to have the correct height of the table. Out of scope here, but I think you can now detect a plane on Meta browser on Quest, so probably you can know the table boundaries and set the width and height of the platform accordingly too.

<!-- We add attachToParentId:platform because we want networked entities to end up as a child of platform entity not the root a-scene. -->
<!-- Here we add the camera. Adding the camera within a 'rig' is standard practice.
We set the camera to head height for e.g. computer users, but otherwise never touch it again; if the user enters VR,
its rotation and position will be updated by the headset in VR. If we need to touch the user's position
Expand All @@ -125,6 +129,8 @@
>
</a-entity>
</a-entity>

</a-box>
</a-scene>

<script>
Expand Down
17 changes: 15 additions & 2 deletions src/NetworkEntities.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class NetworkEntities {

receiveFirstUpdateFromEntity(entityData) {
var parent = entityData.parent;
var attachToParentId = entityData.attachToParentId;
var networkId = entityData.networkId;

var parentNotCreatedYet = parent && !this.hasEntity(parent);
Expand All @@ -96,7 +97,7 @@ class NetworkEntities {
} else {
var remoteEntity = this.createRemoteEntity(entityData);
this.createAndAppendChildren(networkId, remoteEntity);
this.addEntityToPage(remoteEntity, parent);
this.addEntityToPage(remoteEntity, parent, attachToParentId);
}
}

Expand All @@ -121,9 +122,11 @@ class NetworkEntities {
}
}

addEntityToPage(entity, parentId) {
addEntityToPage(entity, parentId, attachToParentId) {
if (this.hasEntity(parentId)) {
this.addEntityToParent(entity, parentId);
} else if (attachToParentId) {
this.addEntityToParentId(entity, attachToParentId)
} else {
this.addEntityToSceneRoot(entity);
}
Expand All @@ -133,6 +136,16 @@ class NetworkEntities {
this.entities[parentId].appendChild(entity);
}

addEntityToParentId(el, attachToParentId) {
var parent = document.querySelector(attachToParentId);
if (!parent) {
NAF.log.warn('Tried to add entity to parent, but parent with id', attachToParentId,
'is not in the scene');
return;
}
parent.appendChild(el);
}

addEntityToSceneRoot(el) {
var scene = document.querySelector('a-scene');
scene.appendChild(el);
Expand Down
2 changes: 2 additions & 0 deletions src/components/networked.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ AFRAME.registerComponent('networked', {
schema: {
template: {default: ''},
attachTemplateToLocal: { default: true },
attachToParentId: { default: '' },
persistent: { default: false },

networkId: {default: ''},
Expand Down Expand Up @@ -427,6 +428,7 @@ AFRAME.registerComponent('networked', {
syncData.template = data.template;
syncData.persistent = data.persistent;
syncData.parent = this.getParentId();
syncData.attachToParentId = data.attachToParentId;
Copy link
Member

Choose a reason for hiding this comment

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

I see we already have syncData.parent here, so adding a new field to sync a parent id seems redundant. I think it would be better to modify getParentId and initNetworkParent here, and keep the addEntityToPage(entity, parentId) signature.

syncData.components = components;
syncData.isFirstSync = !!isFirstSync;
return syncData;
Expand Down