Skip to content

feat: AttachableBehaviour and ComponentController #3518

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

Open
wants to merge 44 commits into
base: develop-2.0.0
Choose a base branch
from

Conversation

NoelStephensUnity
Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity commented Jun 24, 2025

AttachableBehaviour and Support Components

The purpose of this PR (feat) is to address the complexity of "picking up" or "dropping" an item in the world which can become complex when using the traditional NetworkObject parenting pattern. In this PR there are three primary components added to help reduce this complexity:

  • AttachableBehaviour: Provides "out of the box" support for attaching (i.e. parenting) a nested child GameObject that includes an AttachableBehaviour component to another nested child GameObject with an AttachableNode component that is associated with a different NetworkObject.
  • AttachableNode: This component is required by the AttachableBehaviour component in order to be able to attach (i.e. parent) to another GameObject without having to parent the entire NetworkObject component the AttachableBehaviour component is associated with.
  • ComponentController: This component provides users the ability to synchronize the enabling or disabling of any Object derived component that has an enabled property.

This PR also incorporates a new "Helpers" subfolder under the NGO components folder where additional helper components will live.

Documentation Update

New Documentation

AttachableBehaviour

Network Components Section Update

image

New Foundational Components Section

image

New Helper Components Section

image

NetworkBehaviour.OnNetworkPreDespawn

Added another virtual method to NetworkBehaviour, OnNetworkPreDespawn, that is invoked before running through the despawn sequence for the NetworkObject and all NetworkBehaviour children of the NetworkObject being despawned. This provides an opportunity to do any kind of cleanup up or last micro-second state updates prior to despawning.

Changelog

  • Added: AttachableBehaviour helper component to provide an alternate approach to parenting items without using the NetworkObject parenting.
  • Added : AttachableNode helper component that is used by AttachableBehaviour as the target node for parenting.
  • Added: ComponentController helper component that can be used to synchronize the enabling and disabling of components and can be used in conjunction with AttachableBehaviour.
  • Added: NetworkBehaviour.OnNetworkPreDespawn that is invoked before running through the despawn sequence for the NetworkObject and all NetworkBehaviour children of the NetworkObject being despawned.

Testing and Documentation

  • Includes two new integration tests:
    • AttachableBehaviourTests
    • ComponentControllerTests
  • PR-42 contains the manual test.
  • Documentation:
    • Added new documentation section.
    • Refactored network components section.
    • Fixed some image links.

Backport

This is a v2.x only feature.

Adding AttachableBehaviour and ObjectController.
Renaming ObjectController to ComponentController.
Added some additional validation checking and handling.
Updated XML API.
Adding helpers meta.
Adding an AttachableNode as the target for AttachableBehaviour.
Refactoring AttachableBehaviour.
Adding new test for attachables.
Replacing any improperly spelled "detatch" with "detach".
XML API and private methods.
Minor XML API fixes.
Simplified the nameof AttachableBehaviour.Detach to just Detach.
Refactoring the ComponentController to provide more flexibility as well as being able to have component entries that will apply the inverse of the current ComponentController's current state....which allows for switching between different sets of components depending upon the controller's state.
Made some minor adjustments while writing the base test for ComponentController.
Adding the base ComponentController test.
switching to a Light component as opposed to BoxCollider as BoxCollider requires the physics package and we are just testing the functionality of ComponentController and not specifically any one other type of component.
Removing using directive.
Updating the change log entries.
@NoelStephensUnity NoelStephensUnity marked this pull request as ready for review July 10, 2025 19:43
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner July 10, 2025 19:43
Removing debug log info.
Work in progress adjustments for usability under various scenarios.
Using RPCs and synchronizing the property being set using NetworkBehaviour.OnSynchronize in order to assure order of operations when it comes to messages.

Updated the ComponentController to be able to stagger state updates in the event this occurs (wip and I might remove this part and not allow changes to state until any pending state is finished).
Minor adjustment to the range for delays allow it to be zero.
Fixing spelling issues.
Had to make some minor adjustments in order to assure that users could handle sending any last micro-second tasks on any spawned instances prior to them despawning.

Added NetworkBehaviour.OnNetworkPreDespawn.
Did a slight order of operations on NetworkManager.Shutdown internal in order to assure sending RPCs during despawn would still work.

Minor adjustments to the new helper components associated with this PR.
Updating the AttachableBehaviourTests to include testing that an attachable will be automatically detatched upon despawning the AttachableNode it is attached to.
Updating changelog entry.
Doing a second inspector UI pass to include tool tips and rename each element item to the component's standard inspector view naming where it is the GameObject's name followed by the class name that is separated by capitalization and contained within parenthesis.
Fixing an exception that can occur when you have a network prefab opened for editing and then you delete the prefab asset before exiting the prefab edit mode.
Fixing some PVP related issues.
Several improvements on the attach and detach processing.
Added the ability to tie ComponentControllers to AttachableBehaviours in order to auto-notify when something is attaching and detaching.
Several adjustments to fix the issue with ungraceful disconnects and re-synchronizing attachables.
Added forced change based on flags applied, things like when an AttachableNode is despawning, changing ownership, or being destroyed then local instances, whether authority or not, will all force the attach or detach  state.
Added an internal virtual destroy method on NetworkBehaviour to allow for helper components to assure on destroy script is invoked.
Adding documentation to an undocumented enum value.
Updating the core components based on some bugs discovered during testing.
Updated the AttachableBehaviourTests to validate the different types of auto-detach flag combinations.
Fixed some spelling issues with "detatch" (not sure why I got into that habit)...corrected to "detach".
Adjusting the component section from a file and folder context.
Adjusting the component section for the table of contents.
Adding foundational components and helpers sub-sections to network components section.
Moved foundational components into the foundational (components) folder and updated links and image paths.
Adding AttachableBehaviour document along with images.
Added some place holders for AttachableNode and ComponentController.
Fixing some invalid image paths.
Removing white space at the end of lines.
Removing trickier white spaces...
@sentinel-u3d sentinel-u3d bot requested a review from EmandM July 30, 2025 17:50
Adding `AttachableNode` and `ComponentController` sections.
Added "Synchronized RPC driven fields" section to networkbehaviour-synchronize.md.
Updated various areas based on the additions.
Resolving more white space issues.
Removing a single sneaky white space.
@@ -218,7 +218,7 @@ private static void CheckPrefabStage(PrefabStage prefabStage)
s_PrefabAsset = AssetDatabase.LoadAssetAtPath<NetworkObject>(s_PrefabStage.assetPath);
}

if (s_PrefabInstance.GlobalObjectIdHash != s_PrefabAsset.GlobalObjectIdHash)
if (s_PrefabAsset && s_PrefabInstance && s_PrefabInstance.GlobalObjectIdHash != s_PrefabAsset.GlobalObjectIdHash)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This code is inside a block of if (s_PrefabInstance) {} (line 200)

Suggested change
if (s_PrefabAsset && s_PrefabInstance && s_PrefabInstance.GlobalObjectIdHash != s_PrefabAsset.GlobalObjectIdHash)
if (s_PrefabAsset && s_PrefabInstance.GlobalObjectIdHash != s_PrefabAsset.GlobalObjectIdHash)

/// Used for naming each element entry.
/// </summary>
[HideInInspector]
public string name;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mind explaining what this does?

/// as the <see cref="NetworkObject"/>, this will automatically create a child and add an
/// <see cref="AttachableBehaviour"/> to that.
/// </remarks>
protected virtual void OnValidate()
Copy link
Collaborator

Choose a reason for hiding this comment

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

To my untrained eye, it looks like there's two OnValidate functions and only one will run? What's the difference between this OnValidate and the function on line 45?

Comment on lines +99 to +103
if (componentController == null)
{
continue;
}
componentController.OnValidate();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Definitely a nitpick: null propagation can simplify this block.

Using the null conditional allows you to change

            foreach (var componentController in ComponentControllers)
            {
                if (componentController == null)
                {
                    continue;
                }
                componentController.OnValidate();
            }

to

            foreach (var componentController in ComponentControllers)
            {
                componentController?.OnValidate();
            }

Comment on lines +195 to +205
protected AttachState m_AttachState { get; private set; }

/// <summary>
/// The original parent of this <see cref="AttachableBehaviour"/> instance.
/// </summary>
protected GameObject m_DefaultParent { get; private set; }

/// <summary>
/// If attached, attaching, or detaching this will be the <see cref="AttachableNode"/> this <see cref="AttachableBehaviour"/> instance is attached to.
/// </summary>
protected AttachableNode m_AttachableNode { get; private set; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of interest, why do these need to be protected rather than internal?

}

var propertyInfo = Components[i].Component.GetType().GetProperty("enabled", BindingFlags.Instance | BindingFlags.Public);
if (propertyInfo == null && propertyInfo.PropertyType != typeof(bool))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm guessing we don't want to check for is null && has property?

// Scan the GameObject and all of its children and add all valid components to the list.
foreach (var entry in gameObjectsToScan)
{
var asGameObject = entry.Component as GameObject;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it make sense to change gameObjectsToScan from List<ComponentEntry> to List<GameObject>?

gameObjectsToScan.Clear();

// Final (third) pass is to name each list element item as the component is normally viewed in the inspector view.
for (int i = 0; i < Components.Count; i++)
Copy link
Collaborator

Choose a reason for hiding this comment

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

very nit: This might be simpler to read as a foreach loop. I do not have strong feelings about this.

Comment on lines +308 to +309
// Example of how to synchronize late joining clients when using an RPC to update
// a local property's state.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this comment valid here?

/// to have the owner be the authority of this <see cref="ComponentController"/> instance.
/// </remarks>
/// <returns>true = has authoriy | false = does not have authority</returns>
protected virtual bool OnHasAuthority()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment as for AttachableBehaviour here

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.

2 participants