Skip to content

chore: Add prefab logging #3981

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

Draft
EmandM wants to merge 1 commit into
base: develop-2.0.0
Choose a base branch
from
Draft

chore: Add prefab logging #3981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 13 additions & 15 deletions com.unity.netcode.gameobjects/Runtime/Configuration/NetworkConfig.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
using System.Collections.Generic;
using System.Linq;
using Unity.Collections;
using Unity.Netcode.Logging;
using UnityEditor;
using UnityEngine;
using UnityEngine.Serialization;

Expand Down Expand Up @@ -246,11 +248,19 @@ internal NetworkConfig Copy()
/// runtime modifications to a property outside of the recommended range.
/// For each property checked below, provide a brief description of the reason.
/// </remarks>
internal void OnValidate()
internal void OnValidate(ContextualLogger log)
{
// Legacy NGO versions defaulted this value to 1 second that has since been determiend
// any range less than 10 seconds can lead to dropped messages during scene events.
SpawnTimeout = Mathf.Clamp(SpawnTimeout, MinSpawnTimeout, MaxSpawnTimeout);

var activeScene = UnityEngine.SceneManagement.SceneManager.GetActiveScene();

// If the scene is dirty and the asset database is not currently updating then we can validate the NetworkPrefabs
if (activeScene.isDirty && !EditorApplication.isUpdating)
{
Prefabs.Initialize(log);
}
}

/// <summary>
Expand Down Expand Up @@ -378,26 +388,14 @@ public bool CompareConfig(ulong hash)
return hash == GetConfig();
}

internal void InitializePrefabs()
internal void InitializePrefabs(ContextualLogger log)
{
if (HasOldPrefabList())
{
MigrateOldNetworkPrefabsToNetworkPrefabsList();
}

Prefabs.Initialize();
}

[NonSerialized]
private bool m_DidWarnOldPrefabList = false;

private void WarnOldPrefabList()
{
if (!m_DidWarnOldPrefabList)
{
Debug.LogWarning("Using Legacy Network Prefab List. Consider Migrating.");
m_DidWarnOldPrefabList = true;
}
Prefabs.Initialize(log);
}

/// <summary>
Expand Down
53 changes: 28 additions & 25 deletions com.unity.netcode.gameobjects/Runtime/Configuration/NetworkPrefab.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System;
using System.Collections.Generic;
using Unity.Netcode.Logging;
using UnityEngine;

namespace Unity.Netcode
Expand Down Expand Up @@ -146,24 +148,37 @@ public uint TargetPrefabGlobalObjectIdHash
/// <returns>True if the NetworkPrefab is valid and ready for use, false otherwise</returns>
public bool Validate(int index = -1)
{
var log = new ContextualLogger();
return Validate(log, index);
}

internal bool Validate(ContextualLogger log, int index = -1)
{
using var logContext = log.AddDisposableInfo("Invalid prefab", Prefab?.name);

NetworkObject networkObject;
if (Override == NetworkPrefabOverride.None)
{
if (Prefab == null)
{
NetworkLog.LogWarning($"{nameof(NetworkPrefab)} cannot be null ({nameof(NetworkPrefab)} at index: {index})");
log.Warning(new Context(LogLevel.Error, $"{nameof(NetworkPrefab)} cannot be null").AddInfo($"{nameof(NetworkPrefab)} at index", index));
return false;
}

networkObject = Prefab.GetComponent<NetworkObject>();
if (networkObject == null)
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
log.Warning(new Context(LogLevel.Error, $"Prefab is missing a {nameof(NetworkObject)} component!").AddObject(Prefab));
return false;
}

{
var childNetworkObjects = new List<NetworkObject>();
Prefab.GetComponentsInChildren(true, childNetworkObjects);
if (childNetworkObjects.Count > 1) // total count = 1 root NetworkObject + n child NetworkObjects
{
NetworkLog.LogWarning($"{NetworkPrefabHandler.PrefabDebugHelper(this)} is missing a {nameof(NetworkObject)} component (entry will be ignored).");
log.Warning(new Context(LogLevel.Error, $"Prefab has child {nameof(NetworkObject)}(s) but they will not be spawned across the network (unsupported {nameof(NetworkPrefab)} setup)").AddObject(Prefab));
}

return false;
}

return true;
Expand All @@ -176,14 +191,9 @@ public bool Validate(int index = -1)
{
if (SourceHashToOverride == 0)
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
{
NetworkLog.LogWarning($"{nameof(NetworkPrefab)} {nameof(SourceHashToOverride)} is zero (entry will be ignored).");
}

log.Warning(new Context(LogLevel.Error, $"{nameof(NetworkPrefab)} {nameof(SourceHashToOverride)} is zero!"));
return false;
}

break;
}
case NetworkPrefabOverride.Prefab:
Expand All @@ -197,20 +207,16 @@ public bool Validate(int index = -1)
{
SourcePrefabToOverride = Prefab;
}
else if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
else
{
NetworkLog.LogWarning($"{nameof(NetworkPrefab)} {nameof(SourcePrefabToOverride)} is null (entry will be ignored).");
log.Warning(new Context(LogLevel.Error, $"{nameof(NetworkPrefab)} {nameof(SourcePrefabToOverride)} is null!"));
return false;
}
}

if (!SourcePrefabToOverride.TryGetComponent(out networkObject))
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
{
NetworkLog.LogWarning($"{nameof(NetworkPrefab)} ({SourcePrefabToOverride.name}) is missing a {nameof(NetworkObject)} component (entry will be ignored).");
}

log.Warning(new Context(LogLevel.Error, $"{nameof(NetworkPrefab)} is missing a {nameof(NetworkObject)} component!").AddObject(SourcePrefabToOverride));
return false;
}

Expand All @@ -221,21 +227,18 @@ public bool Validate(int index = -1)
// Validate target prefab override values next
if (OverridingTargetPrefab == null)
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
{
NetworkLog.LogWarning($"{nameof(NetworkPrefab)} {nameof(OverridingTargetPrefab)} is null!");
}

// Safe to create context early as this code is not in any hot path
var ctx = new Context(LogLevel.Error, $"{nameof(OverridingTargetPrefab)} is null! {nameof(NetworkPrefab)} entry will be removed and ignored.");
switch (Override)
{
case NetworkPrefabOverride.Hash:
{
Debug.LogWarning($"{nameof(NetworkPrefab)} override entry {SourceHashToOverride} will be removed and ignored.");
log.Warning(ctx.AddInfo(nameof(SourceHashToOverride), SourceHashToOverride));
break;
}
case NetworkPrefabOverride.Prefab:
{
Debug.LogWarning($"{nameof(NetworkPrefab)} override entry ({SourcePrefabToOverride.name}) will be removed and ignored.");
log.Warning(ctx.AddInfo(nameof(SourcePrefabToOverride), SourcePrefabToOverride.name));
break;
}
}
Expand Down
106 changes: 61 additions & 45 deletions com.unity.netcode.gameobjects/Runtime/Configuration/NetworkPrefabs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using System;
using System.Collections.Generic;
using System.Text;
using Unity.Netcode.Logging;
using UnityEngine;

namespace Unity.Netcode
Expand Down Expand Up @@ -50,9 +50,15 @@ public class NetworkPrefabs
[NonSerialized]
private List<NetworkPrefab> m_RuntimeAddedPrefabs = new List<NetworkPrefab>();

private ContextualLogger m_Log;

private bool m_Initialized;


private void AddTriggeredByNetworkPrefabList(NetworkPrefab networkPrefab)
{
if (AddPrefabRegistration(networkPrefab))
// We don't have to re-validate the prefab as the PrefabList will have validated before invoking this
if (AddPrefabRegistrationPreValidated(networkPrefab))
{
// Don't add this to m_RuntimeAddedPrefabs
// This prefab is now in the PrefabList, so if we shutdown and initialize again, we'll pick it up from there.
Expand All @@ -79,6 +85,7 @@ private void RemoveTriggeredByNetworkPrefabList(NetworkPrefab networkPrefab)
/// </summary>
internal void Shutdown()
{
m_Initialized = false;
foreach (var list in NetworkPrefabsLists)
{
list.OnAdd -= AddTriggeredByNetworkPrefabList;
Expand All @@ -93,27 +100,18 @@ internal void Shutdown()
/// <param name="warnInvalid">When true, logs warnings about invalid prefabs that are removed during initialization</param>
public void Initialize(bool warnInvalid = true)
{
Initialize(m_Log ?? new ContextualLogger(), warnInvalid);
}

internal void Initialize(ContextualLogger log, bool warnInvalid = true)
{
m_Log = log;
m_Prefabs.Clear();
NetworkPrefabsLists.RemoveAll(x => x == null);
foreach (var list in NetworkPrefabsLists)
{
list.OnAdd += AddTriggeredByNetworkPrefabList;
list.OnRemove += RemoveTriggeredByNetworkPrefabList;
}

NetworkPrefabOverrideLinks.Clear();
OverrideToNetworkPrefab.Clear();

var prefabs = new List<NetworkPrefab>();

if (NetworkPrefabsLists.Count != 0)
{
foreach (var list in NetworkPrefabsLists)
{
prefabs.AddRange(list.PrefabList);
}
}

m_Prefabs = new List<NetworkPrefab>();

List<NetworkPrefab> removeList = null;
Expand All @@ -122,15 +120,33 @@ public void Initialize(bool warnInvalid = true)
removeList = new List<NetworkPrefab>();
}

foreach (var networkPrefab in prefabs)
foreach (var list in NetworkPrefabsLists)
{
if (AddPrefabRegistration(networkPrefab))
if (list == null)
{
m_Prefabs.Add(networkPrefab);
continue;
}
else
// Validate will remove any invalid items from the list
list.BuildLogger();

list.OnAdd += AddTriggeredByNetworkPrefabList;
list.OnRemove += RemoveTriggeredByNetworkPrefabList;

foreach (var networkPrefab in list.List)
{
removeList?.Add(networkPrefab);
if (networkPrefab == null)
{
continue;
}

if (networkPrefab.Validate(list.Log) && AddPrefabRegistrationPreValidated(networkPrefab))
{
m_Prefabs.Add(networkPrefab);
}
else
{
removeList?.Add(networkPrefab);
}
}
}

Expand All @@ -149,13 +165,10 @@ public void Initialize(bool warnInvalid = true)
// Clear out anything that is invalid or not used
if (removeList?.Count > 0)
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
{
var sb = new StringBuilder("Removing invalid prefabs from Network Prefab registration: ");
sb.AppendJoin(", ", removeList);
NetworkLog.LogWarning(sb.ToString());
}
log.Warning(new Context(LogLevel.Error, "Removing invalid prefabs from Network Prefab registration"));
}

m_Initialized = true;
}

/// <summary>
Expand All @@ -171,6 +184,12 @@ public void Initialize(bool warnInvalid = true)
/// </remarks>
public bool Add(NetworkPrefab networkPrefab)
{
if (!m_Initialized)
{
m_RuntimeAddedPrefabs.Add(networkPrefab);
return true;
}

if (AddPrefabRegistration(networkPrefab))
{
m_Prefabs.Add(networkPrefab);
Expand Down Expand Up @@ -287,43 +306,40 @@ private bool AddPrefabRegistration(NetworkPrefab networkPrefab)
return false;
}
// Safeguard validation check since this method is called from outside of NetworkConfig and we can't control what's passed in.
if (!networkPrefab.Validate())
if (!networkPrefab.Validate(m_Log))
{
return false;
}
return AddPrefabRegistrationPreValidated(networkPrefab);
}

private bool AddPrefabRegistrationPreValidated(NetworkPrefab networkPrefab)
{
uint source = networkPrefab.SourcePrefabGlobalObjectIdHash;
uint target = networkPrefab.TargetPrefabGlobalObjectIdHash;

// Make sure the prefab isn't already registered.
if (NetworkPrefabOverrideLinks.ContainsKey(source))
if (NetworkPrefabOverrideLinks.TryGetValue(source, out var otherPrefab))
{
var networkObject = networkPrefab.Prefab.GetComponent<NetworkObject>();

// This should never happen, but in the case it somehow does log an error and remove the duplicate entry
Debug.LogError($"{nameof(NetworkPrefab)} ({networkObject.name}) has a duplicate {nameof(NetworkObject.GlobalObjectIdHash)} source entry value of: {source}!");
m_Log.Error(new Context(LogLevel.Error, $"{nameof(NetworkPrefab)} has a matching {nameof(NetworkObject.GlobalObjectIdHash)} with another object. This should not happen!").AddInfo(nameof(NetworkObject.GlobalObjectIdHash), source).AddInfo("Duplicated Object", otherPrefab.Prefab.name).AddObject(networkPrefab.Prefab));
return false;
}

// If we don't have an override configured, registration is simple!
if (networkPrefab.Override == NetworkPrefabOverride.None)
{
NetworkPrefabOverrideLinks.Add(source, networkPrefab);
return true;
}

switch (networkPrefab.Override)
{
case NetworkPrefabOverride.None:
{
NetworkPrefabOverrideLinks.Add(source, networkPrefab);
break;
}
case NetworkPrefabOverride.Prefab:
case NetworkPrefabOverride.Hash:
{
NetworkPrefabOverrideLinks.Add(source, networkPrefab);
if (!OverrideToNetworkPrefab.ContainsKey(target))
{
OverrideToNetworkPrefab.Add(target, source);
}
OverrideToNetworkPrefab.TryAdd(target, source);
break;
}
break;
}

return true;
Expand Down
Loading