Skip to content

Commit 904552f

Browse files
NoelStephensUnity0xFA11
NoelStephensUnity
and
0xFA11
authored
fix: networkmanager prefab validation and no scene management manual test (#1073)
* fix This fixes issues with NetworkManager prefab override validation. If you never assigned a Prefab but just wanted to override it would fail. It never properly checked all potential cases, which this fixes. * test This validates the fixes as well as tests that disabling original/legacy scene management works. * fix Fixing issue with the wrong brackets around the wrong pair of initial checks for NetworkPrefab validation. Updated the NetworkPrefabHandlerTests to account for all potential scenarios that should be handled without causing initialization to fail. * refactor and style fixed a typo in a comment in one of the newly added prefab registration checks added a check for a valid hash override but invalid target prefab * test and style fixing comment and adding one of the potential invalid combinations (valid hash but invalid target prefab). * fix Fixing issue with the prefab not pointing to the right network prefab which made the client side not register the proper network prefab to override since disabling scene management requires that in-scene placed NetworkObjects be spawned by the server. * Update com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com> * Update com.unity.netcode.gameobjects/Tests/Runtime/NetworkPrefabHandlerTests.cs Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com> * Update com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com> * Update com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com> * Update com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com> Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com>
1 parent 152178b commit 904552f

10 files changed

+490
-66
lines changed

‎com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs

Lines changed: 122 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,10 @@ private void Initialize(bool server)
465465
// Build the NetworkPrefabOverrideLinks dictionary
466466
for (int i = 0; i < NetworkConfig.NetworkPrefabs.Count; i++)
467467
{
468-
if (NetworkConfig.NetworkPrefabs[i] == null || NetworkConfig.NetworkPrefabs[i].Prefab == null)
468+
var sourcePrefabGlobalObjectIdHash = (uint)0;
469+
var targetPrefabGlobalObjectIdHash = (uint)0;
470+
var networkObject = (NetworkObject)null;
471+
if (NetworkConfig.NetworkPrefabs[i] == null || (NetworkConfig.NetworkPrefabs[i].Prefab == null && NetworkConfig.NetworkPrefabs[i].Override == NetworkPrefabOverride.None))
469472
{
470473
if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
471474
{
@@ -474,60 +477,144 @@ private void Initialize(bool server)
474477
}
475478

476479
removeEmptyPrefabs.Add(i);
477-
478480
continue;
479481
}
480-
else if (NetworkConfig.NetworkPrefabs[i].Prefab.GetComponent<NetworkObject>() == null)
482+
else if (NetworkConfig.NetworkPrefabs[i].Override == NetworkPrefabOverride.None)
481483
{
482-
if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
484+
networkObject = NetworkConfig.NetworkPrefabs[i].Prefab.GetComponent<NetworkObject>();
485+
if (networkObject == null)
483486
{
484-
NetworkLog.LogWarning(
485-
$"{nameof(NetworkPrefab)} (\"{NetworkConfig.NetworkPrefabs[i].Prefab.name}\") is missing a {nameof(NetworkObject)} component");
487+
if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
488+
{
489+
NetworkLog.LogWarning($"{nameof(NetworkPrefab)} (\"{NetworkConfig.NetworkPrefabs[i].Prefab.name}\") is missing " +
490+
$"a {nameof(NetworkObject)} component (entry will be ignored).");
491+
}
492+
removeEmptyPrefabs.Add(i);
493+
continue;
486494
}
487495

488-
// Provide the name of the prefab with issues so the user can more easily find the prefab and fix it
489-
Debug.LogWarning($"{nameof(NetworkPrefab)} (\"{NetworkConfig.NetworkPrefabs[i].Prefab.name}\") will be removed and ignored.");
490-
removeEmptyPrefabs.Add(i);
491-
492-
continue;
496+
// Otherwise get the GlobalObjectIdHash value
497+
sourcePrefabGlobalObjectIdHash = networkObject.GlobalObjectIdHash;
493498
}
494-
495-
var networkObject = NetworkConfig.NetworkPrefabs[i].Prefab.GetComponent<NetworkObject>();
496-
497-
// Assign the appropriate GlobalObjectIdHash to the appropriate NetworkPrefab
498-
if (!NetworkConfig.NetworkPrefabOverrideLinks.ContainsKey(networkObject.GlobalObjectIdHash))
499+
else // Validate Overrides
499500
{
501+
// Validate source prefab override values first
500502
switch (NetworkConfig.NetworkPrefabs[i].Override)
501503
{
502-
default:
503-
case NetworkPrefabOverride.None:
504-
NetworkConfig.NetworkPrefabOverrideLinks.Add(networkObject.GlobalObjectIdHash,
505-
NetworkConfig.NetworkPrefabs[i]);
506-
break;
507-
case NetworkPrefabOverride.Prefab:
504+
case NetworkPrefabOverride.Hash:
508505
{
509-
var sourcePrefabGlobalObjectIdHash = NetworkConfig.NetworkPrefabs[i].SourcePrefabToOverride.GetComponent<NetworkObject>().GlobalObjectIdHash;
510-
NetworkConfig.NetworkPrefabOverrideLinks.Add(sourcePrefabGlobalObjectIdHash, NetworkConfig.NetworkPrefabs[i]);
511-
512-
var targetPrefabGlobalObjectIdHash = NetworkConfig.NetworkPrefabs[i].OverridingTargetPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash;
513-
NetworkConfig.OverrideToNetworkPrefab.Add(targetPrefabGlobalObjectIdHash, sourcePrefabGlobalObjectIdHash);
506+
if (NetworkConfig.NetworkPrefabs[i].SourceHashToOverride == 0)
507+
{
508+
if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
509+
{
510+
NetworkLog.LogWarning($"{nameof(NetworkPrefab)} {nameof(NetworkPrefab.SourceHashToOverride)} is zero (entry will be ignored).");
511+
}
512+
removeEmptyPrefabs.Add(i);
513+
continue;
514+
}
515+
sourcePrefabGlobalObjectIdHash = NetworkConfig.NetworkPrefabs[i].SourceHashToOverride;
516+
break;
514517
}
515-
break;
516-
case NetworkPrefabOverride.Hash:
518+
case NetworkPrefabOverride.Prefab:
517519
{
518-
var sourcePrefabGlobalObjectIdHash = NetworkConfig.NetworkPrefabs[i].SourceHashToOverride;
519-
NetworkConfig.NetworkPrefabOverrideLinks.Add(sourcePrefabGlobalObjectIdHash, NetworkConfig.NetworkPrefabs[i]);
520+
if (NetworkConfig.NetworkPrefabs[i].SourcePrefabToOverride == null)
521+
{
522+
if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
523+
{
524+
NetworkLog.LogWarning($"{nameof(NetworkPrefab)} {nameof(NetworkPrefab.SourcePrefabToOverride)} is null (entry will be ignored).");
525+
}
526+
Debug.LogWarning($"{nameof(NetworkPrefab)} override entry {NetworkConfig.NetworkPrefabs[i].SourceHashToOverride} will be removed and ignored.");
527+
removeEmptyPrefabs.Add(i);
528+
continue;
529+
}
530+
else
531+
{
532+
networkObject = NetworkConfig.NetworkPrefabs[i].SourcePrefabToOverride.GetComponent<NetworkObject>();
533+
if (networkObject == null)
534+
{
535+
if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
536+
{
537+
NetworkLog.LogWarning($"{nameof(NetworkPrefab)} ({NetworkConfig.NetworkPrefabs[i].SourcePrefabToOverride.name}) " +
538+
$"is missing a {nameof(NetworkObject)} component (entry will be ignored).");
539+
}
540+
Debug.LogWarning($"{nameof(NetworkPrefab)} override entry (\"{NetworkConfig.NetworkPrefabs[i].SourcePrefabToOverride.name}\") will be removed and ignored.");
541+
removeEmptyPrefabs.Add(i);
542+
continue;
543+
}
544+
sourcePrefabGlobalObjectIdHash = networkObject.GlobalObjectIdHash;
545+
}
546+
break;
547+
}
548+
}
520549

521-
var targetPrefabGlobalObjectIdHash = NetworkConfig.NetworkPrefabs[i].OverridingTargetPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash;
522-
NetworkConfig.OverrideToNetworkPrefab.Add(targetPrefabGlobalObjectIdHash, sourcePrefabGlobalObjectIdHash);
550+
// Validate target prefab override values next
551+
if (NetworkConfig.NetworkPrefabs[i].OverridingTargetPrefab == null)
552+
{
553+
if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
554+
{
555+
NetworkLog.LogWarning($"{nameof(NetworkPrefab)} {nameof(NetworkPrefab.OverridingTargetPrefab)} is null!");
556+
}
557+
removeEmptyPrefabs.Add(i);
558+
switch (NetworkConfig.NetworkPrefabs[i].Override)
559+
{
560+
case NetworkPrefabOverride.Hash:
561+
{
562+
Debug.LogWarning($"{nameof(NetworkPrefab)} override entry {NetworkConfig.NetworkPrefabs[i].SourceHashToOverride} will be removed and ignored.");
563+
break;
564+
}
565+
case NetworkPrefabOverride.Prefab:
566+
{
567+
Debug.LogWarning($"{nameof(NetworkPrefab)} override entry ({NetworkConfig.NetworkPrefabs[i].SourcePrefabToOverride.name}) will be removed and ignored.");
568+
break;
569+
}
570+
}
571+
continue;
572+
}
573+
else
574+
{
575+
targetPrefabGlobalObjectIdHash = NetworkConfig.NetworkPrefabs[i].OverridingTargetPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash;
576+
}
577+
}
578+
579+
// Assign the appropriate GlobalObjectIdHash to the appropriate NetworkPrefab
580+
if (!NetworkConfig.NetworkPrefabOverrideLinks.ContainsKey(sourcePrefabGlobalObjectIdHash))
581+
{
582+
if (NetworkConfig.NetworkPrefabs[i].Override == NetworkPrefabOverride.None)
583+
{
584+
NetworkConfig.NetworkPrefabOverrideLinks.Add(sourcePrefabGlobalObjectIdHash, NetworkConfig.NetworkPrefabs[i]);
585+
}
586+
else
587+
{
588+
if (!NetworkConfig.OverrideToNetworkPrefab.ContainsKey(targetPrefabGlobalObjectIdHash))
589+
{
590+
switch (NetworkConfig.NetworkPrefabs[i].Override)
591+
{
592+
case NetworkPrefabOverride.Prefab:
593+
{
594+
NetworkConfig.NetworkPrefabOverrideLinks.Add(sourcePrefabGlobalObjectIdHash, NetworkConfig.NetworkPrefabs[i]);
595+
NetworkConfig.OverrideToNetworkPrefab.Add(targetPrefabGlobalObjectIdHash, sourcePrefabGlobalObjectIdHash);
596+
}
597+
break;
598+
case NetworkPrefabOverride.Hash:
599+
{
600+
NetworkConfig.NetworkPrefabOverrideLinks.Add(sourcePrefabGlobalObjectIdHash, NetworkConfig.NetworkPrefabs[i]);
601+
NetworkConfig.OverrideToNetworkPrefab.Add(targetPrefabGlobalObjectIdHash, sourcePrefabGlobalObjectIdHash);
602+
}
603+
break;
523604
}
524-
break;
605+
}
606+
else
607+
{
608+
// This can happen if a user tries to make several GlobalObjectIdHash values point to the same target
609+
Debug.LogError($"{nameof(NetworkPrefab)} (\"{networkObject.name}\") has a duplicate {nameof(NetworkObject.GlobalObjectIdHash)} target entry value of: {targetPrefabGlobalObjectIdHash}! Removing entry from list!");
610+
removeEmptyPrefabs.Add(i);
611+
}
525612
}
526613
}
527614
else
528615
{
529616
// This should never happen, but in the case it somehow does log an error and remove the duplicate entry
530-
Debug.LogError($"{nameof(NetworkPrefab)} (\"{NetworkConfig.NetworkPrefabs[i].Prefab.name}\") has a duplicate {nameof(NetworkObject.GlobalObjectIdHash)} {networkObject.GlobalObjectIdHash} entry! Removing entry from list!");
617+
Debug.LogError($"{nameof(NetworkPrefab)} ({networkObject.name}) has a duplicate {nameof(NetworkObject.GlobalObjectIdHash)} source entry value of: {sourcePrefabGlobalObjectIdHash}! Removing entry from list!");
531618
removeEmptyPrefabs.Add(i);
532619
}
533620
}

‎com.unity.netcode.gameobjects/Tests/Runtime/NetworkPrefabHandlerTests.cs

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,56 @@ namespace Unity.Netcode.RuntimeTests
1515
/// </summary>
1616
public class NetworkPrefabHandlerTests
1717
{
18+
19+
private const string k_TestPrefabObjectName = "NetworkPrefabTestObject";
20+
private uint m_GlobalObjectIdHashBase = 123456;
21+
private GameObject MakeValidNetworkPrefab()
22+
{
23+
Guid baseObjectID = NetworkManagerHelper.AddGameNetworkObject(k_TestPrefabObjectName + m_GlobalObjectIdHashBase.ToString());
24+
NetworkObject validPrefab = NetworkManagerHelper.InstantiatedNetworkObjects[baseObjectID];
25+
MultiInstanceHelpers.MakeNetworkedObjectTestPrefab(validPrefab, m_GlobalObjectIdHashBase);
26+
m_GlobalObjectIdHashBase++;
27+
return validPrefab.gameObject;
28+
}
29+
1830
/// <summary>
19-
/// Tests the NetwokConfig NetworkPrefabs initialization during NetworkManager's Init method
31+
/// Tests the NetwokConfig NetworkPrefabs initialization during NetworkManager's Init method to make sure that
32+
/// it will still initialize but remove the invalid prefabs
2033
/// </summary>
2134
[Test]
2235
public void NetworkConfigInvalidNetworkPrefabTest()
2336
{
24-
var testPrefabObjectName = "NetworkPrefabHandlerTestObject";
25-
Guid baseObjectID = NetworkManagerHelper.AddGameNetworkObject(testPrefabObjectName);
26-
NetworkObject baseObject = NetworkManagerHelper.InstantiatedNetworkObjects[baseObjectID];
2737

2838
// Add null entry
2939
NetworkManagerHelper.NetworkManagerObject.NetworkConfig.NetworkPrefabs.Add(null);
3040

3141
// Add a NetworkPrefab with no prefab
3242
NetworkManagerHelper.NetworkManagerObject.NetworkConfig.NetworkPrefabs.Add(new NetworkPrefab());
3343

34-
var validNetworkPrefab = new NetworkPrefab();
35-
validNetworkPrefab.Prefab = baseObject.gameObject;
44+
// Add a NetworkPrefab override with an invalid hash
45+
NetworkManagerHelper.NetworkManagerObject.NetworkConfig.NetworkPrefabs.Add(new NetworkPrefab() { Override = NetworkPrefabOverride.Hash, SourceHashToOverride = 0 });
46+
47+
// Add a NetworkPrefab override with a valid hash but an invalid target prefab
48+
NetworkManagerHelper.NetworkManagerObject.NetworkConfig.NetworkPrefabs.Add(new NetworkPrefab() { Override = NetworkPrefabOverride.Hash, SourceHashToOverride = 654321, OverridingTargetPrefab = null });
49+
50+
// Add a NetworkPrefab override with a valid hash to override but an invalid target prefab
51+
NetworkManagerHelper.NetworkManagerObject.NetworkConfig.NetworkPrefabs.Add(new NetworkPrefab() { Override = NetworkPrefabOverride.Prefab, SourceHashToOverride = 654321, OverridingTargetPrefab = null });
52+
53+
// Add a NetworkPrefab override with an invalid source prefab to override
54+
NetworkManagerHelper.NetworkManagerObject.NetworkConfig.NetworkPrefabs.Add(new NetworkPrefab() { Override = NetworkPrefabOverride.Prefab, SourcePrefabToOverride = null });
55+
56+
// Add a NetworkPrefab override with a valid source prefab to override but an invalid target prefab
57+
NetworkManagerHelper.NetworkManagerObject.NetworkConfig.NetworkPrefabs.Add(new NetworkPrefab() { Override = NetworkPrefabOverride.Prefab, SourcePrefabToOverride = MakeValidNetworkPrefab(), OverridingTargetPrefab = null });
58+
59+
// Add a valid prefab
60+
NetworkManagerHelper.NetworkManagerObject.NetworkConfig.NetworkPrefabs.Add(new NetworkPrefab() { Prefab = MakeValidNetworkPrefab() });
61+
62+
// Add a NetworkPrefab override with a valid hash and valid target prefab
63+
NetworkManagerHelper.NetworkManagerObject.NetworkConfig.NetworkPrefabs.Add(new NetworkPrefab() { Override = NetworkPrefabOverride.Hash, SourceHashToOverride = 11111111, OverridingTargetPrefab = MakeValidNetworkPrefab() });
64+
65+
// Add a NetworkPrefab override with a valid prefab and valid target prefab
66+
NetworkManagerHelper.NetworkManagerObject.NetworkConfig.NetworkPrefabs.Add(new NetworkPrefab() { Override = NetworkPrefabOverride.Prefab, SourcePrefabToOverride = MakeValidNetworkPrefab(), OverridingTargetPrefab = MakeValidNetworkPrefab() });
3667

37-
//Add a valid prefab
38-
NetworkManagerHelper.NetworkManagerObject.NetworkConfig.NetworkPrefabs.Add(validNetworkPrefab);
3968
var exceptionOccurred = false;
4069
try
4170
{
@@ -47,6 +76,9 @@ public void NetworkConfigInvalidNetworkPrefabTest()
4776
}
4877

4978
Assert.False(exceptionOccurred);
79+
80+
// In the end we should only have 3 valid registered network prefabs
81+
Assert.True(NetworkManagerHelper.NetworkManagerObject.NetworkConfig.NetworkPrefabOverrideLinks.Count == 3);
5082
}
5183

5284
private const string k_PrefabObjectName = "NetworkPrefabHandlerTestObject";

0 commit comments

Comments
 (0)