Skip to content

Conversation

@LukeStampfli
Copy link
Contributor

@LukeStampfli LukeStampfli commented May 25, 2021

See linked RFC document →

RFC proposing renaming NetworkStart to OnNetworkSpawn and introducing a OnNetworkDespawn virtual event function.

@LukeStampfli LukeStampfli added the open RFC is open for review and comments label May 25, 2021
}
```

# Reference-level explanation
Copy link
Contributor

Choose a reason for hiding this comment

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

makes a lot of sense, thank you for this RFC.
I only have one question, do you mind outlining where/how you'd wire OnNetworkDespawn events on the server and clients?
Some neighbor codes and/or call-stack to get to the OnNetworkDespawn would be great!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this to the reference level section of the RFC. I also plan to do some small refactoring around how NetworkStart is called (no behaviour changes just throwing out unused legacy code)


The internal function will be used so that NetworkBehaviours can subscribe to internal MLAPI events.

`OnNetworkDespawn` will also be called on the server when a `NetworkObject` gets destroyed because that also counts as a despawn.

Choose a reason for hiding this comment

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

Surley this should only occur if IsSpawned is true? Otherwise you could have a NetworkObject that is never actually spawned, (thus OnNetworkSpawn is never called) but OnNetworkDespawn is called.

Choose a reason for hiding this comment

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

OnNetworkDespawn is very interesting to me, would this also give me room to gracefully fade away the object over some time on client or the object would be destroyed anyway before I could fade it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You won't be able to use OnNetworkDespawn for this specific use case because MLAPI would still force destroy the object after. In this case you would need to implement a custom destroy handler to override the destroy logic of MLAPI:
https://docs-multiplayer.unity3d.com/docs/advanced-topics/object-pooling

@TwoTenPvP Yes, correct this will only occur if IsSpawned is true.

Choose a reason for hiding this comment

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

@LukeStampfli, Makes sense. thanks for pointing out the object-pooling.

Would OnNetworkDespawn also be called when server runs Show/HideNetwork?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently NetworkShow and NetworkHide spawn/despawn an object on the client and would trigger the OnNetworkSpawnandOnNetworkDespawn` functions

Copy link
Contributor

@0xFA11 0xFA11 left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

Though, only one side-question: Do you have any plans to let docs people know and help them get things up-to-date with this upcoming change?

@LukeStampfli
Copy link
Contributor Author

@MFatihMAR I was planning to let docs people know once the RFC is accepted and the PR is merged.

Copy link
Contributor

@JesseOlmer JesseOlmer left a comment

Choose a reason for hiding this comment

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

lgtm

@LukeStampfli LukeStampfli merged commit 878e503 into master Jun 7, 2021
@LukeStampfli LukeStampfli deleted the rfc/network-spawn branch June 7, 2021 16:04
@LukeStampfli LukeStampfli added accepted RFC has been accepted and removed open RFC is open for review and comments labels Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted RFC has been accepted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants