refactor: simplified interest management #990
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dragonslaya84
requested review from
uweeby,
Lymdun,
phodoval and
James-Frowen
dragonslaya84
changed the title
…otten proximity checker redone only. Has bug atm need fix later. Used james code from distance checker.
dragonslaya84
force-pushed
the
SimplifiedInterestManagement
branch
from
7451f01
to
ae7f26c
Compare
…i. Proximity checker currently only one that is finished and tested atm.
…null references issues due to adding aoi systems at runtime.
implemented scene visibility fixed docs ref. added new unregister so if gameobjects get destroyed they unregister from visibility system.
Implemented a new multi visibility system performance benchmark. Runs Scene + proximity systems.
|
I'm getting this error when opening a prefab, |
|
This is still creating 1 system per spawned, you can check using adding this to Update to test it |
Kk good catch. Its the code I am using for comparing. Maybe its faulty checking now. |
Fixed |
… objects global but not others everything will still work. New global system is internal cannot be used outside
registering in onenable is sometimes too early. It will also cause unspawned objects to be included in AOI, and maybe sent to clients Also adding todo
- NetworkVisibility to VisibilitySystem - NetworkProximityCheckerVisibility to DistanceVisibilitySystem - SceneVisibilityChecker to SceneVisibilitySystem
- _visibilitySystemData to _data - VisibilitySystemData Observers
moving Comparer so it is not hidden inside IM. Makes it more obvious what is going on when using the sturct in a hashset
we can just use the system itself since it'll have the reference to the dictionary
|
Please retry analysis of this Pull-Request directly on SonarCloud. |
- BaseVisibilityInspector to VisibilitySystemFactory - NetworkSceneChecker to SceneVisibilityFactory - NetworkProximityChecker to DistanceVisibilityFactory
we dont need this layer of abstraction
- removing base settings - making register take identity and settings - making unregister just take identity RegisterObject makes more sense if it is given the object, rather than settings that has the object. This will also reduce extra code if a system does not need any settings (if could just pass in null)
- renamed OnAuthenticated to RebuildForPlayer OnAuthenticated seems like a bad name because we will need to call it more than just OnAuthenticated, Like when we spawna new character for the player
making sure we still call IM functions are same time as before - need to rebuild observers for player after setting character - moving show to player to IM - adding OnSpawn and OnDestroy instead of using world events (we need to control the order, our event might be called before/after user added events)
can just use scene instead
changing how it is created. pass variables around rather than using fields
- InterestManager lazy intialize - VisibilitySystemFactory adds to IM on awake, and removes in destroy Key points: - IM always exists on SOM - Systems can and should be added before Server Starts - Systems will add them selves to IM when they are constructed
|
SonarCloud Quality Gate failed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.