Skip to content

refactor: remove NetworkManager.Singleton dependencies, add additional instance tracking and notification when LogLevel.Developer #562

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 18 commits into from
Mar 10, 2021
Merged

refactor: remove NetworkManager.Singleton dependencies, add additional instance tracking and notification when LogLevel.Developer #562

merged 18 commits into from
Mar 10, 2021

Conversation

NoelStephensUnity
Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity commented Mar 9, 2021 •
edited
Loading

This refactor passes the RpcQueueContainer into the RpcQueueProcessor in order to completely remove the RpcQueueContainer's dependencies on the NetworkManager.Singleton.

This refactor removes RpcQueueContainer 's dependencies upon the NetworkManager.Singleton by passing the instance of the NetworkManager that constructed it via the constructor.

This should remove all dependencies from NetworkManager.Singleton from the entire Rpc Queue System!

Refactored the constructor and made the class disposable.

Simplified-Improve the constructor process.

If you set the NetworkLog.CurrentLevel to Developer it will notify you about:

  • If more than one instance is instantiated, it generates a log for each instantiation
  • As instances are disposed, it will notify you of this.
  • It has a "most likely to never ever occur" scenario where if there are no instances but the dispose method is being called by an already invalid/disposed reference.

@NoelStephensUnity
refactor: better handle instantiating and lack of a NetworkManager
81ea9a5 
This refactoring includes additional checks to assure the RpcQueueContainer does not allow itself to be updated via the Network Update Loop system if there is no NetworkManager instantiated or if there is a problem with its initialization.

Refactored the constructor and made the class disposable.
@NoelStephensUnity
fix: increment the instance count before logging
3890232 
Placing the increment above the log.
@NoelStephensUnity
Revert "fix: increment the instance count before logging"
defc3ce 
This reverts commit 3890232.
@NoelStephensUnity
fix: increment before log
1030171 
Moving the instance count above the logging of the instance count
@NoelStephensUnity
style: typo on the shutting down message
1790055 
Had a typo that said it was initializing as opposed to shutting down.
@NoelStephensUnity
refactor: Remove all NetworkManager.Singleton dependencies
e43b18b 
This refactor passes the RpcQueueContainer into the RpcQueueProcessor in order to start remove the RpcQueueContainer's dependencies on the NetworkManager.Singleton.

This refactor removes RpcQueueContainer 's dependencies upon the NetworkManager.Singleton by passing the instance of the NetworkManager that constructed it.

This should remove all dependencies from NetworkManager.Singleton from the entire Rpc Queue System!
@NoelStephensUnity NoelStephensUnity changed the title refactor: better handle instantiating and detecting a lack of NetworkManager availability refactor: remove NetworkManager.Singleton dependencies, add additional instance tracking and notification when LogLevel.Developer Mar 9, 2021
@NoelStephensUnity
refactor: further improved dependencies and expanded unit testing
c8b44bf 
This commit further improves the previous dependencies upon NetworkManager.
This also expands unit testing capabilities that now can include RpcQueueProcessor.
( just detecting if we are listening or testing )
@NoelStephensUnity @0xFA11
Update com.unity.multiplayer.mlapi/Runtime/Messaging/RpcQueue/RpcQueu…
198693d 
…eContainer.cs


Good addition, this will assure that anyone that changes the name of the class will see the name mismatch and update the log,

Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com>
@NoelStephensUnity @0xFA11
Update com.unity.multiplayer.mlapi/Runtime/Messaging/RpcQueue/RpcQueu…
995db9e 
…eContainer.cs


Same catch.. I like that approach!

Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com>
@NoelStephensUnity
style and developer:
beffa6a 
Updated some minor standards related issues and wrapped all of the debug logging related code in #if UNITY_EDITOR || DEVELOPMENT_BUILD
@NoelStephensUnity
Merge branch 'fix/rpcstartupshutdown' of https://github.com/Unity-Tec…
7de00a7 
…hnologies/com.unity.multiplayer.mlapi into fix/rpcstartupshutdown
@NoelStephensUnity
fix:
0c5a194 
Typo in Fatih's committed suggested fix.
:alien:
@NoelStephensUnity
refactor: updated minor debug messages.
67c6f56 
This assures that all log related messages utilize the appropriate class names.
This removes some legacy language from the log related messages.
@NoelStephensUnity
Merge branch 'release/0.1.0' into fix/rpcstartupshutdown
68f4683
NoelStephensUnity and others added 3 commits March 9, 2021 19:00
@NoelStephensUnity
style
5e09060 
Some minor tweaks for camelCase and spacing.
@NoelStephensUnity
style
25c9951 
Filled out the rest of the summary for the NetworkUpdate metod in RpcQueueContainer.
@0xFA11
minor update
a5082f4
0xFA11
0xFA11 approved these changes Mar 10, 2021
View reviewed changes
Copy link
Contributor

@0xFA11 0xFA11 left a comment

Choose a reason for hiding this comment

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

🚀

@NoelStephensUnity
refactor:
2737014 
Wrapping  UNITY_EDITOR || DEVELOPMENT_BUILD around a log message and the s_RpcQueueContainerInstances property.
@NoelStephensUnity NoelStephensUnity merged commit 3673ffc into release/0.1.0 Mar 10, 2021
@NoelStephensUnity NoelStephensUnity deleted the fix/rpcstartupshutdown branch March 10, 2021 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattwalsh-unity mattwalsh-unity mattwalsh-unity left review comments

@0xFA11 0xFA11 0xFA11 approved these changes

@LukeStampfli LukeStampfli Awaiting requested review from LukeStampfli

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@NoelStephensUnity @0xFA11 @mattwalsh-unity