Facepunch / sbox-issues

175 stars 12 forks source link

`[Sync]` properties of Component dynamically added doesn't work #4534

Closed PolSpock closed 8 months ago

PolSpock commented 8 months ago

Describe the bug

Hi,

It was a nightmare to get precisely what was the issue, but here is it.

When you have a Component with [Sync] properties who has been dynamically added after a GameObject.OnAwake(), the properties are not synced between players. So basically GameObject.OnStart() or GameObject.OnEnabled().

So typically Components.Create<CharacterController>(); has Sync Velocity but will never be synced between players instead you create a hotload.

This is the same for all Components i've added dynamically after a GameObject.OnAwake() who contain [Sync] values. (Example through GameObject.OnStart())

To Reproduce

Let's start the reproduction steps : 1) Download https://github.com/Facepunch/sbox-scenestaging/issues 2) Open networking.scene 3) Create TestVelocity.cs Component with:

public class TestVelocity : Component
{
    [Sync] public Vector3 FdpVelocity { get; set; }

    protected override void OnUpdate()
    {
        if ( !IsProxy)
        {
            var cc = GameManager.ActiveScene.Components.GetAll<CharacterController>().Where( x => x.Network.OwnerId == Network.OwnerId ).FirstOrDefault();
            if ( cc is null ) { return; }

            FdpVelocity = cc.Velocity;
        }
    }
}

4) Next, go to NetworkTest.cs

Add this part OnStart() method :

protected override void OnStart()
{
    Log.Info( Network.OwnerId + " OnStart " );

    Components.Create<TestVelocity>();
}

Edit the OnUpdate() method with my shit logs :

protected override void OnUpdate()
{
    if ( IsProxy )
    {
        var testVelocity = Components.Get<TestVelocity>();
        if ( testVelocity is null ) { return; }
        Log.Info( Network.OwnerId + " testVelocity " + testVelocity.FdpVelocity );
    }

    // Same part as original
    if ( IsProxy )
        return;

    UpdatePickup();
}

5) Start networking.scene, and connect the secondary user. 6) Move around and look at the console logs: the FdpVelocity [Sync] is not Synced between players 7) Hotload the game and magically players are resynced

Expected behavior

[Sync] from Component who has been dynamically added must be synced between players (without hotload too)

Media/Files

My reproduce steps in video:

https://github.com/Facepunch/sbox-issues/assets/5229571/9af2f56e-927a-404b-a7e3-4d15e853d899

Additional context

No response

kurozael commented 8 months ago

Because you're adding a component in OnStart, that component won't be serialized across the network because it will essentially be created and added to the GameObject after NetworkSpawn() is called. You should make sure that any components you want serialized with the GameObject when networked are added before NetworkSpawn() is called.

In the future we'll look into networking when components are added or removed from a networked GameObject.

PolSpock commented 8 months ago

Because you're adding a component in OnStart, that component won't be serialized across the network because it will essentially be created and added to the GameObject after NetworkSpawn() is called. You should make sure that any components you want serialized with the GameObject when networked are added before NetworkSpawn() is called.

In the future we'll look into networking when components are added or removed from a networked GameObject.

Yeah by continuing my investigation, this is something i finished to guess.

I was used to switching my PlayerController state from FPSController to SpectatorController by Components.Create or Component.Destroy these Components, creating several sync issues.

So by, your message, this is not good logic anymore and i will edit it.

kurozael commented 8 months ago

In the mean time, just to satisfy my theory, try doing Components.Create in OnAwake and only if we are not the creator (we received this GameObject from someone else).

This is a valid bug report, it's just that it will be resolved at a later date when we add networking of components added or removed after the initial serialization of the GameObject.

PolSpock commented 8 months ago

There is no issue on OnAwake() and Components.Create<TestVelocity>(); This is something i just discovered at the same time you posted your first comment

I have already edited my issue with this information (i have edited a lot my issue lol)

garrynewman commented 8 months ago

We don't support dynamic component adding yet