Skip to content

fix: networkmanager prefab validation and no scene management manual test #1073

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 18 commits into from
Aug 24, 2021

Conversation

NoelStephensUnity
Copy link
Collaborator

This fixes some issues with NetworkManager prefab override validation as well as includes some modifications to the EnableDisableSceneNetworkObject sample where EnableSceneManagement is disabled in order to validate this works as well.
Additional automated tests will be created in the updated NetworkSceneManager Tests contained within PR-1030.

Since this included some NetworkManager changes, I wanted to keep this PR separate.

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.
This validates the fixes as well as tests that disabling original/legacy scene management works.
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.
@NoelStephensUnity NoelStephensUnity enabled auto-merge (squash) August 20, 2021 01:21
NoelStephensUnity and others added 2 commits August 21, 2021 18:30
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
fixing comment and adding one of the potential invalid combinations (valid hash but invalid target prefab).
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.
@@ -465,7 +465,10 @@ private void Initialize(bool server)
// Build the NetworkPrefabOverrideLinks dictionary
for (int i = 0; i < NetworkConfig.NetworkPrefabs.Count; i++)
{
if (NetworkConfig.NetworkPrefabs[i] == null || NetworkConfig.NetworkPrefabs[i].Prefab == null)
var sourcePrefabGlobalObjectIdHash = (uint)0;
Copy link
Contributor

Choose a reason for hiding this comment

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

last but not least, I'd suggest to run ./standards.py --check and maybe ./standards.py --fix locally to check & fix spaces/namings locally — just in case :P

@jeffreyrainy jeffreyrainy self-requested a review August 24, 2021 16:37
@NoelStephensUnity NoelStephensUnity merged commit 904552f into develop Aug 24, 2021
@NoelStephensUnity NoelStephensUnity deleted the fix/prefabValidationNoSceneManagement branch August 24, 2021 18:42
SamuelBellomo added a commit that referenced this pull request Aug 26, 2021
…hub.com:Unity-Technologies/com.unity.multiplayer.mlapi into sam/feature/interpolation-for-network-transform

* 'sam/feature/interpolation-for-network-transform' of github.com:Unity-Technologies/com.unity.multiplayer.mlapi:
  test: NetworkTransformStateTests no longer uses ReplNetworkState (#1084)
  fix: networkmanager prefab validation and no scene management manual test (#1073)
  feat: snapshot. MTT-1088 Snapshot acknowledgment gaps (#1083)
  feat: Add a test to validate registration of metric types (#1072)
  chore!: Remove unsupported UNET Relay behavior (MTT-1000) (#1081)
  fix: 2+ inheritance from network behaviour causes compilation exception (#1078) (#1079)
  test: add networkscenemanager additive scene loading tests (#1076)
mollstam pushed a commit to Keepsake-Games/com.unity.netcode.gameobjects that referenced this pull request Feb 13, 2023
…test (Unity-Technologies#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants