Skip to content
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

refactor: improvements to visiblity system #948

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

dragonslaya84
Copy link
Member

@dragonslaya84 dragonslaya84 commented Sep 25, 2021

Only implements the interest manager that will auto check if any systems are added if not just use all current players on server to send data. Much faster and there is no need to do checks.

Next step will be to implement different systems people may need or use.

Please test and comment. All users using mirage what type of systems do you think you will need.? I only want to implement what end users are using or implement as end users need them.

Implementing there own system is very easy as creating a new class and overriding the NetworkVisibility system. That creates a pure C# system that interest manager finds. If you require showing the information in inspector you need to create a 2nd file that inherits from BaseVisibilityInspector.

I found it better this way for full control. You would implement both there is a single override that you only need to do and its the Start method of unity and implement anything inside there. The system will auto register itself using the base class.

  • Implement basic global system that just runs when no visibility system set,
  • Write tests
  • Spatial Hash visibility
  • Broadphase
  • Quad/Octotree visibility.

Signed-off-by: dragonslaya [email protected]

… all current systems and only implements the interest manager that will auto check if any systems are added if not just use all current players on server to send data. Much faster and there is no need to do checks.

Next step will be to implement different systems people may need or use.

Signed-off-by: dragonslaya <[email protected]>
…esting branch and not get spammed.

Signed-off-by: dragonslaya <[email protected]>

namespace Mirage.InterestManagement
{
public readonly struct ObserverData
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be a struct? doesn't seem like it is created in hot path?

/// </summary>
/// <param name="identity">The identity of the object we want to check if player's can see it or not.</param>
/// <returns></returns>
internal HashSet<INetworkPlayer> Observers(NetworkIdentity identity)
Copy link
Member

Choose a reason for hiding this comment

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

The way this looks atm it will be called once for every send call, which will likely become very expensive compared to a field that we use now

Copy link
Member Author

@dragonslaya84 dragonslaya84 Sep 26, 2021

Choose a reason for hiding this comment

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

We use a field in network identity. That field gets updated by current visibility systems through update method. Its almost exact same thing. This is now a central system versus network identity holding and updating info of visibility which it never should be doing. Open to suggestions but its almost identical to current system.

Also let's measure it. i am not sure how to measure current system since things are all over the place in many locations. I can currently measure performance just in Interest Manager.

changed loop to break out.

Signed-off-by: dragonslaya <[email protected]>
@MirageNet MirageNet deleted a comment from sonarcloud bot Sep 26, 2021
@James-Frowen
Copy link
Member

Some files from old PR to try here

public class DistanceConstantSightInterestManager : InterestManager

public class SpatialHashInterestManagementPerformanceTest : InterestManagementPerformanceBase

@github-actions github-actions bot added the has conflicts Pull Request has merge conflicts label Oct 21, 2021
Missed few files for the sample scene.
…erestManagement

# Conflicts:
#	Assets/Mirage/Components/NetworkMatchChecker.cs
#	Assets/Mirage/Components/NetworkSceneChecker.cs
#	Assets/Mirage/Runtime/NetworkIdentity.cs
#	Assets/Mirage/Runtime/ServerObjectManager.cs
#	Assets/Mirage/Samples~/InterestManagement/Scenes/AOI.unity
#	Assets/Mirage/Samples~/Tanks/Scenes/Scene.unity
#	Assets/Tests/Performance/Runtime/HeadlessBenchmark/Scenes/Scene.unity
@dragonslaya84 dragonslaya84 added keep-open Stops issue being closed because it is stale Work In Progess Shouldn't be merged yet and removed has conflicts Pull Request has merge conflicts labels Oct 23, 2021
@dragonslaya84
Copy link
Member Author

Some files from old PR to try here

public class DistanceConstantSightInterestManager : InterestManager

public class SpatialHashInterestManagementPerformanceTest : InterestManagementPerformanceBase

done

@github-actions github-actions bot added the has conflicts Pull Request has merge conflicts label Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has conflicts Pull Request has merge conflicts keep-open Stops issue being closed because it is stale Work In Progess Shouldn't be merged yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants