UCL-VR / ubiq

Other
98 stars 34 forks source link

Network IDs of pre-existing scene objects are always the same #42

Closed KilliKrate closed 9 months ago

KilliKrate commented 9 months ago

I've been trying to create a scene with different objects placed around it. So far I've created a Ball script and Frisbee script (and their respective prefabs) with some differences regarding their Grasp and Use behaviour. My problem is that I cannot seem to get them to have different Network IDs (and thus be synchronized as different objects on different peers).

Note that this doesn't apply in the case of spawning, as spawned objects have different IDs assigned to them, while the pre-existing ones always have a series of zeroes separated by a dash. How should I assign different IDs to them?

bnco-dev commented 9 months ago

Hi @KilliKrate. If it's a dynamic (spawned) object, as you say, you can rely on the spawner to assign IDs. If it's a static object in the scene, your best bet is using NetworkScene.Register(this); in your script. That way Ubiq signs you up for messages using a unique ID based on the object's position in the scene hierarchy relative to the NetworkScene. It also returns a NetworkContext that is a convenient shortcut for sending messages. Have a look at the Firework.cs script in the samples for an example.

KilliKrate commented 9 months ago

Thanks for the quick response 😄 I've already tried using NetworkScene.Register(this); and even some of its overloaded variants (namely with NetworkId.Create()) with no avail. I took a look at the Firework.cs script and I've attached a Network Id Gizmo to the Firework prefab and while it's true that the IDs are different, it's also a spawned object (namely by FireworkBox.cs). The problem I've had is with objects already present in the scene.

image

bnco-dev commented 9 months ago

No problem! Would you mind attaching your script files for the frisbee and ball?

KilliKrate commented 9 months ago

Not at all, here they are

Ball.cs:

using System.Collections;
using System.Collections.Generic;
using Ubiq.XR;
using Ubiq.Messaging;
using UnityEngine;
using Ubiq.Spawning;
using System.Linq;
using UnityEngine.UIElements;
using Ubiq.Avatars;

namespace CustomAssets
{
    public class Ball : MonoBehaviour, IGraspable, INetworkSpawnable
    {
        Hand grasped;
        NetworkContext context;

        public NetworkId NetworkId { get; set; }

        private struct Message
        {
            public Vector3 position;
        }

        void Start()
        {
            context = NetworkScene.Register(this);
        }

        void Update()
        {
            // Match the hand position to the ball position if it is grabbed
            if (grasped != null)
            {
                transform.localPosition = grasped.transform.position;
                context.SendJson(new Message()
                {
                    position = transform.localPosition
                });
            }
        }

        public void ProcessMessage(ReferenceCountedSceneGraphMessage message)
        {
            // Parse the message
            var m = message.FromJson<Message>();

            // Use the message to update the Component
            transform.localPosition = m.position;
        }

        void IGraspable.Grasp(Hand controller)
        {
            grasped = controller;
        }

        void IGraspable.Release(Hand controller)
        {
            grasped = null;
        }
    }
}

Frisbee.cs:

using System.Collections;
using System.Collections.Generic;
using System.Linq;
using Ubiq.Messaging;
using Ubiq.Spawning;
using Ubiq.XR;
using UnityEngine;

namespace CustomAssets
{
    public class Frisbee : MonoBehaviour, INetworkSpawnable, IGraspable, IUseable
    {
        Hand grasped;
        NetworkContext context;

        public NetworkId NetworkId { get; set; }

        private struct Message
        {
            public Vector3 position;
        }

        void Start()
        {
            context = NetworkScene.Register(this);
        }

        void Update()
        {
            // Match the hand position to the ball position if it is grabbed
            if (grasped != null)
            {
                transform.localPosition = grasped.transform.position;
                context.SendJson(new Message()
                {
                    position = transform.localPosition
                });
            }

        }

        public void ProcessMessage(ReferenceCountedSceneGraphMessage message)
        {
            // Parse the message
            var m = message.FromJson<Message>();

            // Use the message to update the Component
            transform.localPosition = m.position;
        }

        void IUseable.Use(Hand controller)
        {
            transform.localRotation = Quaternion.Euler(90.0f, 0.0f, 0.0f);
        }

        void IUseable.UnUse(Hand controller)
        {
            transform.localRotation = Quaternion.Euler(0.0f, 0.0f, 0.0f);
        }
        void IGraspable.Grasp(Hand controller)
        {
            grasped = controller;
        }

        void IGraspable.Release(Hand controller)
        {
            grasped = null;
        }
    }
}
bnco-dev commented 9 months ago

Thanks for the code. I see the same behavior you've described. What's happening is that the ID in the NetworkID property (which is default, all zeroes) is taking precedence over the scene graph-derived ID. It's not exactly a bug but I don't think we really want this to happen, so I've added it to our bug tracker. For now, I think swapping out your previous context assignment with this should fix it:

context = NetworkScene.Register(this,NetworkId.Create(this));

KilliKrate commented 9 months ago

Yup, that fixes it, thanks a lot 😄. I managed to completely skip over the easiest overload of the NetworkId.Create function, as I've already tried this before: context = NetworkScene.Register(this, NetworkId.Create(GetComponent<Ball>().NetworkId, "We ballin'")); context = NetworkScene.Register(this, NetworkId.Unique());

sebjf commented 9 months ago

which is default, all zeroes...but I don't think we really want this to happen, so I've added it to our bug tracker

A check for this that throws an exception was added as part of https://github.com/UCL-VR/ubiq/issues/40 just a couple of days ago! 😄