Skip to content

feat!: OnNetworkSpawn / OnNetworkDespawn #865

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.

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
merged 14 commits into from
Jun 14, 2021
Merged

Conversation

LukeStampfli
Copy link
Contributor

@LukeStampfli LukeStampfli commented May 28, 2021

This PR implements RFC#16: NetworkSpawn (tracked by #864)

BREAKING CHANGE: NetworkStart is now called OnNetworkSpawn.

Test coverage does not include scene objects because I wasn't able to get that to run with the multiinstancehelper but scene objects work on my machine ¯\(ツ)/¯.

{
for (int i = 0; i < ChildNetworkBehaviours.Count; i++)
{
//We check if we are it's NetworkObject owner incase a NetworkObject exists as a child of our NetworkObject
if (!ChildNetworkBehaviours[i].NetworkStartInvoked)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All these checks did not do anything. They were always set to false before this function was run. The conclusion seems to be that this is legacy code which is not needed.

@@ -599,21 +597,21 @@ internal void ClientCollectSoftSyncSceneObjectSweep(NetworkObject[] networkObjec
}
}

internal void OnDestroyObject(ulong networkId, bool destroyGameObject)
internal void OnDespawnObject(ulong networkId, bool destroyGameObject)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renaming this because this is really not OnDestroy but just OnDespawn.

/// </summary>
/// <param name="stream">The stream containing the spawn payload</param>
public virtual void NetworkStart(Stream stream)
public virtual void OnNetworkSpawn(Stream 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.

This stream overload is something we probably want to remove but will be a separate RFC/ PR

Copy link
Contributor

@TwoTenPvP TwoTenPvP left a comment

Choose a reason for hiding this comment

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

This is a positive change. Looks good 👌🏻

{
InitializeVariables();
}

internal void InternalOnNetworkDespawn()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The plan is to have this empty function filled before pre-release as we will most likely have events to unsubscribe here. That's why I pre-emptively added this function for completeness.

@LukeStampfli LukeStampfli merged commit 459c041 into develop Jun 14, 2021
@LukeStampfli LukeStampfli deleted the feature/network-spawn branch June 14, 2021 10:46
SamuelBellomo added a commit that referenced this pull request Jun 22, 2021
* develop: (21 commits)
  feat: NetworkObject Parenting (#855)
  refactor: move RpcMethodId serialization from ILPP to Core (#910)
  fix: NetworkPrefabs container's elements invalidated in the NetworkManager after relaunching Unity Project (#905)
  feat!: OnNetworkSpawn / OnNetworkDespawn (#865)
  feat: Add missing XMLdoc comment (#897)
  refactor: upgrade ILPP backend, drop 2019.4 support, rename types/fields (#895)
  fix: do not access/render runtime info if not playing in the editor (#898)
  feat: Add name property for network variables (#891)
  chore: delete PhilTestResults.xml (#894)
  feat: MultiInstanceHelpers to use fixed FrameRate by default (#893)
  test: General MultiInstanceHelper improvements (#885)
  refactor: isKinematic set to true for rigid bodies of non-authorized instances (#886)
  docs: adding more info to help debug on network transform error message (#892)
  feat: Add RPC Name Lookup Table Provided by NetworkBehaviourILPP (#875)
  fix: remove OnClientConnectedCallback registration from StatsDisplay (#882)
  feat: Add profiling decorator pattern (#878)
  refactor: Removing dead code for NETWORK_VARIABLE_UPDATE (#880)
  fix: update package version to 0.2.0 because of unity minversion change (#881)
  docs: Fix typo in changelog version title
  docs: Hotfix Changelog for 0.1.1 and manual update (#873)
  ...

# Conflicts:
#	com.unity.multiplayer.mlapi/Runtime/Core/NetworkBehaviour.cs
#	com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs
#	com.unity.multiplayer.mlapi/Tests/Runtime/BaseMultiInstanceTest.cs
#	com.unity.multiplayer.mlapi/Tests/Runtime/BaseMultiInstanceTest.cs.meta
SamuelBellomo added a commit that referenced this pull request Jun 22, 2021
* develop: (67 commits)
  feat: NetworkObject Parenting (#855)
  refactor: move RpcMethodId serialization from ILPP to Core (#910)
  fix: NetworkPrefabs container's elements invalidated in the NetworkManager after relaunching Unity Project (#905)
  feat!: OnNetworkSpawn / OnNetworkDespawn (#865)
  feat: Add missing XMLdoc comment (#897)
  refactor: upgrade ILPP backend, drop 2019.4 support, rename types/fields (#895)
  fix: do not access/render runtime info if not playing in the editor (#898)
  feat: Add name property for network variables (#891)
  chore: delete PhilTestResults.xml (#894)
  feat: MultiInstanceHelpers to use fixed FrameRate by default (#893)
  test: General MultiInstanceHelper improvements (#885)
  refactor: isKinematic set to true for rigid bodies of non-authorized instances (#886)
  docs: adding more info to help debug on network transform error message (#892)
  feat: Add RPC Name Lookup Table Provided by NetworkBehaviourILPP (#875)
  fix: remove OnClientConnectedCallback registration from StatsDisplay (#882)
  feat: Add profiling decorator pattern (#878)
  refactor: Removing dead code for NETWORK_VARIABLE_UPDATE (#880)
  fix: update package version to 0.2.0 because of unity minversion change (#881)
  docs: Fix typo in changelog version title
  docs: Hotfix Changelog for 0.1.1 and manual update (#873)
  ...

# Conflicts:
#	com.unity.multiplayer.mlapi/Tests/Runtime/com.unity.multiplayer.mlapi.runtimetests.asmdef
#	testproject/ProjectSettings/EditorBuildSettings.asset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants