Skip to content

fix: NetworkBehaviour Property Initialization Updates #1785

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

Merged
Merged
88 changes: 74 additions & 14 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs
Original file line number Diff line number Diff line change
Expand Up @@ -235,42 +235,42 @@ internal void __endSendClientRpc(ref FastBufferWriter bufferWriter, uint rpcMeth
public NetworkManager NetworkManager => NetworkObject.NetworkManager;

/// <summary>
/// Gets if the object is the the personal clients player object
/// If a NetworkObject is assigned, it will return whether or not this NetworkObject
/// is the local player object. If no NetworkObject is assigned it will always return false.
/// </summary>
public bool IsLocalPlayer => NetworkObject.IsLocalPlayer;
public bool IsLocalPlayer { get; private set; }

/// <summary>
/// Gets if the object is owned by the local player or if the object is the local player object
/// </summary>
public bool IsOwner => NetworkObject.IsOwner;
public bool IsOwner { get; internal set; }

/// <summary>
/// Gets if we are executing as server
/// </summary>
protected bool IsServer => IsRunning && NetworkManager.IsServer;
protected bool IsServer { get; private set; }

/// <summary>
/// Gets if we are executing as client
/// </summary>
protected bool IsClient => IsRunning && NetworkManager.IsClient;
protected bool IsClient { get; private set; }


/// <summary>
/// Gets if we are executing as Host, I.E Server and Client
/// </summary>
protected bool IsHost => IsRunning && NetworkManager.IsHost;

private bool IsRunning => NetworkManager && NetworkManager.IsListening;
protected bool IsHost { get; private set; }

/// <summary>
/// Gets Whether or not the object has a owner
/// </summary>
public bool IsOwnedByServer => NetworkObject.IsOwnedByServer;
public bool IsOwnedByServer { get; internal set; }

/// <summary>
/// Used to determine if it is safe to access NetworkObject and NetworkManager from within a NetworkBehaviour component
/// Primarily useful when checking NetworkObject/NetworkManager properties within FixedUpate
/// </summary>
public bool IsSpawned => HasNetworkObject ? NetworkObject.IsSpawned : false;
public bool IsSpawned { get; internal set; }

internal bool IsBehaviourEditable()
{
Expand Down Expand Up @@ -327,12 +327,12 @@ public NetworkObject NetworkObject
/// <summary>
/// Gets the NetworkId of the NetworkObject that owns this NetworkBehaviour
/// </summary>
public ulong NetworkObjectId => NetworkObject.NetworkObjectId;
public ulong NetworkObjectId { get; internal set; }

/// <summary>
/// Gets NetworkId for this NetworkBehaviour from the owner NetworkObject
/// </summary>
public ushort NetworkBehaviourId => NetworkObject.GetNetworkBehaviourOrderIndex(this);
public ushort NetworkBehaviourId { get; internal set; }

/// <summary>
/// Internally caches the Id of this behaviour in a NetworkObject. Makes look-up faster
Expand All @@ -352,7 +352,47 @@ protected NetworkBehaviour GetNetworkBehaviour(ushort behaviourId)
/// <summary>
/// Gets the ClientId that owns the NetworkObject
/// </summary>
public ulong OwnerClientId => NetworkObject.OwnerClientId;
public ulong OwnerClientId { get; internal set; }

/// <summary>
/// Updates properties with network session related
/// dependencies such as a NetworkObject's spawned
/// state or NetworkManager's session state.
/// </summary>
internal void UpdateNetworkProperties()
{
// Set NetworkObject dependent properties
if (NetworkObject != null)
{
// Set identification related properties
NetworkObjectId = NetworkObject.NetworkObjectId;
IsLocalPlayer = NetworkObject.IsLocalPlayer;

// This is "OK" because GetNetworkBehaviourOrderIndex uses the order of
// NetworkObject.ChildNetworkBehaviours which is set once when first
// accessed.
NetworkBehaviourId = NetworkObject.GetNetworkBehaviourOrderIndex(this);

// Set ownership related properties
IsOwnedByServer = NetworkObject.IsOwnedByServer;
IsOwner = NetworkObject.IsOwner;
OwnerClientId = NetworkObject.OwnerClientId;

// Set NetworkManager dependent properties
if (NetworkManager != null)
{
IsHost = NetworkManager.IsListening && NetworkManager.IsHost;
IsClient = NetworkManager.IsListening && NetworkManager.IsClient;
IsServer = NetworkManager.IsListening && NetworkManager.IsServer;
}
}
else // Shouldn't happen, but if so then set the properties to their default value;
{
OwnerClientId = NetworkObjectId = default;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this an error condition we should log? Would be a way to spot mis-use / regressions

IsOwnedByServer = IsOwner = IsHost = IsClient = IsServer = default;
NetworkBehaviourId = default;
}
}

/// <summary>
/// Gets called when the <see cref="NetworkObject"/> gets spawned, message handlers are ready to be registered and the network is setup.
Expand All @@ -366,21 +406,41 @@ public virtual void OnNetworkDespawn() { }

internal void InternalOnNetworkSpawn()
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the fact that now we are relying on OnNetworkSpawn and OnNetworkDespawn now <3

{
IsSpawned = true;
InitializeVariables();
UpdateNetworkProperties();
OnNetworkSpawn();
}

internal void InternalOnNetworkDespawn() { }
internal void InternalOnNetworkDespawn()
{
IsSpawned = false;
UpdateNetworkProperties();
OnNetworkDespawn();
}

/// <summary>
/// Gets called when the local client gains ownership of this object
/// </summary>
public virtual void OnGainedOwnership() { }

internal void InternalOnGainedOwnership()
{
UpdateNetworkProperties();
OnGainedOwnership();
}

/// <summary>
/// Gets called when we loose ownership of this object
/// </summary>
public virtual void OnLostOwnership() { }

internal void InternalOnLostOwnership()
{
UpdateNetworkProperties();
OnLostOwnership();
}

/// <summary>
/// Gets called when the parent NetworkObject of this NetworkBehaviour's NetworkObject has changed
/// </summary>
Expand Down
6 changes: 2 additions & 4 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -549,15 +549,15 @@ internal void InvokeBehaviourOnLostOwnership()
{
for (int i = 0; i < ChildNetworkBehaviours.Count; i++)
{
ChildNetworkBehaviours[i].OnLostOwnership();
ChildNetworkBehaviours[i].InternalOnLostOwnership();
}
}

internal void InvokeBehaviourOnGainedOwnership()
{
for (int i = 0; i < ChildNetworkBehaviours.Count; i++)
{
ChildNetworkBehaviours[i].OnGainedOwnership();
ChildNetworkBehaviours[i].InternalOnGainedOwnership();
}
}

Expand Down Expand Up @@ -796,7 +796,6 @@ internal void InvokeBehaviourNetworkSpawn()
for (int i = 0; i < ChildNetworkBehaviours.Count; i++)
{
ChildNetworkBehaviours[i].InternalOnNetworkSpawn();
ChildNetworkBehaviours[i].OnNetworkSpawn();
}
}

Expand All @@ -805,7 +804,6 @@ internal void InvokeBehaviourNetworkDespawn()
for (int i = 0; i < ChildNetworkBehaviours.Count; i++)
{
ChildNetworkBehaviours[i].InternalOnNetworkDespawn();
ChildNetworkBehaviours[i].OnNetworkDespawn();
}
}

Expand Down
10 changes: 7 additions & 3 deletions testproject/Assets/Tests/Runtime/MessageOrdering.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,14 @@ public IEnumerator SpawnRpcDespawn()
var serverObject = Object.Instantiate(m_Prefab, Vector3.zero, Quaternion.identity);
NetworkObject serverNetworkObject = serverObject.GetComponent<NetworkObject>();
serverNetworkObject.NetworkManagerOwner = server;
serverNetworkObject.Spawn();

SpawnRpcDespawn srdComponent = serverObject.GetComponent<SpawnRpcDespawn>();
srdComponent.Activate();

// Wait until all objects have spawned.
int expectedCount = Support.SpawnRpcDespawn.ClientUpdateCount + 1;
const int maxFrames = 240;
int maxFrames = 240 + Time.frameCount;
var doubleCheckTime = Time.realtimeSinceStartup + 5.0f;
while (Support.SpawnRpcDespawn.ClientUpdateCount < expectedCount && !handler.WasSpawned)
{
Expand All @@ -171,8 +173,10 @@ public IEnumerator SpawnRpcDespawn()

Assert.AreEqual(NetworkUpdateStage.EarlyUpdate, Support.SpawnRpcDespawn.StageExecutedByReceiver);
Assert.AreEqual(Support.SpawnRpcDespawn.ServerUpdateCount, Support.SpawnRpcDespawn.ClientUpdateCount);
var lastFrameNumber = Time.frameCount + 1;
yield return new WaitUntil(() => Time.frameCount >= lastFrameNumber);

// Wait 1 tic for the GameObjet and associated components to be destroyed
yield return new WaitForSeconds(1.0f / server.NetworkConfig.TickRate);

Assert.True(handler.WasDestroyed);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ public override void OnDestroy()
private void RunTest()
{
Debug.Log("Running test...");
GetComponent<NetworkObject>().Spawn();
IncrementUpdateCount();
Destroy(gameObject);
m_Active = false;
Expand Down