Persistent sceneid (#563)

* add empty line

* persistent scene ids

* never assign scene ids at runtime

* fix warning

* fix 'sceneid is never assigned to' warning in play mode

* simplify bitwise operation

* set dirty even if only scene index byte changed

* improve log message

* assign build index byte in OnPostProcessScene to also work with unopened scenes and build index changes

* add comment

* improve log

* Add sceneid not set check

* Only PostProcess the objects from this scene.

* Interrupt build if there is an unopened scene that still needs sceneId assignments. This makes it 100% fail safe.

* improve comment and error message

* clear build index byte before or-ing into it
This commit is contained in:
vis2k 2019-03-08 20:24:18 +01:00 committed by GitHub
parent 557dee8234
commit 98a0e23413
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 139 additions and 174 deletions

View File

@ -215,7 +215,7 @@ void GetNetworkInformation(GameObject gameObject)
m_Info = new List<NetworkIdentityInfo>
{
GetAssetId(),
GetString("Scene ID", m_Identity.sceneId.ToString())
GetString("Scene ID", m_Identity.sceneId.ToString("X"))
};
if (!Application.isPlaying)

View File

@ -1,4 +1,3 @@
using System.Collections.Generic;
using System.Linq;
using UnityEditor;
using UnityEditor.Callbacks;
@ -9,168 +8,20 @@ namespace Mirror
{
public class NetworkScenePostProcess : MonoBehaviour
{
// persistent sceneId assignment to fix readstring bug that occurs when restarting the editor and
// connecting to a build again. sceneids were then different because FindObjectsOfType's order
// is not guranteed to be the same.
// -> we need something unique and persistent, aka always the same when pressing play/building the first time
// -> Unity has no built in unique id for GameObjects in the scene
// helper function to figure out a unique, persistent scene id for a GameObject in the hierarchy
// -> Unity's instanceId is unique but not persistent
// -> hashing the whole GameObject is not enough either since a duplicate would have the same hash
// -> we definitely need the transform sibling index in the hierarchy
// -> so we might as well use just that
// -> transforms have children too so we need a list of sibling indices like 0->3->5
public static List<int> SiblingPathFor(Transform t)
{
List<int> result = new List<int>();
while (t != null)
{
result.Add(t.GetSiblingIndex());
t = t.parent;
}
result.Reverse(); // parent to child instead of child to parent order
return result;
}
// we need to compare by using the whole sibling list
// comparing the string won't work work because:
// "1->2"
// "20->2"
// would compare '1' vs '2', then '-' vs '0'
//
// tests:
// CompareSiblingPaths(new List<int>(){0}, new List<int>(){0}) => 0
// CompareSiblingPaths(new List<int>(){0}, new List<int>(){1}) => -1
// CompareSiblingPaths(new List<int>(){1}, new List<int>(){0}) => 1
// CompareSiblingPaths(new List<int>(){0,1}, new List<int>(){0,2}) => -1
// CompareSiblingPaths(new List<int>(){0,2}, new List<int>(){0,1}) => 1
// CompareSiblingPaths(new List<int>(){1}, new List<int>(){0,1}) => 1
// CompareSiblingPaths(new List<int>(){1}, new List<int>(){2,1}) => -1
public static int CompareSiblingPaths(List<int> left, List<int> right)
{
// compare [0], remove it, compare next, etc.
while (left.Count > 0 && right.Count > 0)
{
if (left[0] < right[0])
{
return -1;
}
else if (left[0] > right[0])
{
return 1;
}
else
{
// equal, so they are both children of the same transform
// -> which also means that they both must have one more
// entry, so we can remove both without checking size
left.RemoveAt(0);
right.RemoveAt(0);
}
}
// equal if both were empty or both had the same entry without any
// more children (should never happen in practice)
return 0;
}
public static int CompareNetworkIdentitySiblingPaths(NetworkIdentity left, NetworkIdentity right)
{
return CompareSiblingPaths(SiblingPathFor(left.transform), SiblingPathFor(right.transform));
}
// we might have inactive scenes in the Editor's build settings, which
// aren't actually included in builds.
// so we have to only count the active ones when in Editor, otherwise
// editor and build sceneIds might get out of sync.
public static int GetSceneCount()
{
#if UNITY_EDITOR
return EditorBuildSettings.scenes.Count(scene => scene.enabled);
#else
return SceneManager.sceneCountInBuildSettings;
#endif
}
// helper function to check if a NetworkIdentity is in the active scene
static bool InActiveScene(NetworkIdentity identity) =>
identity.gameObject.scene == SceneManager.GetActiveScene();
[PostProcessScene]
public static void OnPostProcessScene()
{
// vis2k: MISMATCHING SCENEID BUG FIX
// problem:
// * FindObjectsOfType order is not guaranteed. restarting the
// editor results in a different order
// * connecting to a build again would cause Mirror to deserialize
// the wrong objects, causing all kinds of weird errors like
// 'ReadString out of range'
//
// solution:
// sort by sibling-index path, e.g. [0,1,2] vs [1]
// this is the only deterministic way to sort a list of objects in
// the scene.
// -> it's the same result every single time, even after restarts
//
// note: there is a reason why we 'sort by' sibling path instead of
// using it as sceneId directly. networkmanager etc. use Dont-
// DestroyOnLoad, which changes the hierarchy:
//
// World:
// NetworkManager
// Player
//
// ..becomes..
//
// World:
// Player
// DontDestroyOnLoad:
// NetworkManager
//
// so the player's siblingindex would be decreased by one.
// -> this is a problem because when building, OnPostProcessScene
// is called before any dontdestroyonload happens, but when
// entering play mode, it's called after
// -> hence sceneids would differ by one
//
// => but if we only SORT it, then it doesn't matter if one
// inbetween disappeared. as long as no NetworkIdentity used
// DontDestroyOnLoad.
//
// note: assigning a GUID in NetworkIdentity.OnValidate would be way
// cooler, but OnValidate isn't called for other unopened scenes
// when building or pressing play, so the bug would still happen
// there.
//
// note: this can still fail if DontDestroyOnLoad is called for a
// NetworkIdentity - but no one should ever do that anyway.
List<NetworkIdentity> identities = FindObjectsOfType<NetworkIdentity>().ToList();
identities.Sort(CompareNetworkIdentitySiblingPaths);
// sceneId assignments need to work with additive scene loading, so
// it can't always start at 1,2,3,4,..., otherwise there will be
// sceneId duplicates.
// -> we need an offset to start at 1000+1,+2,+3, etc.
// -> the most robust way is to split uint value range by sceneCount
// -> only if more than one scene. otherwise use offset 0 to avoid
// DivisionByZero if no scene in build settings, and to avoid
// different offsets in editor/build if scene wasn't added to
// build settings.
uint offsetPerScene = 0;
if (SceneManager.sceneCountInBuildSettings > 1)
{
offsetPerScene = uint.MaxValue / (uint)GetSceneCount();
// make sure that there aren't more sceneIds than offsetPerScene
// -> only if we have multiple scenes. otherwise offset is 0, in
// which case it doesn't matter.
if (identities.Count >= offsetPerScene)
{
Debug.LogWarning(">=" + offsetPerScene + " NetworkIdentities in scene. Additive scene loading will cause duplicate ids.");
}
}
uint nextSceneId = 1;
foreach (NetworkIdentity identity in identities)
// find all NetworkIdentities in this scene
// => but really only from this scene. this avoids weird situations
// like in NetworkZones when we destroy the local player and
// load another scene afterwards, yet the local player is still
// in the FindObjectsOfType result with scene=DontDestroyOnLoad
// for some reason
foreach (NetworkIdentity identity in FindObjectsOfType<NetworkIdentity>().Where(InActiveScene))
{
// if we had a [ConflictComponent] attribute that would be better than this check.
// also there is no context about which scene this is in.
@ -181,12 +32,21 @@ public static void OnPostProcessScene()
if (identity.isClient || identity.isServer)
continue;
uint offset = (uint)identity.gameObject.scene.buildIndex * offsetPerScene;
identity.ForceSceneId(offset + nextSceneId++);
if (LogFilter.Debug) Debug.Log("PostProcess sceneid assigned: name=" + identity.name + " scene=" + identity.gameObject.scene.name + " sceneid=" + identity.sceneId);
// valid scene id? then set build index byte
// otherwise it might be an unopened scene that still has null
// sceneIds. builds are interrupted if they contain 0 sceneIds,
// but it's still possible that we call LoadScene in Editor
// for a previously unopened scene.
// => throwing an exception would only show it for one object
// because this function would return afterwards.
if (identity.sceneId != 0)
{
identity.SetSceneIdSceneIndexByteInternal();
}
else Debug.LogError("Scene " + identity.gameObject.scene.path + " needs to be opened and resaved, because the scene object " + identity.name + " has no valid sceneId yet.");
// disable it AFTER assigning the sceneId.
// -> this way NetworkIdentity.OnDisable adds itself to the
// disable it
// note: NetworkIdentity.OnDisable adds itself to the
// spawnableObjects dictionary (only if sceneId != 0)
identity.gameObject.SetActive(false);

View File

@ -2,7 +2,6 @@
using System.Collections.Generic;
using System.Linq;
using UnityEngine;
#if UNITY_EDITOR
using UnityEditor;
#if UNITY_2018_3_OR_NEWER
@ -19,7 +18,6 @@ namespace Mirror
public sealed class NetworkIdentity : MonoBehaviour
{
// configuration
[SerializeField] uint m_SceneId;
[SerializeField] bool m_ServerOnly;
[SerializeField] bool m_LocalPlayerAuthority;
bool m_IsServer;
@ -90,6 +88,12 @@ internal set
}
}
// persistent scene id
[SerializeField] uint m_SceneId = 0;
// keep track of all sceneIds to detect scene duplicates
static Dictionary<uint, NetworkIdentity> sceneIds = new Dictionary<uint, NetworkIdentity>();
// used when adding players
internal void SetClientOwner(NetworkConnection conn)
{
@ -125,9 +129,6 @@ internal void ForceAuthority(bool authority)
public delegate void ClientAuthorityCallback(NetworkConnection conn, NetworkIdentity identity, bool authorityState);
public static ClientAuthorityCallback clientAuthorityCallback;
// only used when fixing duplicate scene IDs during post-processing
public void ForceSceneId(uint newSceneId) => m_SceneId = newSceneId;
// used when the player object for a connection changes
internal void SetNotLocalPlayer()
{
@ -184,30 +185,134 @@ bool ThisIsASceneObjectWithPrefabParent(out GameObject prefab)
return true;
}
// persistent sceneId assignment
// (because scene objects have no persistent unique ID in Unity)
//
// original UNET used OnPostProcessScene to assign an index based on
// FindObjectOfType<NetworkIdentity> order.
// -> this didn't work because FindObjectOfType order isn't deterministic.
// -> one workaround is to sort them by sibling paths, but it can still
// get out of sync when we open scene2 in editor and we have
// DontDestroyOnLoad objects that messed with the sibling index.
//
// we absolutely need a persistent id. challenges:
// * it needs to be 0 for prefabs
// => we set it to 0 in SetupIDs() if prefab!
// * it needs to be only assigned in edit time, not at runtime because
// only the objects that were in the scene since beginning should have
// a scene id.
// => Application.isPlaying check solves that
// * it needs to detect duplicated sceneIds after duplicating scene
// objects
// => sceneIds dict takes care of that
// * duplicating the whole scene file shouldn't result in duplicate
// scene objects
// => buildIndex is shifted into sceneId for that.
// => if we have no scenes in build index then it doesn't matter
// because by definition a build can't switch to other scenes
// => if we do have scenes in build index then it will be != -1
// note: the duplicated scene still needs to be opened once for it to
// be set properly
// * scene objects need the correct scene index byte even if the scene's
// build index was changed or a duplicated scene wasn't opened yet.
// => OnPostProcessScene is the only function that gets called for
// each scene before runtime, so this is where we set the scene
// byte.
// * disabled scenes in build settings should result in same scene index
// in editor and in build
// => .gameObject.scene.buildIndex filters out disabled scenes by
// default
// * generated sceneIds absolutely need to set scene dirty and force the
// user to resave.
// => Undo.RecordObject does that perfectly.
// * sceneIds should never be generated temporarily for unopened scenes
// when building, otherwise editor and build get out of sync
// => BuildPipeline.isBuildingPlayer check solves that
void AssignSceneID()
{
// we only ever assign sceneIds at edit time, never at runtime.
// by definition, only the original scene objects should get one.
// -> if we assign at runtime then server and client would generate
// different random numbers!
if (Application.isPlaying)
return;
// no valid sceneId yet, or duplicate?
NetworkIdentity existing;
bool duplicate = sceneIds.TryGetValue(m_SceneId, out existing) && existing != null && existing != this;
if (m_SceneId == 0 || duplicate)
{
// if a scene was never opened and we are building it, then a
// sceneId would be assigned to build but not saved in editor,
// resulting in them getting out of sync.
// => don't ever assign temporary ids. they always need to be
// permanent
// => throw an exception to cancel the build and let the user
// know how to fix it!
if (BuildPipeline.isBuildingPlayer)
throw new Exception("Scene " + gameObject.scene.path + " needs to be opened and resaved before building, because the scene object " + name + " has no valid sceneId yet.");
// if we generate the sceneId then we MUST be sure to set dirty
// in order to save the scene object properly. otherwise it
// would be regenerated every time we reopen the scene, and
// upgrading would be very difficult.
// -> Undo.RecordObject is the new EditorUtility.SetDirty!
// -> we need to call it before changing.
Undo.RecordObject(this, "Generated SceneId");
// generate random sceneId
// range: 3 bytes to fill 0x00FFFFFF
m_SceneId = (uint)UnityEngine.Random.Range(0, 0xFFFFFF);
Debug.Log(name + " in scene=" + gameObject.scene.name + " sceneId assigned to: " + m_SceneId.ToString("X") + (duplicate ? " because duplicated" : ""));
}
// add to sceneIds dict no matter what
// -> even if we didn't generate anything new, because we still need
// existing sceneIds in there to check duplicates
sceneIds[m_SceneId] = this;
}
// copy scene build index into sceneId for scene objects.
// this is the only way for scene file duplication to not contain
// duplicate sceneIds as it seems.
// -> sceneId before: 0x00AABBCCDD
// -> then we clear the left most byte, so that our 'OR' uses 0x00
// -> then we OR buildIndex into the 0x00 part
// => ONLY USE THIS FROM POSTPROCESSSCENE!
public void SetSceneIdSceneIndexByteInternal()
{
byte buildIndex = (byte)gameObject.scene.buildIndex;
m_SceneId = (m_SceneId & 0x00FFFFFF) | (uint)(buildIndex << 24);
Debug.Log(name + " in scene=" + gameObject.scene.name + " scene index byte(" + buildIndex.ToString("X") + ") copied into sceneId: " + m_SceneId.ToString("X"));
}
void SetupIDs()
{
if (ThisIsAPrefab())
{
ForceSceneId(0);
m_SceneId = 0; // force 0 for prefabs
AssignAssetID(gameObject);
}
else if (ThisIsASceneObjectWithPrefabParent(out GameObject prefab))
{
AssignSceneID();
AssignAssetID(prefab);
}
else if (PrefabStageUtility.GetCurrentPrefabStage() != null)
{
ForceSceneId(0);
m_SceneId = 0; // force 0 for prefabs
string path = PrefabStageUtility.GetCurrentPrefabStage().prefabAssetPath;
AssignAssetID(path);
}
else
{
AssignSceneID();
m_AssetId = "";
}
}
#endif
void OnDestroy()
{
if (m_IsServer && NetworkServer.active)