Skip to content

fix: Add RequireComponent attribute to NetworkNavMeshAgent. - #615

Merged
mattwalsh-unity merged 1 commit into
release/0.1.0from
fix/networknavagent-requirecomponent
Mar 15, 2021
Merged

fix: Add RequireComponent attribute to NetworkNavMeshAgent.#615
mattwalsh-unity merged 1 commit into
release/0.1.0from
fix/networknavagent-requirecomponent

Conversation

@LukeStampfli

Copy link
Copy Markdown
Contributor

NetworkNavMeshAgent expects a NavMeshAgent and throws a null ref if it's not there. So this attribute should have been there to prevent user error.

@@ -10,6 +10,7 @@ namespace MLAPI.Prototyping
/// A prototype component for syncing NavMeshAgents

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NavMeshAgents -> NetworkNavMeshAgents, keeps grep clean

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this comment is fine. NetworkNavMeshAgent is a component to sync a regular NavMeshAgent.

/// A prototype component for syncing NavMeshAgents
/// </summary>
[AddComponentMenu("MLAPI/NetworkNavMeshAgent")]
[RequireComponent(typeof(NavMeshAgent))]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wouldn't this need to be ?

Suggested change
[RequireComponent(typeof(NavMeshAgent))]
[RequireComponent(typeof(NetworkNavMeshAgent))]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No this should be NavMeshAgent. The NetworkNavMeshAgent component requires that there also is a NavMeshAgent on the same object. That's why we are using this attribute.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, this is referring to the private NavMeshAgent m_Agent; in NetworkNavMeshAgent.cs

@mattwalsh-unity
mattwalsh-unity merged commit 25d341c into release/0.1.0 Mar 15, 2021
@mattwalsh-unity
mattwalsh-unity deleted the fix/networknavagent-requirecomponent branch March 15, 2021 20:54
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