Facepunch / sbox-issues

175 stars 12 forks source link

`[HostSync, Sync]` property not getting set on host #6467

Open matekdev opened 1 month ago

matekdev commented 1 month ago

Describe the bug

I have a [HostSync, Sync] property that is NOT getting set for the host when a non-host client sets the value.

To Reproduce

  1. Every client that joins sets their own Pawn value
    [HostSync, Sync]
    public Pawn Pawn { get; private set; }
  2. When a new client joins it properly syncs the Pawn value of the host
  3. The host however does NOT get the Pawn value synced of the new connecting client

Expected behavior

In the same project, the value of

[HostSync, Sync]
public Pawn Pawn { get; private set; }

should get synced for the host.

Media/Files

No response

Additional context

No response

matekdev commented 1 month ago

If you remove the HostSync from the Pawn property everything works correctly. I've double checked and the client IS the owner of the object before it attempts to change the Pawn value.

Metapyziks commented 1 month ago

[HostSync] means the host is the authority for that property, so only the host should be able to set it, right? I'm not sure that having both HostSync and Sync on the same property is supported or makes sense.

chrisspieler commented 1 month ago

[HostSync] means the host is the authority for that property, so only the host should be able to set it, right? I'm not sure that having both HostSync and Sync on the same property is supported or makes sense.

The docs say that the two attributes can be used together to have a property that can be set by the host and the owner.

https://docs.facepunch.com/s/sbox-dev/doc/sync-properties-jKFHwTGVgR#h-hostsync-attribute

trundlr commented 1 month ago

It feels silly to have both. We should just be able to pass params into a single Sync attribute.

matekdev commented 1 month ago

It feels silly to have both. We should just be able to pass params into a single Sync attribute.

Hmm... might be a better idea than what it currently is. For instance, if an enum is provided to an attribute, more people might be aware of this functionality.

Metapyziks commented 1 month ago

The docs say that the two attributes can be used together to have a property that can be set by the host and the owner.

My mistake, thanks for the correction.

Coomzy commented 1 month ago

Adding a Task.Frame() delay seems to fix the issue, still feels like unintended behaviour unless OnLoad() is not network safe? If so, NetworkSpawn should probably have thrown a warning or something if it wasn't ready?

protected override async Task OnLoad()
{
    if ( !Sandbox.Game.IsPlaying )
        return;

    if ( !GameNetworkSystem.IsActive )
        GameNetworkSystem.CreateLobby();

    DelaySpawn(Connection.Local);

    await GameTask.CompletedTask;
}

async void DelaySpawn(Connection connection)
{
    await Task.Frame();

    var clientObj = SceneUtility.GetPrefabScene( ClientPrefab ).Clone();
    clientObj.NetworkSpawn( connection );

    var client = clientObj.Components.Get<Client>();
    client.AssignConnection( connection );

    // Option #1
    // The pawn component MUST have PawnAttribute.
    client.AssignPawn<SpectatePawn>();

    // Option #2
    // The pawn component does NOT need PawnAttribute in this case.
    // client.AssignPawn<SpectatePawn>(SpectatePrefab);
}
GavrilovNI commented 1 month ago

isn't Sync and HostSync unreliable shouldn't you use Broadcast for that use case?

kurozael commented 1 month ago

I think in this case it may be that the Pawn doesn't exist yet when the Sync property is received on the host. Since you're creating the object on the client and then network spawning it - so when the host gets the Sync property value (Id) there is no GameObject yet.

This may be that we need to introduce some kind of handle.

matekdev commented 1 month ago

I think in this case it may be that the Pawn doesn't exist yet when the Sync property is received on the host. Since you're creating the object on the client and then network spawning it - so when the host gets the Sync property value (Id) there is no GameObject yet.

This may be that we need to introduce some kind of handle.

What is interesting though is if you remove the HostSync it works without any issue. So I'm not sure if this could be the cause?