charleslemaux/Mirror
1
0
Fork 0
mirror of https://github.com/MirrorNetworking/Mirror.git synced 2024-11-18 02:50:32 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

fix: #3315 ClientToServer SyncList support (#3320)

Browse Source 
* SyncDirection demo: add SyncList test

* resaved prefab

* update error message

* logs

* disable AddOperation ReadOnly checks for now

* logs

* IsRecording adjusted to client

* kcp: larger timeout for debugging

* WORKS but not very clean yet

* update comments

* IsReadOnly moved to SyncObject

* IsWriteable lambda

* better

* fix tests

* fix host mode

* tests

* tests

* throw if undefined behaviour

* fix test

* remove logs
This commit is contained in:
mischa 2022-12-26 10:39:55 +01:00 committed by GitHub
parent 0b4448a0f4
commit 6cf03d60a3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 173 additions and 101 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
Assets/Mirror/Core/NetworkBehaviour.cs
View File

@ -221,12 +221,38 @@ protected void InitSyncObject(SyncObject syncObject)
ulong nthBit = 1UL << index;
syncObject.OnDirty = () => SetSyncObjectDirtyBit(nthBit);
// only record changes while we have observers.
// who is allowed to modify SyncList/SyncSet/etc.:
// on client: only if owned ClientToserver
// on server: only if ServerToClient.
// but also for initial state when spawning.
// need to set a lambda because 'isClient' isn't available in
// InitSyncObject yet, which is called from the constructor.
syncObject.IsWritable = () =>
{
// check isServer first.
// if we check isClient first, it wouldn't work in host mode.
if (isServer) return syncDirection == SyncDirection.ServerToClient;
if (isClient) return isOwned;
// undefined behaviour should throw to make it very obvious
throw new Exception("InitSyncObject: neither isServer nor isClient are true.");
};
// when do we record changes:
// on client: only if owned ClientToServer
// on server: only if we have observers.
// prevents ever growing .changes lists:
// if a monster has no observers but we keep modifing a SyncObject,
// then the changes would never be flushed and keep growing,
// because OnSerialize isn't called without observers.
syncObject.IsRecording = () => netIdentity.observers.Count > 0;
syncObject.IsRecording = () =>
{
// check isServer first.
// if we check isClient first, it wouldn't work in host mode.
if (isServer) return netIdentity.observers.Count > 0;
if (isClient) return isOwned;
// undefined behaviour should throw to make it very obvious
throw new Exception("InitSyncObject: neither isServer nor isClient are true.");
};
}
// pass full function name to avoid ClassA.Func <-> ClassB.Func collisions

56
Assets/Mirror/Core/SyncDictionary.cs
View File

@ -10,7 +10,7 @@ public class SyncIDictionary<TKey, TValue> : SyncObject, IDictionary<TKey, TValu
protected readonly IDictionary<TKey, TValue> objects;
public int Count => objects.Count;
public bool IsReadOnly { get; private set; }
public bool IsReadOnly => !IsWritable();
public event SyncDictionaryChanged Callback;
public enum Operation : byte
@ -43,7 +43,6 @@ struct Change
public override void Reset()
{
IsReadOnly = false;
changes.Clear();
changesAhead = 0;
objects.Clear();
@ -66,11 +65,11 @@ public SyncIDictionary(IDictionary<TKey, TValue> objects)
this.objects = objects;
}
void AddOperation(Operation op, TKey key, TValue item)
void AddOperation(Operation op, TKey key, TValue item, bool checkAccess)
{
if (IsReadOnly)
if (checkAccess && IsReadOnly)
{
throw new System.InvalidOperationException("SyncDictionaries can only be modified by the server");
throw new System.InvalidOperationException("SyncDictionaries can only be modified by the owner.");
}
Change change = new Change
@ -133,9 +132,6 @@ public override void OnSerializeDelta(NetworkWriter writer)
public override void OnDeserializeAll(NetworkReader reader)
{
// This list can now only be modified by synchronization
IsReadOnly = true;
// if init, write the full list content
int count = (int)reader.ReadUInt();
@ -157,9 +153,6 @@ public override void OnDeserializeAll(NetworkReader reader)
public override void OnDeserializeDelta(NetworkReader reader)
{
// This list can now only be modified by synchronization
IsReadOnly = true;
int changesCount = (int)reader.ReadUInt();
for (int i = 0; i < changesCount; i++)
@ -180,7 +173,20 @@ public override void OnDeserializeDelta(NetworkReader reader)
item = reader.Read<TValue>();
if (apply)
{
objects[key] = item;
// add dirty + changes.
// ClientToServer needs to set dirty in server OnDeserialize.
// no access check: server OnDeserialize can always
// write, even for ClientToServer (for broadcasting).
if (ContainsKey(key))
{
objects[key] = item; // assign after ContainsKey check
AddOperation(Operation.OP_SET, key, item, false);
}
else
{
objects[key] = item; // assign after ContainsKey check
AddOperation(Operation.OP_ADD, key, item, false);
}
}
break;
@ -188,6 +194,11 @@ public override void OnDeserializeDelta(NetworkReader reader)
if (apply)
{
objects.Clear();
// add dirty + changes.
// ClientToServer needs to set dirty in server OnDeserialize.
// no access check: server OnDeserialize can always
// write, even for ClientToServer (for broadcasting).
AddOperation(Operation.OP_CLEAR, default, default, false);
}
break;
@ -196,7 +207,14 @@ public override void OnDeserializeDelta(NetworkReader reader)
item = reader.Read<TValue>();
if (apply)
{
objects.Remove(key);
if (objects.Remove(key))
{
// add dirty + changes.
// ClientToServer needs to set dirty in server OnDeserialize.
// no access check: server OnDeserialize can always
// write, even for ClientToServer (for broadcasting).
AddOperation(Operation.OP_REMOVE, key, item, false);
}
}
break;
}
@ -216,7 +234,7 @@ public override void OnDeserializeDelta(NetworkReader reader)
public void Clear()
{
objects.Clear();
AddOperation(Operation.OP_CLEAR, default, default);
AddOperation(Operation.OP_CLEAR, default, default, true);
}
public bool ContainsKey(TKey key) => objects.ContainsKey(key);
@ -225,7 +243,7 @@ public bool Remove(TKey key)
{
if (objects.TryGetValue(key, out TValue item) && objects.Remove(key))
{
AddOperation(Operation.OP_REMOVE, key, item);
AddOperation(Operation.OP_REMOVE, key, item, true);
return true;
}
return false;
@ -239,12 +257,12 @@ public TValue this[TKey i]
if (ContainsKey(i))
{
objects[i] = value;
AddOperation(Operation.OP_SET, i, value);
AddOperation(Operation.OP_SET, i, value, true);
}
else
{
objects[i] = value;
AddOperation(Operation.OP_ADD, i, value);
AddOperation(Operation.OP_ADD, i, value, true);
}
}
}
@ -254,7 +272,7 @@ public TValue this[TKey i]
public void Add(TKey key, TValue value)
{
objects.Add(key, value);
AddOperation(Operation.OP_ADD, key, value);
AddOperation(Operation.OP_ADD, key, value, true);
}
public void Add(KeyValuePair<TKey, TValue> item) => Add(item.Key, item.Value);
@ -288,7 +306,7 @@ public bool Remove(KeyValuePair<TKey, TValue> item)
bool result = objects.Remove(item.Key);
if (result)
{
AddOperation(Operation.OP_REMOVE, item.Key, item.Value);
AddOperation(Operation.OP_REMOVE, item.Key, item.Value, true);
}
return result;
}

52
Assets/Mirror/Core/SyncList.cs
View File

@ -1,6 +1,7 @@
using System;
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
namespace Mirror
{
@ -12,7 +13,7 @@ public class SyncList<T> : SyncObject, IList<T>, IReadOnlyList<T>
readonly IEqualityComparer<T> comparer;
public int Count => objects.Count;
public bool IsReadOnly { get; private set; }
public bool IsReadOnly => !IsWritable();
public event SyncListChanged Callback;
public enum Operation : byte
@ -63,17 +64,16 @@ public SyncList(IList<T> objects, IEqualityComparer<T> comparer = null)
public override void Reset()
{
IsReadOnly = false;
changes.Clear();
changesAhead = 0;
objects.Clear();
}
void AddOperation(Operation op, int itemIndex, T oldItem, T newItem)
void AddOperation(Operation op, int itemIndex, T oldItem, T newItem, bool checkAccess)
{
if (IsReadOnly)
if (checkAccess && IsReadOnly)
{
throw new InvalidOperationException("Synclists can only be modified at the server");
throw new InvalidOperationException("Synclists can only be modified by the owner.");
}
Change change = new Change
@ -94,6 +94,7 @@ void AddOperation(Operation op, int itemIndex, T oldItem, T newItem)
public override void OnSerializeAll(NetworkWriter writer)
{
Debug.Log($"SyncList OnSerializeAll with {objects.Count} items");
// if init, write the full list content
writer.WriteUInt((uint)objects.Count);
@ -144,9 +145,6 @@ public override void OnSerializeDelta(NetworkWriter writer)
public override void OnDeserializeAll(NetworkReader reader)
{
// This list can now only be modified by synchronization
IsReadOnly = true;
// if init, write the full list content
int count = (int)reader.ReadUInt();
@ -167,9 +165,6 @@ public override void OnDeserializeAll(NetworkReader reader)
public override void OnDeserializeDelta(NetworkReader reader)
{
// This list can now only be modified by synchronization
IsReadOnly = true;
int changesCount = (int)reader.ReadUInt();
for (int i = 0; i < changesCount; i++)
@ -191,6 +186,11 @@ public override void OnDeserializeDelta(NetworkReader reader)
{
index = objects.Count;
objects.Add(newItem);
// add dirty + changes.
// ClientToServer needs to set dirty in server OnDeserialize.
// no access check: server OnDeserialize can always
// write, even for ClientToServer (for broadcasting).
AddOperation(Operation.OP_ADD, objects.Count - 1, default, newItem, false);
}
break;
@ -198,6 +198,11 @@ public override void OnDeserializeDelta(NetworkReader reader)
if (apply)
{
objects.Clear();
// add dirty + changes.
// ClientToServer needs to set dirty in server OnDeserialize.
// no access check: server OnDeserialize can always
// write, even for ClientToServer (for broadcasting).
AddOperation(Operation.OP_CLEAR, 0, default, default, false);
}
break;
@ -207,6 +212,11 @@ public override void OnDeserializeDelta(NetworkReader reader)
if (apply)
{
objects.Insert(index, newItem);
// add dirty + changes.
// ClientToServer needs to set dirty in server OnDeserialize.
// no access check: server OnDeserialize can always
// write, even for ClientToServer (for broadcasting).
AddOperation(Operation.OP_INSERT, index, default, newItem, false);
}
break;
@ -216,6 +226,11 @@ public override void OnDeserializeDelta(NetworkReader reader)
{
oldItem = objects[index];
objects.RemoveAt(index);
// add dirty + changes.
// ClientToServer needs to set dirty in server OnDeserialize.
// no access check: server OnDeserialize can always
// write, even for ClientToServer (for broadcasting).
AddOperation(Operation.OP_REMOVEAT, index, oldItem, default, false);
}
break;
@ -226,6 +241,11 @@ public override void OnDeserializeDelta(NetworkReader reader)
{
oldItem = objects[index];
objects[index] = newItem;
// add dirty + changes.
// ClientToServer needs to set dirty in server OnDeserialize.
// no access check: server OnDeserialize can always
// write, even for ClientToServer (for broadcasting).
AddOperation(Operation.OP_SET, i, oldItem, newItem, false);
}
break;
}
@ -245,7 +265,7 @@ public override void OnDeserializeDelta(NetworkReader reader)
public void Add(T item)
{
objects.Add(item);
AddOperation(Operation.OP_ADD, objects.Count - 1, default, item);
AddOperation(Operation.OP_ADD, objects.Count - 1, default, item, true);
}
public void AddRange(IEnumerable<T> range)
@ -259,7 +279,7 @@ public void AddRange(IEnumerable<T> range)
public void Clear()
{
objects.Clear();
AddOperation(Operation.OP_CLEAR, 0, default, default);
AddOperation(Operation.OP_CLEAR, 0, default, default, true);
}
public bool Contains(T item) => IndexOf(item) >= 0;
@ -300,7 +320,7 @@ public List<T> FindAll(Predicate<T> match)
public void Insert(int index, T item)
{
objects.Insert(index, item);
AddOperation(Operation.OP_INSERT, index, default, item);
AddOperation(Operation.OP_INSERT, index, default, item, true);
}
public void InsertRange(int index, IEnumerable<T> range)
@ -327,7 +347,7 @@ public void RemoveAt(int index)
{
T oldItem = objects[index];
objects.RemoveAt(index);
AddOperation(Operation.OP_REMOVEAT, index, oldItem, default);
AddOperation(Operation.OP_REMOVEAT, index, oldItem, default, true);
}
public int RemoveAll(Predicate<T> match)
@ -354,7 +374,7 @@ public T this[int i]
{
T oldItem = objects[i];
objects[i] = value;
AddOperation(Operation.OP_SET, i, oldItem, value);
AddOperation(Operation.OP_SET, i, oldItem, value, true);
}
}
}

6
Assets/Mirror/Core/SyncObject.cs
View File

@ -25,6 +25,12 @@ public abstract class SyncObject
// => virtual so it simply always records by default
public Func<bool> IsRecording = () => true;
// SyncList/Set/etc. shouldn't be modifiable if not owned.
// otherwise they would silently get out of sync.
// need a lambda because InitSyncObject is called in ctor, when
// 'isClient' etc. aren't initialized yet.
public Func<bool> IsWritable = () => true;
/// <summary>Discard all the queued changes</summary>
// Consider the object fully synchronized with clients
public abstract void ClearChanges();

40
Assets/Mirror/Core/SyncSet.cs
View File

@ -11,7 +11,7 @@ public class SyncSet<T> : SyncObject, ISet<T>
protected readonly ISet<T> objects;
public int Count => objects.Count;
public bool IsReadOnly { get; private set; }
public bool IsReadOnly => !IsWritable();
public event SyncSetChanged Callback;
public enum Operation : byte
@ -47,7 +47,6 @@ public SyncSet(ISet<T> objects)
public override void Reset()
{
IsReadOnly = false;
changes.Clear();
changesAhead = 0;
objects.Clear();
@ -57,11 +56,11 @@ public override void Reset()
// this should be called after a successful sync
public override void ClearChanges() => changes.Clear();
void AddOperation(Operation op, T item)
void AddOperation(Operation op, T item, bool checkAccess)
{
if (IsReadOnly)
if (checkAccess && IsReadOnly)
{
throw new InvalidOperationException("SyncSets can only be modified at the server");
throw new InvalidOperationException("SyncSets can only be modified by the owner.");
}
Change change = new Change
@ -79,7 +78,7 @@ void AddOperation(Operation op, T item)
Callback?.Invoke(op, item);
}
void AddOperation(Operation op) => AddOperation(op, default);
void AddOperation(Operation op, bool checkAccess) => AddOperation(op, default, checkAccess);
public override void OnSerializeAll(NetworkWriter writer)
{
@ -126,9 +125,6 @@ public override void OnSerializeDelta(NetworkWriter writer)
public override void OnDeserializeAll(NetworkReader reader)
{
// This list can now only be modified by synchronization
IsReadOnly = true;
// if init, write the full list content
int count = (int)reader.ReadUInt();
@ -149,9 +145,6 @@ public override void OnDeserializeAll(NetworkReader reader)
public override void OnDeserializeDelta(NetworkReader reader)
{
// This list can now only be modified by synchronization
IsReadOnly = true;
int changesCount = (int)reader.ReadUInt();
for (int i = 0; i < changesCount; i++)
@ -170,6 +163,11 @@ public override void OnDeserializeDelta(NetworkReader reader)
if (apply)
{
objects.Add(item);
// add dirty + changes.
// ClientToServer needs to set dirty in server OnDeserialize.
// no access check: server OnDeserialize can always
// write, even for ClientToServer (for broadcasting).
AddOperation(Operation.OP_ADD, item, false);
}
break;
@ -177,6 +175,11 @@ public override void OnDeserializeDelta(NetworkReader reader)
if (apply)
{
objects.Clear();
// add dirty + changes.
// ClientToServer needs to set dirty in server OnDeserialize.
// no access check: server OnDeserialize can always
// write, even for ClientToServer (for broadcasting).
AddOperation(Operation.OP_CLEAR, false);
}
break;
@ -185,6 +188,11 @@ public override void OnDeserializeDelta(NetworkReader reader)
if (apply)
{
objects.Remove(item);
// add dirty + changes.
// ClientToServer needs to set dirty in server OnDeserialize.
// no access check: server OnDeserialize can always
// write, even for ClientToServer (for broadcasting).
AddOperation(Operation.OP_REMOVE, item, false);
}
break;
}
@ -205,7 +213,7 @@ public bool Add(T item)
{
if (objects.Add(item))
{
AddOperation(Operation.OP_ADD, item);
AddOperation(Operation.OP_ADD, item, true);
return true;
}
return false;
@ -215,14 +223,14 @@ void ICollection<T>.Add(T item)
{
if (objects.Add(item))
{
AddOperation(Operation.OP_ADD, item);
AddOperation(Operation.OP_ADD, item, true);
}
}
public void Clear()
{
objects.Clear();
AddOperation(Operation.OP_CLEAR);
AddOperation(Operation.OP_CLEAR, true);
}
public bool Contains(T item) => objects.Contains(item);
@ -233,7 +241,7 @@ public bool Remove(T item)
{
if (objects.Remove(item))
{
AddOperation(Operation.OP_REMOVE, item);
AddOperation(Operation.OP_REMOVE, item, true);
return true;
}
return false;

14
Assets/Mirror/Examples/Benchmark/Prefabs/Player.prefab
View File

@ -117,20 +117,20 @@ MonoBehaviour:
m_EditorClassIdentifier:
syncDirection: 1
syncMode: 0
syncInterval: 0.033333335
syncInterval: 0
target: {fileID: 2697352357490696306}
clientAuthority: 0
syncPosition: 1
syncRotation: 0
syncScale: 0
showGizmos: 1
showOverlay: 1
overlayColor: {r: 0, g: 0, b: 0, a: 0.5}
onlySyncOnChange: 0
bufferResetMultiplier: 5
positionSensitivity: 0.01
rotationSensitivity: 0.01
scaleSensitivity: 0.01
syncPosition: 1
syncRotation: 0
syncScale: 0
showGizmos: 0
showOverlay: 0
overlayColor: {r: 0, g: 0, b: 0, a: 0.5}
--- !u!114 &644305951047116972
MonoBehaviour:
m_ObjectHideFlags: 0

14
Assets/Mirror/Examples/SyncDirection/Player.cs
View File

@ -8,6 +8,7 @@ public class Player : NetworkBehaviour
public Color localColor = Color.white;
[SyncVar] public int health;
readonly SyncList<int> list = new SyncList<int>();
public override void OnStartLocalPlayer()
{
@ -16,10 +17,10 @@ public override void OnStartLocalPlayer()
void Update()
{
// show health for everyone
textMesh.text = health.ToString();
// show health and list for everyone
textMesh.text = $"{health} / {list.Count}";
// space bar increases health for local player.
// key presses increase health / list for local player.
// note that trusting the client is a bad idea, especially with health.
// SyncDirection is usually used for movement.
//
@ -35,6 +36,9 @@ void Update()
{
if (Input.GetKeyDown(KeyCode.Space))
++health;
if (Input.GetKeyDown(KeyCode.L))
list.Add(list.Count);
}
}
@ -44,11 +48,11 @@ void OnGUI()
if (!isLocalPlayer) return;
int width = 250;
int height = 20;
int height = 50;
GUI.color = localColor;
GUI.Label(
new Rect(Screen.width / 2 - width / 2, Screen.height / 2 - height / 2, width, height),
"Press Space to increase your own health!");
"Press Space to increase your own health!\nPress L to add to your SyncList!");
}
}
}

2
Assets/Mirror/Examples/SyncDirection/Scene.unity
View File

@ -264,7 +264,7 @@ MonoBehaviour:
DualMode: 1
NoDelay: 1
Interval: 10
Timeout: 10000
Timeout: 1000000
FastResend: 2
SendWindowSize: 4096
ReceiveWindowSize: 4096

2
Assets/Mirror/Tests/Editor/NetworkBehaviourTests.cs
View File

@ -796,6 +796,8 @@ public void SerializeAndDeserializeObjectsAll()
{
CreateNetworked(out GameObject _, out NetworkIdentity _, out NetworkBehaviourWithSyncVarsAndCollections comp);
comp.netIdentity.isServer = true;
// add values to synclist
comp.list.Add(42);
comp.list.Add(43);

19
Assets/Mirror/Tests/Editor/SyncDictionaryTest.cs
View File

@ -35,6 +35,10 @@ public void SetUp()
serverSyncDictionary = new SyncDictionary<int, string>();
clientSyncDictionary = new SyncDictionary<int, string>();
// set writable
serverSyncDictionary.IsWritable = () => true;
clientSyncDictionary.IsWritable = () => false;
// add some data to the list
serverSyncDictionary.Add(0, "Hello");
serverSyncDictionary.Add(1, "World");
@ -305,27 +309,18 @@ public void DirtyTest()
[Test]
public void ObjectCanBeReusedAfterReset()
{
clientSyncDictionary.Reset();
serverSyncDictionary.Reset();
// make old client the host
SyncDictionary<int, string> hostList = clientSyncDictionary;
// make old server the host
SyncDictionary<int, string> hostList = serverSyncDictionary;
SyncDictionary<int, string> clientList2 = new SyncDictionary<int, string>();
Assert.That(hostList.IsReadOnly, Is.False);
// Check Add and Sync without errors
hostList.Add(30, "hello");
hostList.Add(35, "world");
SerializeDeltaTo(hostList, clientList2);
}
[Test]
public void ResetShouldSetReadOnlyToFalse()
{
clientSyncDictionary.Reset();
Assert.That(clientSyncDictionary.IsReadOnly, Is.False);
}
[Test]
public void ResetShouldClearChanges()
{

16
Assets/Mirror/Tests/Editor/SyncListTest.cs
View File

@ -44,6 +44,10 @@ public void SetUp()
serverSyncList = new SyncList<string>();
clientSyncList = new SyncList<string>();
// set writable
serverSyncList.IsWritable = () => true;
clientSyncList.IsWritable = () => false;
// add some data to the list
serverSyncList.Add("Hello");
serverSyncList.Add("World");
@ -367,10 +371,10 @@ public void DirtyTest()
[Test]
public void ObjectCanBeReusedAfterReset()
{
clientSyncList.Reset();
serverSyncList.Reset();
// make old client the host
SyncList<string> hostList = clientSyncList;
SyncList<string> hostList = serverSyncList;
SyncList<string> clientList2 = new SyncList<string>();
Assert.That(hostList.IsReadOnly, Is.False);
@ -381,14 +385,6 @@ public void ObjectCanBeReusedAfterReset()
SerializeDeltaTo(hostList, clientList2);
}
[Test]
public void ResetShouldSetReadOnlyToFalse()
{
clientSyncList.Reset();
Assert.That(clientSyncList.IsReadOnly, Is.False);
}
[Test]
public void ResetShouldClearChanges()
{

15
Assets/Mirror/Tests/Editor/SyncSetTest.cs
View File

@ -35,6 +35,10 @@ public void SetUp()
serverSyncSet = new SyncHashSet<string>();
clientSyncSet = new SyncHashSet<string>();
// set writable
serverSyncSet.IsWritable = () => true;
clientSyncSet.IsWritable = () => false;
// add some data to the list
serverSyncSet.Add("Hello");
serverSyncSet.Add("World");
@ -273,10 +277,10 @@ public void WritingToReadOnlyThrows()
[Test]
public void ObjectCanBeReusedAfterReset()
{
clientSyncSet.Reset();
serverSyncSet.Reset();
// make old client the host
SyncHashSet<string> hostList = clientSyncSet;
SyncHashSet<string> hostList = serverSyncSet;
SyncHashSet<string> clientList2 = new SyncHashSet<string>();
Assert.That(hostList.IsReadOnly, Is.False);
@ -288,13 +292,6 @@ public void ObjectCanBeReusedAfterReset()
SerializeDeltaTo(hostList, clientList2);
}
[Test]
public void ResetShouldSetReadOnlyToFalse()
{
clientSyncSet.Reset();
Assert.That(clientSyncSet.IsReadOnly, Is.False);
}
[Test]
public void ResetShouldClearChanges()
{