Skip to content

Experimental/singleton removal merge #495

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.

Sign up for GitHub

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

Closed
wants to merge 10 commits into from
Closed

Experimental/singleton removal merge #495

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bc9cef3
first pass on singleton removal work. I have gotten things as far as …
dwoodruffsf Jan 29, 2021
d4579ba
setting up example of server and client connecting in process
dwoodruffsf Feb 11, 2021
e6bb49e
Fix: Allowing singleton to work with uNET
wackoisgod Feb 23, 2021
8085b11
singleton fixups
mattwalsh-unity Feb 23, 2021
6c41534
merge branch 'develop' into 'experimental/singleton-removal-merge'
0xFA11 Feb 24, 2021
35f3d20
restore netowrked animator headers, etc.
mattwalsh-unity Feb 26, 2021
d961ec1
linked networked manager to transport
mattwalsh-unity Feb 27, 2021
aa43072
hack checkpoint
mattwalsh-unity Feb 27, 2021
1537ab6
one more netmanager dependency removal
mattwalsh-unity Feb 28, 2021
5498470
more misc fixups
mattwalsh-unity Mar 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 32 additions & 30 deletions com.unity.multiplayer.mlapi/Editor/MLAPIProfiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Collections.Generic;
using System.Collections.Generic;
using System.IO;
using MLAPI.Profiling;
using MLAPI.Serialization;
Expand Down Expand Up @@ -40,36 +40,38 @@ class ProfilerContainer
public ProfilerTick[] ticks;

public byte[] ToBytes()
{
BitStream stream = new BitStream();
BitWriter writer = new BitWriter(stream);
writer.WriteUInt16Packed((ushort)ticks.Length);

for (int i = 0; i < ticks.Length; i++)
{
ticks[i].SerializeToStream(stream);
}

return stream.ToArray();
}

public static ProfilerContainer FromBytes(byte[] bytes)
{
ProfilerContainer container = new ProfilerContainer();
BitStream stream = new BitStream(bytes);
BitReader reader = new BitReader(stream);
ushort count = reader.ReadUInt16Packed();
container.ticks = new ProfilerTick[count];
{
BitStream stream = new BitStream();
BitWriter writer = new BitWriter(stream);
writer.WriteUInt16Packed((ushort)ticks.Length);

for (int i = 0; i < ticks.Length; i++)
{
ticks[i].SerializeToStream(stream);
}

return stream.ToArray();
}

public static ProfilerContainer FromBytes(byte[] bytes)
{
MLAPI.NetworkingManager manager = NetworkingManagerEditor.GetAnyNetworkingManager();

ProfilerContainer container = new ProfilerContainer();
BitStream stream = new BitStream(bytes);
BitReader reader = new BitReader(manager, stream);
ushort count = reader.ReadUInt16Packed();
container.ticks = new ProfilerTick[count];
for (int i = 0; i < count; i++)
{
container.ticks[i] = ProfilerTick.FromStream(stream);
}
{
container.ticks[i] = ProfilerTick.FromStream(manager, stream);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same story here, in the FromStream lambda, you can first read the NM id, then de-ref and not need to pass this in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also was able to break this dep same was as above

}

return container;
}
}
return container;
}
}

private void StopRecording()
private void StopRecording()
{
NetworkProfiler.Stop();
}
Expand Down Expand Up @@ -124,7 +126,7 @@ private void OnGUI()
string path = EditorUtility.OpenFilePanel("Choose a NetworkProfiler file", "", "");
if (!string.IsNullOrEmpty(path))
{
ProfilerTick[] ticks = ProfilerContainer.FromBytes(File.ReadAllBytes(path)).ticks;
ProfilerTick[] ticks = ProfilerContainer.FromBytes(File.ReadAllBytes(path)).ticks;
if (ticks.Length >= 2)
{
curve = AnimationCurve.Constant(ticks[0].EventId, ticks[(ticks.Length - 1)].EventId, 0);
Expand Down Expand Up @@ -160,7 +162,7 @@ private void OnGUI()
ProfilerTick[] ticks = new ProfilerTick[ticksInRange];
for (int i = min; i < max; i++) ticks[i - min] = currentTicks[i];
string path = EditorUtility.SaveFilePanel("Save NetworkProfiler data", "", "networkProfilerData", "");
if (!string.IsNullOrEmpty(path)) File.WriteAllBytes(path, new ProfilerContainer() { ticks = ticks }.ToBytes());
if (!string.IsNullOrEmpty(path)) File.WriteAllBytes(path, new ProfilerContainer() { ticks = ticks }.ToBytes());
}

EditorGUILayout.EndHorizontal();
Expand Down
12 changes: 6 additions & 6 deletions com.unity.multiplayer.mlapi/Editor/NetworkedAnimatorEditor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
using System;
using MLAPI.Prototyping;
using UnityEditor.Animations;
using UnityEngine;
//using System;
//using MLAPI.Prototyping;
//using UnityEditor.Animations;
//using UnityEngine;

namespace UnityEditor
{
[CustomEditor(typeof(NetworkedAnimator), true)]
[CanEditMultipleObjects]
//[CustomEditor(typeof(NetworkedAnimator), true)]
//[CanEditMultipleObjects]
public class NetworkAnimatorEditor : Editor
{
// TODO @mfatihmar (Unity): Re-implement this after `NetworkedAnimator` re-implementation
Expand Down
6 changes: 4 additions & 2 deletions com.unity.multiplayer.mlapi/Editor/NetworkedBehaviourEditor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Reflection;
using MLAPI;
Expand Down Expand Up @@ -91,7 +91,9 @@ void RenderNetworkedVarValueType<T>(int index) where T : struct
object val = var.Value;
string name = networkedVarNames[index];

if (NetworkingManager.Singleton != null && NetworkingManager.Singleton.IsListening)
NetworkingManager manager = NetworkingManagerEditor.GetNetworkingManager(true);

if (manager != null && manager.IsListening)
{
if (type == typeof(int))
val = EditorGUILayout.IntField(name, (int)val);
Expand Down
18 changes: 11 additions & 7 deletions com.unity.multiplayer.mlapi/Editor/NetworkedObjectEditor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@ private void Init()
networkedObject = (NetworkedObject)target;
}

//FIXME:SINGLETONCONVERSION. What is the proper way to get the NetworkingManager in the active scene in editor code?
private bool IsServer { get { return false; } }

public override void OnInspectorGUI()
{
Init();

if (!networkedObject.IsSpawned && NetworkingManager.Singleton != null && NetworkingManager.Singleton.IsServer)
if (!networkedObject.IsSpawned && IsServer )
{
EditorGUILayout.BeginHorizontal();
EditorGUILayout.LabelField(new GUIContent("Spawn", "Spawns the object across the network"));
Expand All @@ -45,11 +48,11 @@ public override void OnInspectorGUI()
EditorGUILayout.LabelField("IsSpawned: ", networkedObject.IsSpawned.ToString(), EditorStyles.label);
EditorGUILayout.LabelField("IsLocalPlayer: ", networkedObject.IsLocalPlayer.ToString(), EditorStyles.label);
EditorGUILayout.LabelField("IsOwner: ", networkedObject.IsOwner.ToString(), EditorStyles.label);
EditorGUILayout.LabelField("IsOwnedByServer: ", networkedObject.IsOwnedByServer.ToString(), EditorStyles.label);
EditorGUILayout.LabelField("IsOwnedByServer: ", networkedObject.IsOwnedByServer.ToString(), EditorStyles.label);
EditorGUILayout.LabelField("IsPlayerObject: ", networkedObject.IsPlayerObject.ToString(), EditorStyles.label);
EditorGUILayout.LabelField("IsSceneObject: ", (networkedObject.IsSceneObject == null ? "Null" : networkedObject.IsSceneObject.Value.ToString()), EditorStyles.label);

if (NetworkingManager.Singleton != null && NetworkingManager.Singleton.IsServer)
if (IsServer)
{
showObservers = EditorGUILayout.Foldout(showObservers, "Observers");

Expand All @@ -61,10 +64,11 @@ public override void OnInspectorGUI()

while (observerClientIds.MoveNext())
{
if (NetworkingManager.Singleton.ConnectedClients[observerClientIds.Current].PlayerObject != null)
EditorGUILayout.ObjectField("ClientId: " + observerClientIds.Current, NetworkingManager.Singleton.ConnectedClients[observerClientIds.Current].PlayerObject, typeof(GameObject), false);
else
EditorGUILayout.TextField("ClientId: " + observerClientIds.Current, EditorStyles.label);
//FIXME:SINGLETONCONVERSION
//if (NetworkingManager.Singleton.ConnectedClients[observerClientIds.Current].PlayerObject != null)
// EditorGUILayout.ObjectField("ClientId: " + observerClientIds.Current, NetworkingManager.Singleton.ConnectedClients[observerClientIds.Current].PlayerObject, typeof(GameObject), false);
//else
// EditorGUILayout.TextField("ClientId: " + observerClientIds.Current, EditorStyles.label);
}

EditorGUI.indentLevel -= 1;
Expand Down
27 changes: 25 additions & 2 deletions com.unity.multiplayer.mlapi/Editor/NetworkingManagerEditor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,29 @@ public class NetworkingManagerEditor : Editor
private readonly List<Type> transportTypes = new List<Type>();
private string[] transportNames = new string[] { "Select transport..." };

/// <summary>
/// Helper method that gets either the server or non-server NetworkingManager in the scene, or null if not present.
/// </summary>
public static NetworkingManager GetNetworkingManager(bool isServer)
{
foreach (var manager in FindObjectsOfType<NetworkingManager>())
{
if (manager.IsServer == isServer)
Copy link
Contributor

Choose a reason for hiding this comment

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

So I am confused by the IsServer check because from what I can tell its always false? https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/pull/495/files#diff-641f8f5b259017aefa68498910442cb599d894ed91c96e4811d0a79b5f0151feR24

Copy link
Contributor

Choose a reason for hiding this comment

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

no, that's for the NetworkedObjectEditor.cs in the Editor, not at Runtime I believe, no?

{
return manager;
}
}
return null;
}

/// <summary>
/// Gets the first NetworkingManager in the scene, of any type.
/// </summary>
public static NetworkingManager GetAnyNetworkingManager()
{
return FindObjectOfType<NetworkingManager>();
Copy link
Contributor

Choose a reason for hiding this comment

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

While I get this is just editor code FindObjectOfType is one of the worst functions ever! Also assume we have more than one manager in a scene that has different settings does that matter ?

Copy link
Contributor

Choose a reason for hiding this comment

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

We had similar discussions over at the RFC-side in case you'd like to contribute :)

}

private void ReloadTransports()
{
transportTypes.Clear();
Expand Down Expand Up @@ -97,7 +120,7 @@ private void Init()
// Base properties
dontDestroyOnLoadProperty = serializedObject.FindProperty("DontDestroy");
runInBackgroundProperty = serializedObject.FindProperty("RunInBackground");
logLevelProperty = serializedObject.FindProperty("LogLevel");
logLevelProperty = serializedObject.FindProperty("LogLevelLocal");
networkConfigProperty = serializedObject.FindProperty("NetworkConfig");

// NetworkConfig properties
Expand Down Expand Up @@ -137,7 +160,7 @@ private void CheckNullProperties()
// Base properties
dontDestroyOnLoadProperty = serializedObject.FindProperty("DontDestroy");
runInBackgroundProperty = serializedObject.FindProperty("RunInBackground");
logLevelProperty = serializedObject.FindProperty("LogLevel");
logLevelProperty = serializedObject.FindProperty("LogLevelLocal");
networkConfigProperty = serializedObject.FindProperty("NetworkConfig");

// NetworkConfig properties
Expand Down
8 changes: 6 additions & 2 deletions com.unity.multiplayer.mlapi/Editor/TrackedObjectEditor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using MLAPI;
using MLAPI;
using MLAPI.LagCompensation;

namespace UnityEditor
Expand All @@ -23,7 +23,11 @@ public override void OnInspectorGUI()
{
Init();
base.OnInspectorGUI();
if(NetworkingManager.Singleton != null && NetworkingManager.Singleton.IsServer)

//FIXME: Singleton Conversion, get all of them and be sure to get the server!
NetworkingManager manager = NetworkingManagerEditor.GetNetworkingManager(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

So I think generally in the non-singleton case I would expect the editors to show only useful data if the actual object being selected is a NetworkManager... I just don't think that picking the first thing you see if actually useful.

Also FindObjectOfType doesn't guarantee order so you could call it more than once and get different objects which doesn't seem like the desired behavior either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is probably the #1 unresolved issue in this addition

Copy link
Contributor

Choose a reason for hiding this comment

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

I called out something similar on the RFC-side, inlining my suggestions here too:

Rather than finding NetworkManager with GameObject.FindWithTag, it might be useful to have a static NetworkManager[] array globally where we could quickly iterate over and find the one we're looking for.

Similar to what I said above, we could store NetworkManager instances in a static NetworkManager[] container to avoid Object.FindObjectOfType call.

(response to Matt's Dictionary idea)
oh, static array or hashmap, the main idea being avoiding gameobject hierarchy traversal with Object.FindObjectOfType because seems like it could be unnecessary with a faster centralized static container lookup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think one way or another we'll want a central table of NMs


if(manager != null )
{
EditorGUILayout.LabelField("Total points: ", trackedObject.TotalPoints.ToString(), EditorStyles.label);
EditorGUILayout.LabelField("Avg time between points: ", trackedObject.AvgTimeBetweenPointsMs.ToString() + " ms", EditorStyles.label);
Expand Down
10 changes: 5 additions & 5 deletions com.unity.multiplayer.mlapi/Prototyping/NetworkedNavMeshAgent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Collections.Generic;
using System.Collections.Generic;
using MLAPI.Connection;
using MLAPI.Messaging;
using UnityEngine;
Expand Down Expand Up @@ -64,7 +64,7 @@ private void Update()
else
{
List<ulong> proximityClients = new List<ulong>();
foreach (KeyValuePair<ulong, NetworkedClient> client in NetworkingManager.Singleton.ConnectedClients)
foreach (KeyValuePair<ulong, NetworkedClient> client in NetManager.ConnectedClients)
{
if (client.Value.PlayerObject == null || Vector3.Distance(client.Value.PlayerObject.transform.position, transform.position) <= ProximityRange)
proximityClients.Add(client.Key);
Expand All @@ -75,7 +75,7 @@ private void Update()
}
}

if (NetworkingManager.Singleton.NetworkTime - lastCorrectionTime >= CorrectionDelay)
if (NetManager.NetworkTime - lastCorrectionTime >= CorrectionDelay)
{
if (!EnableProximity)
{
Expand All @@ -84,7 +84,7 @@ private void Update()
else
{
List<ulong> proximityClients = new List<ulong>();
foreach (KeyValuePair<ulong, NetworkedClient> client in NetworkingManager.Singleton.ConnectedClients)
foreach (KeyValuePair<ulong, NetworkedClient> client in NetManager.ConnectedClients)
{
if (client.Value.PlayerObject == null || Vector3.Distance(client.Value.PlayerObject.transform.position, transform.position) <= ProximityRange)
proximityClients.Add(client.Key);
Expand All @@ -94,7 +94,7 @@ private void Update()
new ClientRpcParams {Send = new ClientRpcSendParams {TargetClientIds = proximityClients.ToArray()}});
}

lastCorrectionTime = NetworkingManager.Singleton.NetworkTime;
lastCorrectionTime = NetManager.NetworkTime;
}
}

Expand Down
Loading