Add support for stations, overhaul zone detection#9
Open
JanSharp wants to merge 174 commits into
Open
Conversation
Add button to inspector to create it as well for clarity
…udioZonePlayerObject by exluding all Layers for colission
Clear ZoneIds temporarily before play mode and upload.
…e is a AudioZonePlayerObject by exluding all Layers for colission" This reverts commit 8817978.
Fix audio zone manager ZoneIdMapping not getting set to an empty array when all audio zones have been removed from the scene
…rObjectSync instead
Since the sync script is a child now
Rename IntegerAudioZonePlayerObjectSync to IntegerAudioZoneSync Add folder for all the audio zone syncing scripts
An option to remove the log spam since logging eats performance, which is a concern in the case of the local player changing zones and updating every remote player in the same frame
It's simply bad for readability. There is very rare cases where using var may increase maintainability since one could change the type of a variable and other things interacting with it may just automatically use the correct new type, but at the same time is the now changed logic actually going to be correct with the changed type? Or was it working on the assumption that it was an int specifically, and not a long for example When it comes to readability, having to sometimes read the left hand side and sometimes the right hand side of an assignment to figure out the type of a variable is simply annoying and ultimately wastes time, especially in cases where the type of the right hand side is not immediately obvious. Which a lot of the time it isn't, most of the time it requires parsing the whole right hand side to be certain
|
Thanks for all the hard work, u did it better than I would have been able to - especially the documentation xD
I dont really care about it. But feel free to add |
Specifically when there were audio or setting zones, on build ran, then in the same session the either all audio or all setting zones get removed, then on build runs, and it'd be the wrong count
Make missing managers a full error preventing entering play mode and building
Uncertain if this would be an issue in prefab instances or not, but this is known functional therefore good
Contributor
Author
Tried to :) react to that, apparently that's a laughing emoji, whoops! |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
High Level Description
See the Update Log in the readme of the PR for most user facing changes.
Technical Description
This PR changes the way audio zones are detected, removing logic using VRChat's player trigger enter and exit events, replacing said logic with physics overlap checks at the head's position 5 times per second for the local player.
Detected zones are kept track of and whenever they change the list of zones the local player is in gets synced to all players through a per player sync script.
This applies to both audio zones and setting zones, however for setting zones only 1 id gets synced, the active one.
Syncing is optimized for least network bandwidth utilization as possible. Well, almost, so within reason.
There are 5 different sync scripts of 2 different categories:
The system automatically picks the appropriate sync script to use at build time. In small scenes with few zones the 64, 128 or 192 bit sync scripts would be used, avoiding the stupidly high flat overhead of 64 bytes of a script with a single synced array. But even huge scenes with 4 billion audio zones would be supported, which is to say the sync scripts do not impose any artificial restriction.
The bit field sync scripts also improve checking whether two players share zones, it is
O(1), while the array sync scripts areO(n*log(n)).The editor scripting has been refactored significantly, deduplicating the logic for audio and setting zones, generally cleaning it up, though most of this is also stated in the update log already. Notably here is the use of serialized objects for all persistent changes made to objects, which makes unity automatically handle prefab overrides, flagging as dirty, undo, redo.
Performance is good, on my machine in the dev world it spikes to like 0.4 ms upon zones changing when all logging is disabled, otherwise uses less than 0.1 ms for each check, which it does 5 per second of, which averages out to like less than 0.01 ms per frame. More complex scenes in terms of physics geometry may be slower, and when the local player changes zones that is
O(n)with the amount of players in the world, however that was already the case beforehand.The option to disable info logging has been added because every log message - on my machine - takes about 0.6 ms. And again, local player changes is
O(n)with the amount of players, in the worst case scenario of 80 people in the world, there are 2 log messages when voice settings get changed (the new settings and the start fade log message), so a local player changing zones could cause all players to have their settings changed. That would be a spike of79 * 2 * 0.6 = 94.8ms just spent on logging messages, in theory. Pretty bad.Could we disable the logging of fade start and end entirely (as in changing it to be debug logging, off by default)?
This is a very large PR, it really should have been split into several smaller ones, though it's kind of hard to with several changes depending on each other. So this PR is truly not just adding support for stations... it's a significant refator. Which is hard to review and likely bad practice, my apologies.
The kill switch probably should be removed for the final release? Unsure.
Misc
Don't forget to update the license year in both of the LICENSE files,
2023-2026. (And probably make your name match in both of them, though I suppose maybe you did that intentionally.)Also note that Unity wants you to call the license file inside of the project very specifically
LICENSE.md, which enables the hyperlink in the package manager window in Unity. Though this is not important at the end of the day.I would also like to request, if you woudln't mind, to be added to the license files. For example:
I will not speak for Fallen in this regard, leaving it up to him what his decision in regards to the licensing and copyright subject might be.