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: GlobalObjectIdHash generation for already existing in-scene placed prefab instances [MTT-7055] #2707

Conversation

NoelStephensUnity
Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity commented Sep 19, 2023

This is a fix for handling the scenario where several in-scene placed instances of a normal prefab might already exist in one or more scenes (i.e. an already existing single player project that the user wants to make multiplayer) and the user wishes to make the source/original prefab a network prefab by adding a NetworkObject component to it.

MTT-7055

fix: #2644

Changelog

  • Added: A context menu tool that provides users with the ability to quickly update the GlobalObjectIdHash value for all in-scene placed prefab instances that were created prior to adding a NetworkObject component to it.

Testing and Documentation

  • Includes adjustment to internal automated GlobalObjectIdTest (PR-4)
  • Additional NGO documentation updates (PR-1120)

Additional Information

Context menu was added to NetworkObject that allows one to refresh all in-scene placed prefab instances in any of the enabled scenes in the build list:
image

WIP to resolve MTT-7055.
adding comment
Left out including using UnityEditor.SceneManagement for earlier builds
Cleaned up some of the code.
Put GlobalObjectIdHash back to internal.
Added additional comments for draft POC review.
removing unintentional CR/LF
Split apart the NetworkObjectRefreshTool from NetworkObject.
Made some updates that don't require any form of editor application update.
Added script in NetworkObject.RefreshAllPrefabInstances context menu method that handles refreshing the currently active scene and all enabled scenes in the build list.
Removing accidental addition of a test prefab.
Adding dialog notification when attempting to do a NetworkObject Refresh on an in-scene placed prefab instance as opposed to a prefab instance.
updates for the manual test
@NoelStephensUnity NoelStephensUnity marked this pull request as ready for review September 21, 2023 22:33
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner September 21, 2023 22:33
adding change log entry
// 1 = Imported Asset
// 2 = Scene Object
// 3 = Source Asset.
var objetType = 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can these types be an enum something similar?

Copy link
Collaborator Author

@NoelStephensUnity NoelStephensUnity Sep 25, 2023

Choose a reason for hiding this comment

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

Unfortunately, that is actually part of the format for the GlobalObjectId that is generated by GlobalObjectId.GetGlobalObjectIdSlow. I added those comments to remind anyone what the values for that section of the GlobalObjectId is:

The format of the string representation of the ID is "GlobalObjectId_V1-{i}-{a}-{l}-{p}" where:
{i} is the identifier type represented by an integer (0 = Null, 1 = Imported Asset, 2 = Scene Object, 3 = Source Asset).
{a} is the asset GUID.
{l} is the local file ID of the object. For objects inside a prefab instance this is the local file ID of the source object in the prefab.
{p} is the local file ID of the prefab instance of the object (or 0 when the object is not part of a prefab instance).

How about I make those constants?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah constants would be good:

var objectType = k_GlobalObjectIdObjectType_SourceAsset;

would look much better, or a define or whatever :)

Copy link
Collaborator Author

@NoelStephensUnity NoelStephensUnity Sep 25, 2023

Choose a reason for hiding this comment

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

Just pushed an update that applies this suggested modification.

Added object type identifier constants for code clarity purposes.
@NoelStephensUnity NoelStephensUnity merged commit c84a1dc into develop Oct 2, 2023
1 check passed
@NoelStephensUnity NoelStephensUnity deleted the fix/globalobjectidhash-generation-pre-serialization-MTT-7055 branch October 2, 2023 19:41
NoelStephensUnity added a commit that referenced this pull request Oct 5, 2023
…ed prefab instances [MTT-7055] (#2707)

* update

Resolves MTT-7055.
Split apart the NetworkObjectRefreshTool from NetworkObject.
Made some updates that don't require any form of editor application update.
Added script in NetworkObject.RefreshAllPrefabInstances context menu method that handles refreshing the currently active scene and all enabled scenes in the build list.
Added dialog notification when attempting to do a NetworkObject Refresh on an in-scene placed prefab instance as opposed to a prefab instance.

* test

Made some minor updates for the manual test

* update

adding change log entry

* style

Added object type identifier constants for code clarity purposes.
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.

GlobalObjectIdHash collisions occur when a prefab has instances in multiple scenes before adding NetworkObject
2 participants