Skip to content

Conversation

@0xFA11
Copy link
Contributor

@0xFA11 0xFA11 commented May 31, 2021

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.

@0xFA11 0xFA11 added the open RFC is open for review and comments label Jun 1, 2021
@0xFA11 0xFA11 marked this pull request as ready for review June 1, 2021 23:57
# 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.
Copy link
Contributor

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?

Copy link
Contributor Author

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.
Copy link
Contributor

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?

Copy link
Contributor Author

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.
Copy link
Contributor

@ShadauxCat ShadauxCat Jun 7, 2021

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.

Copy link
Contributor

@LukeStampfli LukeStampfli left a 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.

@0xFA11 0xFA11 merged commit 2761873 into master Jun 18, 2021
@0xFA11 0xFA11 deleted the rfc/object-parenting branch June 18, 2021 19:44
@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.

3 participants