-
Notifications
You must be signed in to change notification settings - Fork 447
feat: Instantiation payload support for INetworkPrefabInstanceHandler #3430
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
base: develop-2.0.0
Are you sure you want to change the base?
Changes from 11 commits
3ff9326
7f395ba
f0d49aa
3f34e83
47680f5
b8765a2
de9fc05
6419ad0
1d746fe
4005c50
a103e49
43e8ff4
0fab0a6
36a041c
fd787e8
96b7af6
e49cb63
89eb5c4
adead9d
e4481b2
3522fc0
6757e63
a5670e3
426d82c
6899764
c1d6503
096c85c
7f6c9c7
89dd4c0
b992b7c
1f27a68
5b68637
d42bf95
4c7f63d
2b6befd
3dea6aa
015e384
536642b
fa774e7
0d62ea4
3fad66c
81d574a
36ec35f
7326a83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
namespace Unity.Netcode | ||
{ | ||
/// <summary> | ||
/// Interface for synchronizing custom instantiation payloads during NetworkObject spawning. | ||
/// Used alongside <see cref="INetworkPrefabInstanceHandler"/> to extend instantiation behavior. | ||
/// </summary> | ||
public interface INetworkInstantiationPayloadSynchronizer | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This name could be more descriptive. How about something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, this interface doesn't handle instantiation itself, that’s entirely the role of The purpose of If we go with the I used the term payload since it directly refers to the custom data being passed along with the spawn message, which is exactly what this interface handles I'll explore a few alternative naming options in tomorrow commit to see if any feel like a better fit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Along with this note, I was also thinking it might be nice if this new interface extends from the base interface. So rather than needing class MyHandler : INetworkPrefabInstanceHandler, INetworkInstantiationPayloadSynchronizer It could instead be used as class MyHandler : INetworkPrefabPayloadHandler or something of that type. Keeps it clearer for developers to implement and simpler to use. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a pretty good idea, i like how it looks simpler now, something like
What do you think, could we name it |
||
{ | ||
/// <summary> | ||
/// Provides a method for synchronizing instantiation payload data during the spawn process. | ||
/// Extends <see cref="INetworkPrefabInstanceHandler"/> to allow passing additional data prior to instantiation | ||
/// to help identify or configure the local object instance that should be linked to the spawned NetworkObject. | ||
/// | ||
/// This method is invoked immediately before <see cref="INetworkPrefabInstanceHandler.Instantiate"/> is called, | ||
/// allowing you to cache or prepare information needed during instantiation. | ||
/// </summary> | ||
/// <param name="serializer">The buffer serializer used to read or write custom instantiation data.</param> | ||
void OnSynchronize<T>(ref BufferSerializer<T> serializer) where T : IReaderWriter; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To avoid confusion with NetworkBehaviours, could we rename this to something more descriptive like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good to me! As long as the name clearly reflects that this method is solely responsible for synchronizing data prior to instantiation, I’m happy to update it. OnBeforeInstantiation works well for that, as long as we keep in mind that it is actually serializing and deserializing data There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might rename the interface to something like: This way, the naming makes it clearer that its about the data flow before instantiation, not the instantiation itself, and it also avoids confusion with NetworkBehaviour.OnSynchronize() Let me know if any of these seem closer to NGO’s naming conventions, or if you would prefer a shorter variation |
||
//void Serialize(FastBufferWriter writer); | ||
//void Deserialize(FastBufferReader reader); | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.