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

Make PVS overrides respect vismasks #5598

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

ElectroJr
Copy link
Member

Currently, if an entity is added to a session specific PVS override, it will ignore the normal visibility masks. This has lead to bugs where audio entities temporarily make entities that should be hidden visible to everyone in range of the sound (e.g., space-wizards/space-station-14#29801). This PR makes PVS overrides respect visibility masks, except for any "force send" entities, which now ignore them.

@PJB3005
Copy link
Member

PJB3005 commented Jan 16, 2025

This has lead to bugs where audio entities temporarily make entities that should be hidden visible to everyone in range of the sound (e.g., space-wizards/space-station-14#29801)

The actual bug here is that content was relying solely on PVS to avoid visibility of ghost entities, which is incorrect. Without making the root entity visible, how would audio entities know where to play with this change? Would they just not?

@ElectroJr
Copy link
Member Author

ElectroJr commented Jan 17, 2025

The actual bug here is that content was relying solely on PVS to avoid visibility of ghost entities, which is incorrect.

There is also a client-side system that should also be making ghosts invisible that I forgot about, which should've also been preventing that (i.e., see GhostSystem.OnStartup) . I guess I'll go try figure out why it wasn't working.

Edit:
Okay, for whatever fucking reason ghost visibility is set to true by default?

Without making the root entity visible, how would audio entities know where to play with this change? Would they just not?

The audio ents wouldn't get sent to the client, so the sound wouldn't play. IMO that's what should happen, sound shouldn't suddenly override PVS visibility masks. And it also acts as an extra safety net that invisible ghosts aren't going around accidentally emitting sounds, though again that should really also be ensured by some other system. I.e., ghosts still shouldn't emit sounds when PVS is disabled.

@PJB3005
Copy link
Member

PJB3005 commented Jan 17, 2025

There is also a client-side system that should also be making ghosts invisible that I forgot about, which should've also been preventing that (i.e., see GhostSystem.OnStartup) . I guess I'll go try figure out why it wasn't working.

The PR that added revenants broke it, IIRC.

@PJB3005
Copy link
Member

PJB3005 commented Jan 17, 2025

The audio ents wouldn't get sent to the client, so the sound wouldn't play. IMO that's what should happen, sound shouldn't suddenly override PVS visibility masks. And it also acts as an extra safety net that invisible ghosts aren't going around accidentally emitting sounds, though again that should really also be ensured by some other system. I.e., ghosts still shouldn't emit sounds when PVS is disabled.

Yeah alright.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants