Skip to content

refactor!: NetworkSceneManager is no longer static #738

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

Merged
merged 23 commits into from
Apr 19, 2021

Conversation

TwoTenPvP
Copy link
Contributor

BREAKING CHANGE: Access NetworkSceneManager with NetworkManager.Singleton.SceneManager

@@ -304,11 +308,6 @@ private void Init(bool server)
MessageHandler = new InternalMessageHandler(this);
}

NetworkSceneManager.RegisteredSceneNames.Clear();
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume we no longer need this because, even if we call Init() multiple times (e.g. call StartServer again after stopping) we are creating a new NetworkSceneManager, and the old one will be out of scope and be freed

Still, is there value in, if there already is a NetworkSceneManager, that we call perhaps a cleanup function on it that would do things like the code that was removed? And then ditto for the others here (CustomMessagingManager, BufferManager, etc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This way we do it currently fully cleans it up though. (Lists etc are actually thrown out instead of their Count being set to 0 and their underlying array still being present).

Copy link
Contributor

@mattwalsh-unity mattwalsh-unity left a comment

Choose a reason for hiding this comment

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

Mainly want your thoughts on the need for still doing the cleanup I pointed out

Base automatically changed from singleton-removal-custom-message-manager to develop April 16, 2021 08:59
@TwoTenPvP TwoTenPvP enabled auto-merge (squash) April 19, 2021 19:48
@TwoTenPvP TwoTenPvP merged commit 4a13060 into develop Apr 19, 2021
@TwoTenPvP TwoTenPvP deleted the singleton-removal-scene-manager branch April 19, 2021 20:10
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