Skip to content

feat: Adding support for netsim to be able to work with Tools adapter #2184

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

Merged
DenninDalke merged 17 commits into from
Sep 23, 2022
Merged

feat: Adding support for netsim to be able to work with Tools adapter #2184

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
7c1ebbc
Adding support for netsim to be able to work with Tools adapter
Sep 8, 2022
92de75a
Adding the simulator only if active in build inside unity transport.
Sep 12, 2022
49a0ceb
Setting up Simulator Pipeline stage if NetSim is available
Sep 12, 2022
95ba089
Merge branch 'develop' into feature/tools-netsim-support
Sep 13, 2022
d748605
Apply suggestions from code review
Sep 14, 2022
23a0f5b
Removal of Debug Simulator in UTP1, updated callback to avoid moving to
Sep 14, 2022
1461103
Merge branch 'develop' into feature/tools-netsim-support
Sep 14, 2022
55a131c
Fixed compiling issues
Sep 14, 2022
87144f0
Fix compiler error typo: "river" should be "driver"
DenninDalke Sep 19, 2022
3093b22
Deprecate DebugSimulator when using UTP2
DenninDalke Sep 19, 2022
838485b
Deprecate SetDebugSimulatorParameters on UTP1
DenninDalke Sep 20, 2022
3abb7b0
Merge branch 'develop' into feature/tools-netsim-support
DenninDalke Sep 20, 2022
9263fd1
Add tool suggestion to DebugSimulator deprecation
DenninDalke Sep 21, 2022
afbcdcc
Add tool suggestion for DebugSimulator deprecation and comment
DenninDalke Sep 21, 2022
5063a87
Move comments out the parameter list to avoid CI failure
DenninDalke Sep 22, 2022
61e04e5
Fix formatting
DenninDalke Sep 22, 2022
af4f757
Merge branch 'develop' into feature/tools-netsim-support
DenninDalke Sep 22, 2022
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
1 change: 1 addition & 0 deletions com.unity.netcode.gameobjects/Runtime/AssemblyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@
[assembly: InternalsVisibleTo("Unity.Netcode.RuntimeTests")]
[assembly: InternalsVisibleTo("Unity.Netcode.TestHelpers.Runtime")]
[assembly: InternalsVisibleTo("Unity.Netcode.Adapter.UTP")]
[assembly: InternalsVisibleTo("Unity.Multiplayer.Tools.Adapters.Ngo1WithUtp2")]
Comment thread
JesseOlmer marked this conversation as resolved.
60 changes: 43 additions & 17 deletions com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
// NetSim Implementation compilation boilerplate
// All references to UNITY_MP_TOOLS_NETSIM_IMPLEMENTATION_ENABLED should be defined in the same way,
// as any discrepancies are likely to result in build failures
// ---------------------------------------------------------------------------------------------------------------------
Comment thread
Rosme marked this conversation as resolved.
Outdated
#if UNITY_EDITOR || ((DEVELOPMENT_BUILD && !UNITY_MP_TOOLS_NETSIM_DISABLED_IN_DEVELOP) || (!DEVELOPMENT_BUILD && UNITY_MP_TOOLS_NETSIM_ENABLED_IN_RELEASE))
Comment thread
Rosme marked this conversation as resolved.
Outdated
#define UNITY_MP_TOOLS_NETSIM_IMPLEMENTATION_ENABLED
#endif
// ---------------------------------------------------------------------------------------------------------------------
Comment thread
Rosme marked this conversation as resolved.
Outdated

using System;
using System.Collections.Generic;
using UnityEngine;
Expand Down Expand Up @@ -346,6 +355,10 @@ private struct PacketLossCache
public float PacketLoss;
};

internal static event Action<UnityTransport> TransportInitialized;
internal static event Action<UnityTransport> TransportDisposed;
Copy link
Copy Markdown
Contributor

@ian-m-unity ian-m-unity Sep 8, 2022

Choose a reason for hiding this comment

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

Should a reference to the Transport be included in the disposed message? Is it still valid at that point?

Copy link
Copy Markdown
Author

@Rosme Rosme Sep 12, 2022

Choose a reason for hiding this comment

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

Yes, since we are using the Instance ID to track the adapter.

Copy link
Copy Markdown
Contributor

@JesseOlmer JesseOlmer Sep 13, 2022

Choose a reason for hiding this comment

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

Can you expand on what the value is of tracking dispose instead of shutdown? Does this need to be an event or can you use a weak reference?

To expand on Ian's concern and my other comment about internals vs public, at the point you have this being invoked the object is essentially "off limits" to other objects as far as best practices go - regardless of how this may be used right now this feels dangerous and dependent on tribal knowledge about the control flow. If all you need is the ID, why not pass that as the event parameter instead of the entire (mostly disposed) object?

Copy link
Copy Markdown
Author

@Rosme Rosme Sep 13, 2022

Choose a reason for hiding this comment

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

There's more to it. We are using the whole UnityTransport to access the Network Driver. Arguably, we could provide the Instance ID and the Network Driver only as arguments. I am not against this change. As for the Disposed, we definitely only need the Instance ID and I can change it in the same manner if you'd prefer that.

internal NetworkDriver NetworkDriver => m_Driver;

private PacketLossCache m_PacketLossCache = new PacketLossCache();

private State m_State = State.Disconnected;
Expand Down Expand Up @@ -389,6 +402,8 @@ private void InitDriver()
out m_UnreliableFragmentedPipeline,
out m_UnreliableSequencedFragmentedPipeline,
out m_ReliableSequencedPipeline);

TransportInitialized?.Invoke(this);
}

private void DisposeInternals()
Expand All @@ -406,6 +421,8 @@ private void DisposeInternals()
}

m_SendQueue.Clear();

TransportDisposed?.Invoke(this);
}

private NetworkPipeline SelectSendPipeline(NetworkDelivery delivery)
Expand Down Expand Up @@ -1330,6 +1347,10 @@ private void ConfigureSimulator()
, mode: ApplyMode.AllPackets
#endif
);

#if UTP_TRANSPORT_2_0_ABOVE
Comment thread
becksebenius-unity marked this conversation as resolved.
Outdated
m_NetworkSettings.WithNetworkSimulatorParameters();
#endif
}

/// <summary>
Expand All @@ -1348,12 +1369,12 @@ public void CreateDriver(UnityTransport transport, out NetworkDriver driver,
#if MULTIPLAYER_TOOLS_1_0_0_PRE_7
#if !UTP_TRANSPORT_2_0_ABOVE
NetworkPipelineStageCollection.RegisterPipelineStage(new NetworkMetricsPipelineStage());
#endif
#endif
#endif //!UTP_TRANSPORT_2_0_ABOVE
Comment thread
DenninDalke marked this conversation as resolved.
Outdated
#endif //MULTIPLAYER_TOOLS_1_0_0_PRE_7

#if UNITY_EDITOR || DEVELOPMENT_BUILD
#if UNITY_EDITOR || DEVELOPMENT_BUILD || UNITY_MP_TOOLS_NETSIM_IMPLEMENTATION_ENABLED
Copy link
Copy Markdown
Contributor

@JesseOlmer JesseOlmer Sep 13, 2022

Choose a reason for hiding this comment

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

Is UNITY_EDITOR || DEVELOPMENT_BUILD here because the NETSIM define is 2.0 only? Can't we simplify these cases in the first define?

Copy link
Copy Markdown
Author

@Rosme Rosme Sep 13, 2022

Choose a reason for hiding this comment

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

We need to maintain the existing Debug Simulator as well if the Tools are not present. But since the Network Simulator can also be included in Release builds if the user wished so, we need all three.

ConfigureSimulator();
#endif
#endif //UNITY_EDITOR || DEVELOPMENT_BUILD || UNITY_MP_TOOLS_NETSIM_IMPLEMENTATION_ENABLED

m_NetworkSettings.WithNetworkConfigParameters(
maxConnectAttempts: transport.m_MaxConnectAttempts,
Expand All @@ -1362,76 +1383,81 @@ public void CreateDriver(UnityTransport transport, out NetworkDriver driver,
#if UTP_TRANSPORT_2_0_ABOVE
sendQueueCapacity: m_MaxPacketQueueSize,
receiveQueueCapacity: m_MaxPacketQueueSize,
#endif
#endif //UTP_TRANSPORT_2_0_ABOVE
heartbeatTimeoutMS: transport.m_HeartbeatTimeoutMS);

driver = NetworkDriver.Create(m_NetworkSettings);

#if MULTIPLAYER_TOOLS_1_0_0_PRE_7
#if UTP_TRANSPORT_2_0_ABOVE
driver.RegisterPipelineStage<NetworkMetricsPipelineStage>(new NetworkMetricsPipelineStage());
#endif
#endif
#endif //UTP_TRANSPORT_2_0_ABOVE
Comment thread
DenninDalke marked this conversation as resolved.
Outdated
#endif //MULTIPLAYER_TOOLS_1_0_0_PRE_7

#if UNITY_EDITOR || DEVELOPMENT_BUILD
#if !UNITY_MP_TOOLS_NETSIM_IMPLEMENTATION_ENABLED
Comment thread
DenninDalke marked this conversation as resolved.
Outdated
if (DebugSimulator.PacketDelayMS > 0 || DebugSimulator.PacketDropRate > 0)
#endif //!UNITY_MP_TOOLS_NETSIM_IMPLEMENTATION_ENABLED
{
unreliableFragmentedPipeline = driver.CreatePipeline(
typeof(FragmentationPipelineStage),
typeof(SimulatorPipelineStage)
#if !UTP_TRANSPORT_2_0_ABOVE
, typeof(SimulatorPipelineStageInSend)
#endif
#endif //!UTP_TRANSPORT_2_0_ABOVE
#if MULTIPLAYER_TOOLS_1_0_0_PRE_7
, typeof(NetworkMetricsPipelineStage)
#endif
#endif //MULTIPLAYER_TOOLS_1_0_0_PRE_7
);
unreliableSequencedFragmentedPipeline = driver.CreatePipeline(
typeof(FragmentationPipelineStage),
typeof(UnreliableSequencedPipelineStage),
typeof(SimulatorPipelineStage)
#if !UTP_TRANSPORT_2_0_ABOVE
, typeof(SimulatorPipelineStageInSend)
#endif
#endif //!UTP_TRANSPORT_2_0_ABOVE
#if MULTIPLAYER_TOOLS_1_0_0_PRE_7
, typeof(NetworkMetricsPipelineStage)
#endif
#endif //MULTIPLAYER_TOOLS_1_0_0_PRE_7
);
reliableSequencedPipeline = driver.CreatePipeline(
typeof(ReliableSequencedPipelineStage),
typeof(SimulatorPipelineStage)
#if !UTP_TRANSPORT_2_0_ABOVE
, typeof(SimulatorPipelineStageInSend)
#endif
#endif //!UTP_TRANSPORT_2_0_ABOVE
#if MULTIPLAYER_TOOLS_1_0_0_PRE_7
, typeof(NetworkMetricsPipelineStage)
#endif
#endif //MULTIPLAYER_TOOLS_1_0_0_PRE_7
);
}
#if !UNITY_MP_TOOLS_NETSIM_IMPLEMENTATION_ENABLED
else
#endif
{
unreliableFragmentedPipeline = driver.CreatePipeline(
typeof(FragmentationPipelineStage)
#if MULTIPLAYER_TOOLS_1_0_0_PRE_7
, typeof(NetworkMetricsPipelineStage)
#endif
#endif //MULTIPLAYER_TOOLS_1_0_0_PRE_7
);
unreliableSequencedFragmentedPipeline = driver.CreatePipeline(
typeof(FragmentationPipelineStage),
typeof(UnreliableSequencedPipelineStage)
#if MULTIPLAYER_TOOLS_1_0_0_PRE_7
, typeof(NetworkMetricsPipelineStage)
#endif
#endif //MULTIPLAYER_TOOLS_1_0_0_PRE_7
);
reliableSequencedPipeline = driver.CreatePipeline(
typeof(ReliableSequencedPipelineStage)
#if MULTIPLAYER_TOOLS_1_0_0_PRE_7
, typeof(NetworkMetricsPipelineStage)
#endif
#endif //MULTIPLAYER_TOOLS_1_0_0_PRE_7
);
}
}
#endif //UNITY_MP_TOOLS_NETSIM_IMPLEMENTATION_ENABLED
#endif //UNITY_EDITOR || DEVELOPMENT_BUILD
}

// -------------- Utility Types -------------------------------------------------------------------------------

Expand Down