Skip to content
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

fix: Cannot Create Multiple Network Prefab Overrides That Target Same Network Prefab as Override [MTT-7399] #2710

Merged
merged 39 commits into from
Dec 12, 2023

Conversation

NoelStephensUnity
Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity commented Sep 25, 2023

This resolves the issue where you could not have multiple source network prefab overrides targeting the same network prefab as their override.

MTT-7399

** Includes additional spawning sample and integration test specific assets (required for integration test)

Discovered when supporting user requests about NGO documentation in Isssue-1053

Addeded/Changed Functionality Update

  • Added NetworkSpawnManafer.InstantiateAndSpawn that simplifies the spawning of a prefab asset without having to worry about instantiating or making sure to get the overridden version of the prefab when spawning.
    • The included "SimpleSpawn" sample demonstrates how this method simplifies the instantiate and spawn process.
  • This update includes automatic linking of in-scene placed NetworkObjects to their source prefab asset's GlobalObjetIdHash which removes the high user pain associated with having to create a prefab override per in-scene placed network prefab instance within the NetworkPrefabList.
    • This is specifically useful for when scene management is disabled as it removes the requirement to create a prefab override per in-scene placed NetworkObject.
    • This also assures that a prefab with a prefab with override can still be used as an in-scene placed NetworkObject without the client spawning the override when scene management is disabled while also still being able to dynamically spawn the same prefab and still yielding the desired override target prefab.
  • Increased the default SpawnTimeout to 10s due several user reported issues with messages being dropped prior to a client finishing synchronization.

Changelog

  • Added: NetworkObject.InstantiateAndSpawn and NetworkSpawnManager.InstantiateAndSpawn methods to simplify prefab spawning by assuring that the prefab is valid and applies any override prior to instantiating the GameObject and spawning the NetworkObject instance.
  • Fixed: Issue where you could not have multiple source network prefab overrides targeting the same network prefab as their override.
  • Changed: NetworkPrefabs.OverrideToNetworkPrefab dictionary is no longer used/populated due to it ending up being related to a regression bug and not allowing more than one override to be assigned to a network prefab asset.
  • Changed: In-scene placed NetworkObjects now store their source network prefab asset's GlobalObjectIdHash internally that is used, when scene management is disabled, by clients to spawn the correct prefab even if the NetworkPrefab entry has an override. This does not impact dynamically spawning the same prefab which will yield the override on both host and client.
  • Changed: In-scene placed NetworkObjects no longer require a NetworkPrefab entry with GlobalObjectIdHash override in order for clients to properly synchronize.
  • Changed: In-scene placed NetworkObjects now set their IsSceneObject value when generating their GlobalObjectIdHash value.
  • Changed: Increased the default NetworkConfig.SpawnTimeout value from 1.0s to 10.0s.

Testing and Documentation

  • Includes Test Project Sample/Manual Test: "SimpleSpawn"
  • Includes integration test: PrefabExtendedTests.TestPrefabsSpawning
  • Documentation Updates: PR-1142

WIP fix for prefab overrides
Removing restriction on PrefabIdHash.
No longer using OverrideToNetworkPrefab.
Removing remains of OverrideToNetworkPrefab
Putting back the NetworkPrefabs.OverrideToNetworkPrefab to avoid validation issues.
Removing commented out code.
Adding a new method: NetworkSpawnManager.InstantiateAndSpawn that handles several steps for users. Basically passing in a prefab NetworkObject will instantiate, spawn, and then return the spawned prefab instance. This takes into account prefab overrides.

Added a manual test "SimpleSpawn" to manually validate the updates.
spelling in xml API
minor rewording
expanded upon the GetNetworkObjectToSpawn  description
fixing namespace ordinal ordering
Added some additional improvements to the simple spawn sample/manual test that also provides an example of the legacy/alternate way to spawn prefabs along with unique prefabs that are named appropriately for each scenario while also setting the color of the NetworkObjectId label to the assigned owner/client assigned color.
This update includes automatic linking of in-scene placed NetworkObjects to their source prefab asset. This is specifically useful for when scene management is disabled as it removes the requirement to create a prefab override per in-scene placed NetworkObject. This also assures that a prefab with an override can still be used as an in-scene placed NetworkObject without the client spawning the overide when scene management is disabled while also still spawing the override of the same prefab when dynamically spawned.

This also includes an integration test to validate both with and without scene management enabled that clients synchronize to the proper prefab instance.
Reverting the NetworkPrefabs change and just using the SourcePrefabToOverride.
Renamed InScenePlacedSourceGlobalObjectId to InScenePlacedSourceGlobalObjectIdHash to avoid confusion.
Updated the PrefabExtendedTests and test assets to include the scenario that is not handled when scnee management is disabled (i.e. in-scene defined NetworkObjects).
null check standard adjustment
Adding logic to only spawn override (as a default) when running as a host.
@NoelStephensUnity NoelStephensUnity marked this pull request as ready for review November 19, 2023 23:40
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner November 19, 2023 23:40
Repopulating the override dictionary to avoid breaking any user's project that utilizes it.
Added NetworkObject.InstantiateAndSpawn method.
Adding change log entries.
@NoelStephensUnity NoelStephensUnity enabled auto-merge (squash) December 8, 2023 19:15
Updating how errors and warning messages are handled.
Validating that all error messages occur when expected.
removing CR and adding an s to the end of TestsInstantiateAndSpawnErrors
if (OverrideToNetworkPrefab.ContainsKey(target))
{
// If so, just return false and don't attempt to add another.
return false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Weren't we going to leave this check out?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😞
Yes... it is in the wrong location... need to write another test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok... check the update... that is where that check should be.

Adding back the OverrideToNetworkPrefab to continue to support the legacy manual instantiate and spawn usage pattern.

Updated comments to reflect this and to point users to use InstantiateAndSpawn methods.
Adding a static version of InstantiateAndSpawn to NetworkObject.
Adding a test pass for validating that the legacy instantiate and spawn usage pattern still works.
@NoelStephensUnity NoelStephensUnity enabled auto-merge (squash) December 11, 2023 22:11
@NoelStephensUnity NoelStephensUnity merged commit 3fd2bf5 into develop Dec 12, 2023
23 checks passed
@NoelStephensUnity NoelStephensUnity deleted the fix/prefab-override-hash-value branch December 12, 2023 16:06
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.

3 participants