-
Notifications
You must be signed in to change notification settings - Fork 4
NetworkObject Parenting #17
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
Conversation
| # Rationale and alternatives | ||
| [rationale-and-alternatives]: #rationale-and-alternatives | ||
|
|
||
| - Given the limited time and scope, we think we cannot re-architect MLAPI fundamentals to support an approach where `NetworkObject` and `NetworkBehaviour` components are simply unaware of scene and transform hierarchy but they are plus parenting to be provided by a higher-level construct outside MLAPI core such as `NetworkTransform`. We wanted to move forward with what we already have. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just wondering. If we moved this feature into a separate high level NetworkBehaviour NetworkParenting and just told users to add this to every NetworkObject root if they want automagical parenting. How would that increase scope? I do not see any technical blockers to do that? Wouldn't that be a better approach since we cannot re-architect MLAPI fundamentals?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can't because if we were to move a NetworkObject, that info needs to be written down to the NetworkObject itself — it's the main identification/linking layer between local and remote NetworkObjects. Even if we were to move the actual logic implementation up to the NetworkTransform (or NetworkParenting level), we still need to sync NetworkObjects, not NetworkTransform/NetworkParenting. At login, we still need to serialize NetworkObject's m_IsReparented, m_LastestParent etc.
having said that, all this feature design is subject to change when we start working on bigger refactorings and architectures. so, even though I'm not entirely happy with this proposal, I'm not too bothered to make it "just right" and/or "fancy" for now since all of it is temporary.
|
|
||
| ### Only Reparenting During Networking Is Valid | ||
|
|
||
| A `NetworkObject` can only be reparented while networking, in other terms you can only reparent while listening/running as a server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can an unspawned NetworkObject on the server be reparented while the server is listening/running?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope.
|
|
||
| ## Opt-out | ||
|
|
||
| Before we dig deeper into this feature proposal, it needs to be said that this feature is going to be behind a bool flag that can be toggled on the `NetworkObject` inspector UI, which will be enabled by default but experienced developers can opt-out from it and wouldn't be running any of this logic therefore they could to implement a solution on their own as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it's better to make this opt-out or opt-in. I can see arguments for both. I'm not taking any firm position here, but feel like it's worth discussing the merits of each side.
On the one hand, making it opt-out means that reparenting will work correctly without any effort from the developer (so long as they use it correctly, which isn't necessarily safe to assume here). But, on the other hand, given the limitations of automatic reparenting and the number of situations it doesn't work, making it opt-out may effectively result in giving developers rope to hang themselves with.
With the limitations, it might be better to make it opt-in, because the very fact that it requires the user to manually opt-in would be a signal to developers that there are limitations and gotchas and should be an incentive to read the documentation before enabling it. Reading the documentation for this seems like it'll be critical to make it work correctly, so having that checkbox as a funnel toward documentation (maybe even with a link to the documentation page in the help tooltip?) could improve developers' rate of success in using this feature.
Just food for thought, so to speak.
LukeStampfli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RFC looks fine to me when keeping in mind that we will remove / rework this again later.
I think some of the special cases such as "orphans" etc. could be quickly described in this RFC but besides that it's all good.
See the linked RFC document →
This RFC proposal is an attempt to outline basic needs for a
NetworkObject(re)parenting managed by the server at both scene authoring time and at runtime.There are a couple of limitations, however, we still want to deliver this functionality within MLAPI which would work out-of-the-box for some use-cases and some non-expert network programmers.