Skip to content

Commit 8dc06bf

Browse files
chore: Add a DA service unit test (#3423)
<!-- Replace this block with what this PR does and why. Describe what you'd like reviewers to know, how you applied the engineering principles, and any interesting tradeoffs made. Delete bullet points below that don't apply, and update the changelog section as appropriate. --> <!-- Add short version of the JIRA ticket to the PR title (e.g. "feat: new shiny feature [MTT-123]") --> [MTT-11550](https://jira.unity3d.com/browse/MTT-11550) ## Changelog - Added: A unit test against the da service ## Testing and Documentation - Includes unit tests. <!-- Uncomment and mark items off with a * if this PR deprecates any API: ### Deprecated API - [ ] An `[Obsolete]` attribute was added along with a `(RemovedAfter yyyy-mm-dd)` entry. - [ ] An [api updater] was added. - [ ] Deprecation of the API is explained in the CHANGELOG. - [ ] The users can understand why this API was removed and what they should use instead. --> ## Backport <!-- If this is a backport: - Add the following to the PR title: "\[Backport\] ..." . - Link to the original PR. If this needs a backport - state this here If a backport is not needed please provide the reason why. If the "Backports" section is not present it will lead to a CI test failure. --> No backport applicable - DA only --------- Co-authored-by: NoelStephensUnity <[email protected]>
1 parent 83a713a commit 8dc06bf

File tree

68 files changed

+1000
-745
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

68 files changed

+1000
-745
lines changed

.yamato/desktop-standalone-tests.yml

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
# DESCRIPTION--------------------------------------------------------------------------
55
# This job is responsible for Desktop platform test validation.
66
# Those tests cover both PlayMode and EditMode tests from package test assemblies.
7-
7+
88
# CONFIGURATION STRUCTURE--------------------------------------------------------------
99
# Jobs are generated using nested loops (separate build phase and run phase). Worth noting that run phase uses the build as dependency:
1010
# 1. For all desktop platform (Windows, macOS, Ubuntu)
@@ -17,17 +17,17 @@
1717
# 1. Build Phase: Creates standalone players for desktop platforms
1818
# 2. Run Phase: Executes runtime tests on actual desktop devices
1919
# The Run phase uses build job as dependency
20-
20+
2121
# Note: More of a Unity specific but test assemblies need to be included in the build phase command
2222
# Note: All builds can be made on x64 machines since those are compatible with ARM64 target devices
2323

2424
# QUALITY THOUGHTS--------------------------------------------------------------------
2525
# TODO: consider adding all projects that have tests
2626
# To see where this job is included (in trigger job definitions) look into _triggers.yml file
27-
27+
2828
#-----------------------------------------------------------------------------------
29-
30-
29+
30+
3131
# BUILD PHASE CONFIGURATION------------------------------------------------------------------------------------
3232
{% for project in projects.default -%}
3333
{% for platform in test_platforms.desktop -%}
@@ -58,10 +58,10 @@ desktop_standalone_build_{{ project.name }}_{{ platform.name }}_{{ backend }}_{{
5858
{% endfor -%}
5959
{% endfor -%}
6060
{% endfor -%}
61-
62-
63-
64-
61+
62+
63+
64+
6565
# RUN PHASE CONFIGURATION------------------------------------------------------------------------------------
6666
{% for project in projects.default -%}
6767
{% for platform in test_platforms.desktop -%}
@@ -80,22 +80,25 @@ desktop_standalone_test_{{ project.name }}_{{ platform.name }}_{{ backend }}_{{
8080
# Set additional variables for running the echo server (This is needed ONLY for NGOv2.X because relates to Distributed Authority)
8181
{% if platform.name != "win" %} # Issues with win and mac are tracked in MTT-11606
8282
variables:
83+
# The echo server is a "mock" server that is only used to test encoding/decoding of messages.
84+
# It is used by the DistributedAuthorityCodecTests. These are tests that are built and maintained by the CMB service team.
8385
ECHO_SERVER_PORT: "7788"
84-
# Set this to ensure the DA codec tests will fail if they cannot connect to the echo-server
85-
# The default is to ignore the codec tests if the echo-server fails to connect
86+
# Set this to ensure the DistributedAuthorityCodecTests will fail if they cannot connect to the echo server.
87+
# The default is to ignore the codec tests if the echo server fails to connect
8688
ENSURE_CODEC_TESTS: "true"
89+
90+
# When USE_CMB_SERVICE is set to true, any C# tests configured to use the DA host will instead use the CMB service.
91+
USE_CMB_SERVICE: "true"
92+
# This is the port on which to run the full standalone CMB service.
93+
# The port needs to be different from the echo server port as two processes cannot bind to same port.
94+
CMB_SERVICE_PORT: "7799"
8795
{% endif %}
8896

8997
commands:
9098
# If ubuntu, run rust echo server (This is needed ONLY for NGOv2.X because relates to Distributed Authority)
9199
{% if platform.name != "win" %} # Issues with win and mac are tracked in MTT-11606
92-
- git clone https://github.com/Unity-Technologies/mps-common-multiplayer-backend.git
93-
# Install rust
94-
- curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y
95-
# Build the echo server
96-
- cd ./mps-common-multiplayer-backend/runtime && $HOME/.cargo/bin/cargo build --example ngo_echo_server
97-
# Run the echo server in the background - this will reuse the artifacts from the build
98-
- cd ./mps-common-multiplayer-backend/runtime && $HOME/.cargo/bin/cargo run --example ngo_echo_server -- --port $ECHO_SERVER_PORT &
100+
# run_cmb_service.sh builds and starts a release version of the full CMB service (along with the limited echo server)
101+
- ./Tools/CI/run_cmb_service.sh -e $ECHO_SERVER_PORT -s $CMB_SERVICE_PORT
99102
{% endif %}
100103

101104
- unity-downloader-cli --fast --wait -u {{ editor }} -c Editor {% if backend == "il2cpp" %} -c il2cpp {% endif %} {% if platform.name == "mac" %} --arch arm64 {% endif %} # For macOS we use ARM64 models

Tools/CI/run_cmb_service.sh

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
#!/bin/bash
2+
3+
# DESCRIPTION--------------------------------------------------------------------------
4+
# This bash file is used to build and run CMB service resources.
5+
# These are resources relating to the distributed authority feature. The CMB service is built and maintained by the services team.
6+
7+
# CONTENTS-----------------------------------------------------------------------------
8+
# There are two resources that are built and run:
9+
# 1. The echo server - this is a "mock" server that simply echoes back the messages it receives. It is used to test message serialization.
10+
# The echo server is used inside the DistributedAuthorityCodecTests. These are tests that are built and maintained by the CMB service team.
11+
# 2. The standalone server - this is a release build of the full CMB service.
12+
13+
# USAGE---------------------------------------------------------------------------------
14+
# The script requires ports to be defined. This works via the following command:
15+
# ./<path-to-script>/run_cmb_service.sh -e <echo-server-port> -s <cmb-service-port>
16+
17+
# Example usage:
18+
# ./<path-to-script>/run_cmb_service.sh -e 7788 -s 7799
19+
20+
# This script is currently used in the desktop-standalone-tests yamato job.
21+
22+
# TECHNICAL CONSIDERATIONS---------------------------------------------------------------
23+
# This is a bash script and so needs to be run on a Unix based system.
24+
# - It runs on mac and ubuntu bokken machines. It will not run on windows bokken machines.
25+
26+
# This script starts processes running in the background. All logs are still written to stdout and will be seen in the logs of the job.
27+
# Running in the background means that the processes will continue to run after the script exits.
28+
# This is not a concern for bokken machines as all processes are killed when the machine is shut down.
29+
30+
# QUALITY THOUGHTS--------------------------------------------------------------------
31+
# Currently this script simply uses the head of the cmb service repo.
32+
# We might want to consider using the latest released version in the future.
33+
34+
#-------------------------------------------------------------------------------------
35+
36+
# Define error message if ports are not defined
37+
ERROR="Error: Expected ports to be defined! Example script usage:"
38+
EXAMPLE="run_cmb_service.sh -e <echo-server-port> -s <cmb-service-port>"
39+
40+
# get arguments passed to the script
41+
while getopts 'e:s:' flag; do
42+
case "${flag}" in
43+
e) echo_port="${OPTARG}" ;;
44+
s) service_port="${OPTARG}" ;;
45+
*) printf "%s\n" "$ERROR" "$EXAMPLE"
46+
exit 1 ;;
47+
esac
48+
done
49+
50+
# ensure arguments were passed and the ports are defined
51+
if [ -z "$echo_port" ] || [ -z "$service_port" ]; then
52+
printf "%s\n" "$ERROR" "$EXAMPLE";
53+
exit 1;
54+
elif [[ "$echo_port" == "$service_port" ]]; then
55+
printf "Ports cannot be the same! Please use different ports.\n";
56+
exit 1;
57+
fi
58+
59+
echo "Starting with echo server on port: $echo_port and the cmb service on port: $service_port"
60+
61+
# Setup -------------------------------------------------------------------------
62+
63+
# clone the cmb service repo
64+
git clone https://github.com/Unity-Technologies/mps-common-multiplayer-backend.git
65+
# navigate to the cmb service directory
66+
cd ./mps-common-multiplayer-backend/runtime
67+
68+
# Install rust
69+
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y
70+
# Add the cargo bin directory to the PATH
71+
export PATH="$HOME/.cargo/bin:$PATH"
72+
73+
74+
# Echo server -------------------------------------------------------------------
75+
76+
# Build the echo server
77+
cargo build --example ngo_echo_server
78+
# Run the echo server in the background
79+
cargo run --example ngo_echo_server -- --port $echo_port &
80+
81+
82+
# CMB Service -------------------------------------------------------------------
83+
84+
# Build a release version of the standalone cmb service
85+
cargo build --release --locked
86+
87+
# Run the standalone service on an infinite loop in the background.
88+
# The infinite loop is required as the service will exit each time all connected clients disconnect.
89+
# This means the service will exit after each test. The infinite loop will immediately restart the service each time it exits.
90+
while :; do
91+
./target/release/comb-server -l error --metrics-port 5000 standalone --port $service_port -t 60m;
92+
done & # <- use & to run the entire loop in the background

com.unity.netcode.gameobjects/Editor/Analytics/NetcodeAnalytics.cs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,26 +29,21 @@ internal class NetcodeAnalytics : NetworkManager.NetcodeAnalytics
2929
/// Determines if we are running an integration test of the analytics integration
3030
/// </summary>
3131
internal static bool IsIntegrationTest = false;
32+
internal static bool EnableIntegrationTestAnalytics = false;
3233
#if ENABLE_NGO_ANALYTICS_LOGGING
3334
internal static bool EnableLogging = true;
3435
#else
3536
internal static bool EnableLogging = false;
3637
#endif
3738

38-
// Preserves the analytics enabled flag
39-
private bool m_OriginalAnalyticsEnabled;
40-
4139
internal override void OnOneTimeSetup()
4240
{
43-
m_OriginalAnalyticsEnabled = EditorAnalytics.enabled;
44-
// By default, we always disable analytics during integration testing
45-
EditorAnalytics.enabled = false;
41+
IsIntegrationTest = true;
4642
}
4743

4844
internal override void OnOneTimeTearDown()
4945
{
50-
// Reset analytics to the original value
51-
EditorAnalytics.enabled = m_OriginalAnalyticsEnabled;
46+
IsIntegrationTest = false;
5247
}
5348

5449
internal List<NetworkManagerAnalyticsHandler> AnalyticsTestResults = new List<NetworkManagerAnalyticsHandler>();
@@ -80,15 +75,19 @@ internal override void ModeChanged(PlayModeStateChange playModeState, NetworkMan
8075
}
8176
}
8277

78+
private bool ShouldLogAnalytics()
79+
{
80+
return (IsIntegrationTest && EnableIntegrationTestAnalytics) || (!IsIntegrationTest && EditorAnalytics.enabled);
81+
}
82+
8383
/// <summary>
8484
/// Editor Only
8585
/// Invoked when the session is started.
8686
/// </summary>
8787
/// <param name="networkManager">The <see cref="NetworkManager"/> instance when the session is started.</param>
8888
internal override void SessionStarted(NetworkManager networkManager)
8989
{
90-
// If analytics is disabled and we are not running an integration test, then exit early.
91-
if (!EditorAnalytics.enabled && !IsIntegrationTest)
90+
if (!ShouldLogAnalytics())
9291
{
9392
return;
9493
}
@@ -112,7 +111,7 @@ internal override void SessionStarted(NetworkManager networkManager)
112111
internal override void SessionStopped(NetworkManager networkManager)
113112
{
114113
// If analytics is disabled and we are not running an integration test or there are no sessions, then exit early.
115-
if ((!EditorAnalytics.enabled && !IsIntegrationTest) || RecentSessions.Count == 0)
114+
if (!ShouldLogAnalytics() || RecentSessions.Count == 0)
116115
{
117116
return;
118117
}
@@ -135,7 +134,7 @@ internal override void SessionStopped(NetworkManager networkManager)
135134
private void UpdateAnalytics(NetworkManager networkManager)
136135
{
137136
// If analytics is disabled and we are not running an integration test or there are no sessions to process, then exit early.
138-
if ((!EditorAnalytics.enabled && !IsIntegrationTest) || RecentSessions.Count == 0)
137+
if (!ShouldLogAnalytics() || RecentSessions.Count == 0)
139138
{
140139
return;
141140
}

com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,10 @@ public bool SetOwnershipLock(bool lockOwnership = true)
628628
RemoveOwnershipExtended(OwnershipStatusExtended.Locked);
629629
}
630630

631-
SendOwnershipStatusUpdate();
631+
if (IsSpawned)
632+
{
633+
SendOwnershipStatusUpdate();
634+
}
632635

633636
return true;
634637
}

com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2042,6 +2042,7 @@ internal void SynchronizeNetworkObjects(ulong clientId, bool synchronizingServic
20422042
}
20432043

20442044
// Organize how (and when) we serialize our NetworkObjects
2045+
var hasSynchronizedActive = false;
20452046
for (int i = 0; i < SceneManager.sceneCount; i++)
20462047
{
20472048
var scene = SceneManager.GetSceneAt(i);
@@ -2067,6 +2068,10 @@ internal void SynchronizeNetworkObjects(ulong clientId, bool synchronizingServic
20672068
continue;
20682069
}
20692070
sceneEventData.SceneHash = SceneHashFromNameOrPath(scene.path);
2071+
if (sceneEventData.SceneHash == sceneEventData.ActiveSceneHash)
2072+
{
2073+
hasSynchronizedActive = true;
2074+
}
20702075

20712076
// If we are just a normal client, then always use the server scene handle
20722077
if (NetworkManager.DistributedAuthorityMode)
@@ -2085,7 +2090,7 @@ internal void SynchronizeNetworkObjects(ulong clientId, bool synchronizingServic
20852090
}
20862091

20872092
// If we are just a normal client and in distributed authority mode, then always use the known server scene handle
2088-
if (NetworkManager.DistributedAuthorityMode && !NetworkManager.DAHost)
2093+
if (NetworkManager.DistributedAuthorityMode && NetworkManager.CMBServiceConnection)
20892094
{
20902095
sceneEventData.AddSceneToSynchronize(SceneHashFromNameOrPath(scene.path), ClientSceneHandleToServerSceneHandle[scene.handle]);
20912096
}
@@ -2095,6 +2100,11 @@ internal void SynchronizeNetworkObjects(ulong clientId, bool synchronizingServic
20952100
}
20962101
}
20972102

2103+
if (!hasSynchronizedActive && NetworkManager.CMBServiceConnection && synchronizingService)
2104+
{
2105+
sceneEventData.AddSceneToSynchronize(BuildIndexToHash[activeScene.buildIndex], ClientSceneHandleToServerSceneHandle[activeScene.handle]);
2106+
}
2107+
20982108
sceneEventData.AddSpawnedNetworkObjects();
20992109
sceneEventData.AddDespawnedInSceneNetworkObjects();
21002110
var message = new SceneEventMessage

com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,7 +1118,6 @@ private void SpawnNetworkObjectLocallyCommon(NetworkObject networkObject, ulong
11181118
return;
11191119
}
11201120

1121-
networkObject.IsSpawned = true;
11221121
networkObject.IsSceneObject = sceneObject;
11231122

11241123
// Always check to make sure our scene of origin is properly set for in-scene placed NetworkObjects
@@ -1151,6 +1150,8 @@ private void SpawnNetworkObjectLocallyCommon(NetworkObject networkObject, ulong
11511150
{
11521151
networkObject.SetOwnershipLock();
11531152
}
1153+
1154+
networkObject.IsSpawned = true;
11541155
SpawnedObjects.Add(networkObject.NetworkObjectId, networkObject);
11551156
SpawnedObjectsList.Add(networkObject);
11561157

@@ -2228,14 +2229,7 @@ internal void SynchronizeObjectsToNewlyJoinedClient(ulong newClientId)
22282229
{
22292230
if (networkObject.Observers.Contains(newClientId))
22302231
{
2231-
if (NetworkManager.LogLevel <= LogLevel.Developer)
2232-
{
2233-
// Temporary tracking to make sure we are not showing something already visibile (should never be the case for this)
2234-
Debug.LogWarning($"[{nameof(SynchronizeObjectsToNewlyJoinedClient)}][{networkObject.name}] New client as already an observer!");
2235-
}
2236-
// For now, remove the client (impossible for the new client to have an instance since the session owner doesn't) to make sure newly added
2237-
// code to handle this edge case works.
2238-
networkObject.Observers.Remove(newClientId);
2232+
continue;
22392233
}
22402234
networkObject.NetworkShow(newClientId);
22412235
}

0 commit comments

Comments
 (0)