Skip to content

refactor: add [RequireComponent(typeof(NetworkManager))] to NetworkAuthenticator#4116

Merged
miwarnec merged 4 commits into
masterfrom
copilot/analyze-hierarchy-of-dependencies
Jun 27, 2026
Merged

refactor: add [RequireComponent(typeof(NetworkManager))] to NetworkAuthenticator#4116
miwarnec merged 4 commits into
masterfrom
copilot/analyze-hierarchy-of-dependencies

Conversation

Copilot AI commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

NetworkAuthenticator doesn't work without NetworkManager, but the dependency was never declared explicitly.

Changes

  • NetworkAuthenticator.cs: Added [RequireComponent(typeof(NetworkManager))] to the base class, making the dependency explicit at the Unity level.

No change in user-facing behavior — Unity will auto-add a NetworkManager if one isn't present when adding an authenticator.

…r to NetworkManager

NetworkManager owns the authenticator reference, so it should own the
wiring logic. This removes the backwards dependency where
NetworkAuthenticator needed to know about NetworkManager.
@codecov

codecov Bot commented Jun 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.19%. Comparing base (b25a14b) to head (090893f).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4116   +/-   ##
=======================================
  Coverage   42.19%   42.19%           
=======================================
  Files         156      156           
  Lines       15092    15092           
=======================================
  Hits         6368     6368           
  Misses       8724     8724           
Flag Coverage Δ
unittests 42.19% <ø> (ø)
unity-6000.5.0f1 ?
unity-6000.5.1f1 42.19% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
Assets/Mirror/Core/NetworkAuthenticator.cs 0.00% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MrGadget1024

Copy link
Copy Markdown
Collaborator

@copilot NetworkAuthenticator doesn't work without NetworkManager. It should have [RequireComponent(typeof(NetworkManager))] on the base class. NetworkManager's Reset doesn't auto-fire when component added to same object in Unity.

…thenticator

Reverts the Reset() move — Unity calls Reset() on the component being
added, not on siblings, so NetworkManager.Reset() wouldn't fire when
adding an authenticator.

Instead, makes the dependency explicit with RequireComponent and keeps
the existing auto-assignment in NetworkAuthenticator.Reset().

Copilot AI commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

@copilot NetworkAuthenticator doesn't work without NetworkManager. It should have [RequireComponent(typeof(NetworkManager))] on the base class. NetworkManager's Reset doesn't auto-fire when componen...

Good catch — Unity calls Reset() on the component being added, not on siblings, so the move to NetworkManager.Reset() wouldn't actually trigger when adding an authenticator.

Reverted the move and added [RequireComponent(typeof(NetworkManager))] to NetworkAuthenticator instead to make the dependency explicit. The existing Reset() auto-assignment stays in NetworkAuthenticator where Unity will actually call it. See 2cc6c5b.

Copilot AI changed the title refactor: move authenticator auto-assignment from NetworkAuthenticator to NetworkManager refactor: add [RequireComponent(typeof(NetworkManager))] to NetworkAuthenticator Jun 27, 2026
Copilot AI requested a review from MrGadget1024 June 27, 2026 20:34
@miwarnec

Copy link
Copy Markdown
Collaborator

@copilot remove authenticator reset modifying networkmanager. instead networkmanager.awake can check if authenticator==null and getcomponent, then auto assign it

Remove Reset() from NetworkAuthenticator — it no longer modifies
NetworkManager. Instead, NetworkManager.Awake() auto-assigns the
authenticator via GetComponent if authenticator is null.

Copilot AI commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

@copilot remove authenticator reset modifying networkmanager. instead networkmanager.awake can check if authenticator==null and getcomponent, then auto assign it

Done in 95f526d. Removed Reset() from NetworkAuthenticator entirely and added the auto-assign in NetworkManager.Awake() — checks if (authenticator == null) then GetComponent<NetworkAuthenticator>().

Copilot AI changed the title refactor: add [RequireComponent(typeof(NetworkManager))] to NetworkAuthenticator refactor: decouple NetworkAuthenticator from NetworkManager Jun 27, 2026
@miwarnec

Copy link
Copy Markdown
Collaborator

@copilot nvm only do the requirecomponent thing, nothing else

…orkAuthenticator

Reverts all other changes. The only modification is the RequireComponent
attribute on the NetworkAuthenticator base class.

Copilot AI commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

@copilot nvm only do the requirecomponent thing, nothing else

Reverted everything else in 7acfd19. The only change now is adding [RequireComponent(typeof(NetworkManager))] to NetworkAuthenticator.

Copilot AI changed the title refactor: decouple NetworkAuthenticator from NetworkManager refactor: add [RequireComponent(typeof(NetworkManager))] to NetworkAuthenticator Jun 27, 2026
@miwarnec miwarnec merged commit 31163ab into master Jun 27, 2026
@miwarnec miwarnec deleted the copilot/analyze-hierarchy-of-dependencies branch June 27, 2026 21:12
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.

3 participants