Skip to content

Commit 1a17102

Browse files
ShadauxCatShadauxCat
authored
fix: Refactored DeferredMessagingTests to not use coroutines and increased the slush value for the two timing-based tests... (#2248)
* fix: Refactored DeferredMessagingTests to not use coroutines and increased the slush value for the two timing-based tests... hopefully this will clear up the instability * standards * fixed failing tests * Removed some commented-out code and left-in debug code.
1 parent 0592550 commit 1a17102

File tree

4 files changed

+117
-123
lines changed

4 files changed

+117
-123
lines changed

‎com.unity.netcode.gameobjects/TestHelpers/Runtime/MessageHooks.cs

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,21 @@ namespace Unity.Netcode.TestHelpers.Runtime
55
internal class MessageHooks : INetworkHooks
66
{
77
public bool IsWaiting = true;
8-
public delegate bool MessageReceiptCheck(object receivedMessage);
8+
public delegate bool MessageReceiptCheck(Type receivedMessageType);
99
public MessageReceiptCheck ReceiptCheck;
10+
public delegate bool MessageHandleCheck(object receivedMessage);
11+
public MessageHandleCheck HandleCheck;
1012

1113
public static bool CurrentMessageHasTriggerdAHook = false;
1214

13-
public static bool CheckForMessageOfType<T>(object receivedMessage) where T : INetworkMessage
15+
public static bool CheckForMessageOfTypeHandled<T>(object receivedMessage) where T : INetworkMessage
1416
{
1517
return receivedMessage is T;
1618
}
19+
public static bool CheckForMessageOfTypeReceived<T>(Type receivedMessageType) where T : INetworkMessage
20+
{
21+
return receivedMessageType == typeof(T);
22+
}
1723

1824
public void OnBeforeSendMessage<T>(ulong clientId, ref T message, NetworkDelivery delivery) where T : INetworkMessage
1925
{
@@ -25,10 +31,24 @@ public void OnAfterSendMessage<T>(ulong clientId, ref T message, NetworkDelivery
2531

2632
public void OnBeforeReceiveMessage(ulong senderId, Type messageType, int messageSizeBytes)
2733
{
34+
// The way the system works, it goes through all hooks and calls OnBeforeHandleMessage, then handles the message,
35+
// then goes thorugh all hooks and calls OnAfterHandleMessage.
36+
// This ensures each message only manages to activate a single message hook - because we know that only
37+
// one message will ever be handled between OnBeforeHandleMessage and OnAfterHandleMessage,
38+
// we can reset the flag here, and then in OnAfterHandleMessage, the moment the message matches a hook,
39+
// it'll flip this flag back on, and then other hooks will stop checking that message.
40+
// Without this flag, waiting for 10 messages of the same type isn't possible - all 10 hooks would get
41+
// tripped by the first message.
42+
CurrentMessageHasTriggerdAHook = false;
2843
}
2944

3045
public void OnAfterReceiveMessage(ulong senderId, Type messageType, int messageSizeBytes)
3146
{
47+
if (!CurrentMessageHasTriggerdAHook && IsWaiting && (HandleCheck == null || HandleCheck.Invoke(messageType)))
48+
{
49+
IsWaiting = false;
50+
CurrentMessageHasTriggerdAHook = true;
51+
}
3252
}
3353

3454
public void OnBeforeSendBatch(ulong clientId, int messageCount, int batchSizeInBytes, NetworkDelivery delivery)
@@ -72,7 +92,7 @@ public void OnBeforeHandleMessage<T>(ref T message, ref NetworkContext context)
7292

7393
public void OnAfterHandleMessage<T>(ref T message, ref NetworkContext context) where T : INetworkMessage
7494
{
75-
if (!CurrentMessageHasTriggerdAHook && IsWaiting && (ReceiptCheck == null || ReceiptCheck.Invoke(message)))
95+
if (!CurrentMessageHasTriggerdAHook && IsWaiting && (HandleCheck == null || HandleCheck.Invoke(message)))
7696
{
7797
IsWaiting = false;
7898
CurrentMessageHasTriggerdAHook = true;

‎com.unity.netcode.gameobjects/TestHelpers/Runtime/MessageHooksConditional.cs

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,42 +64,76 @@ public MessageHooksConditional(List<MessageHookEntry> messageHookEntries)
6464
}
6565
}
6666

67+
public enum ReceiptType
68+
{
69+
Received,
70+
Handled
71+
}
72+
6773
public class MessageHookEntry
6874
{
6975
internal MessageHooks MessageHooks;
7076
protected NetworkManager m_NetworkManager;
7177
private MessageHooks.MessageReceiptCheck m_MessageReceiptCheck;
78+
private MessageHooks.MessageHandleCheck m_MessageHandleCheck;
7279
internal string MessageType;
80+
private ReceiptType m_ReceiptType;
7381

7482
public void Initialize()
7583
{
76-
Assert.IsNotNull(m_MessageReceiptCheck, $"{nameof(m_MessageReceiptCheck)} is null, did you forget to initialize?");
7784
MessageHooks = new MessageHooks();
78-
MessageHooks.ReceiptCheck = m_MessageReceiptCheck;
85+
if (m_ReceiptType == ReceiptType.Handled)
86+
{
87+
Assert.IsNotNull(m_MessageHandleCheck, $"{nameof(m_MessageHandleCheck)} is null, did you forget to initialize?");
88+
MessageHooks.HandleCheck = m_MessageHandleCheck;
89+
}
90+
else
91+
{
92+
Assert.IsNotNull(m_MessageReceiptCheck, $"{nameof(m_MessageReceiptCheck)} is null, did you forget to initialize?");
93+
MessageHooks.ReceiptCheck = m_MessageReceiptCheck;
94+
}
7995
Assert.IsNotNull(m_NetworkManager.MessagingSystem, $"{nameof(NetworkManager.MessagingSystem)} is null! Did you forget to start first?");
8096
m_NetworkManager.MessagingSystem.Hook(MessageHooks);
8197
}
8298

8399
internal void AssignMessageType<T>() where T : INetworkMessage
84100
{
85101
MessageType = typeof(T).Name;
86-
m_MessageReceiptCheck = MessageHooks.CheckForMessageOfType<T>;
102+
if (m_ReceiptType == ReceiptType.Handled)
103+
{
104+
m_MessageHandleCheck = MessageHooks.CheckForMessageOfTypeHandled<T>;
105+
}
106+
else
107+
{
108+
m_MessageReceiptCheck = MessageHooks.CheckForMessageOfTypeReceived<T>;
109+
}
87110
Initialize();
88111
}
89112

90113
internal void AssignMessageType(Type type)
91114
{
92115
MessageType = type.Name;
93-
m_MessageReceiptCheck = (message) =>
116+
if (m_ReceiptType == ReceiptType.Handled)
94117
{
95-
return message.GetType() == type;
96-
};
118+
m_MessageHandleCheck = (message) =>
119+
{
120+
return message.GetType() == type;
121+
};
122+
}
123+
else
124+
{
125+
m_MessageReceiptCheck = (messageType) =>
126+
{
127+
return messageType == type;
128+
};
129+
}
97130
Initialize();
98131
}
99132

100-
public MessageHookEntry(NetworkManager networkManager)
133+
public MessageHookEntry(NetworkManager networkManager, ReceiptType type = ReceiptType.Handled)
101134
{
102135
m_NetworkManager = networkManager;
136+
m_ReceiptType = type;
103137
}
104138
}
105139
}

‎com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -762,37 +762,39 @@ protected IEnumerator WaitForClientsConnectedOrTimeOut()
762762
yield return WaitForClientsConnectedOrTimeOut(m_ClientNetworkManagers);
763763
}
764764

765-
internal IEnumerator WaitForMessageReceived<T>(List<NetworkManager> wiatForReceivedBy) where T : INetworkMessage
765+
internal IEnumerator WaitForMessageReceived<T>(List<NetworkManager> wiatForReceivedBy, ReceiptType type = ReceiptType.Handled) where T : INetworkMessage
766766
{
767767
// Build our message hook entries tables so we can determine if all clients received spawn or ownership messages
768768
var messageHookEntriesForSpawn = new List<MessageHookEntry>();
769769
foreach (var clientNetworkManager in wiatForReceivedBy)
770770
{
771-
var messageHook = new MessageHookEntry(clientNetworkManager);
771+
var messageHook = new MessageHookEntry(clientNetworkManager, type);
772772
messageHook.AssignMessageType<T>();
773773
messageHookEntriesForSpawn.Add(messageHook);
774774
}
775775
// Used to determine if all clients received the CreateObjectMessage
776776
var hooks = new MessageHooksConditional(messageHookEntriesForSpawn);
777777
yield return WaitForConditionOrTimeOut(hooks);
778+
Assert.False(s_GlobalTimeoutHelper.TimedOut);
778779
}
779780

780-
internal IEnumerator WaitForMessagesReceived(List<Type> messagesInOrder, List<NetworkManager> wiatForReceivedBy)
781+
internal IEnumerator WaitForMessagesReceived(List<Type> messagesInOrder, List<NetworkManager> wiatForReceivedBy, ReceiptType type = ReceiptType.Handled)
781782
{
782783
// Build our message hook entries tables so we can determine if all clients received spawn or ownership messages
783784
var messageHookEntriesForSpawn = new List<MessageHookEntry>();
784785
foreach (var clientNetworkManager in wiatForReceivedBy)
785786
{
786787
foreach (var message in messagesInOrder)
787788
{
788-
var messageHook = new MessageHookEntry(clientNetworkManager);
789+
var messageHook = new MessageHookEntry(clientNetworkManager, type);
789790
messageHook.AssignMessageType(message);
790791
messageHookEntriesForSpawn.Add(messageHook);
791792
}
792793
}
793794
// Used to determine if all clients received the CreateObjectMessage
794795
var hooks = new MessageHooksConditional(messageHookEntriesForSpawn);
795796
yield return WaitForConditionOrTimeOut(hooks);
797+
Assert.False(s_GlobalTimeoutHelper.TimedOut);
796798
}
797799

798800
/// <summary>

0 commit comments

Comments
 (0)