diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 93b7361a5db..1a0a5c7c286 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -35,7 +35,11 @@ END TEMPLATE--> ### Breaking changes -*None yet* +* The interaction between PVS overrides and visibility masks / layers have changed: + * Any forced entities (i.e., `PvsOverrideSystem.AddForceSend()`) now ignore visibility masks. + * Any global & session overrides (`PvsOverrideSystem.AddGlobalOverride()` & `PvsOverrideSystem.AddSessionOverride()`) now respect visibility masks. + * Entities added via the `ExpandPvsEvent` respect visibility masks. + * The mask used for any global/session overrides can be modified via `ExpandPvsEvent.Mask`. ### New features @@ -43,7 +47,7 @@ END TEMPLATE--> ### Bugfixes -*None yet* +* Audio entities attached to invisible / masked entities should no longer be able to temporarily make those entities visible to all players. ### Other diff --git a/Robust.Server/Audio/AudioSystem.cs b/Robust.Server/Audio/AudioSystem.cs index 981c01e770e..163df02f5b8 100644 --- a/Robust.Server/Audio/AudioSystem.cs +++ b/Robust.Server/Audio/AudioSystem.cs @@ -104,6 +104,11 @@ public override (EntityUid Entity, AudioComponent Component)? PlayEntity(string? var entity = SetupAudio(filename, audioParams); // Move it after setting it up XformSystem.SetCoordinates(entity, new EntityCoordinates(uid, Vector2.Zero)); + + // TODO AUDIO + // Add methods that allow for custom audio range. + // Some methods try to reduce the audio range, resulting in a custom filter which then unnecessarily has to + // use PVS overrides. PlayEntity with a reduced range shouldn't need PVS overrides at all. AddAudioFilter(entity, entity.Comp, playerFilter); return (entity, entity.Comp); diff --git a/Robust.Server/GameStates/PvsData.cs b/Robust.Server/GameStates/PvsData.cs index c4df89b5c44..a2f5708824d 100644 --- a/Robust.Server/GameStates/PvsData.cs +++ b/Robust.Server/GameStates/PvsData.cs @@ -180,6 +180,9 @@ internal struct PvsMetadata public NetEntity NetEntity; public GameTick LastModifiedTick; + + // TODO PVS maybe store as int? + // Theres extra space anyways, and the mask checks always need to convert to an int first, so it'd probably be faster too. public ushort VisMask; public EntityLifeStage LifeStage; #if DEBUG diff --git a/Robust.Server/GameStates/PvsOverrideSystem.cs b/Robust.Server/GameStates/PvsOverrideSystem.cs index bef471af085..b9fc16bafc7 100644 --- a/Robust.Server/GameStates/PvsOverrideSystem.cs +++ b/Robust.Server/GameStates/PvsOverrideSystem.cs @@ -132,7 +132,7 @@ private void Clear(EntityUid uid) /// /// Forces the entity, all of its parents, and all of its children to ignore normal PVS range limitations, - /// causing them to always be sent to all clients. + /// causing them to be sent to all clients. This will still respect visibility masks, it only overrides the range. /// public void AddGlobalOverride(EntityUid uid) { @@ -154,8 +154,9 @@ public void RemoveGlobalOverride(EntityUid uid) /// This causes an entity and all of its parents to always be sent to all players. /// /// - /// This differs from as it does not send children, and will ignore a players usual - /// PVS budget. You generally shouldn't use this unless an entity absolutely always needs to be sent to all clients. + /// This differs from as it does not send children, will ignore a players usual + /// PVS budget, and ignores visibility masks. You generally shouldn't use this unless an entity absolutely always + /// needs to be sent to all clients. /// public void AddForceSend(EntityUid uid) { @@ -171,11 +172,12 @@ public void RemoveForceSend(EntityUid uid) } /// - /// This causes an entity and all of its parents to always be sent to a player.. + /// This causes an entity and all of its parents to always be sent to a player. /// /// - /// This differs from as it does not send children, and will ignore a players usual - /// PVS budget. You generally shouldn't use this unless an entity absolutely always needs to be sent to a client. + /// This differs from as it does not send children, will ignore a players usual + /// PVS budget, and ignores visibility masks. You generally shouldn't use this unless an entity absolutely always + /// needs to be sent to a client. /// public void AddForceSend(EntityUid uid, ICommonSession session) { @@ -201,7 +203,7 @@ public void RemoveForceSend(EntityUid uid, ICommonSession session) /// /// Forces the entity, all of its parents, and all of its children to ignore normal PVS range limitations for a - /// specific session. + /// specific session. This will still respect visibility masks, it only overrides the range. /// public void AddSessionOverride(EntityUid uid, ICommonSession session) { @@ -226,13 +228,15 @@ public void RemoveSessionOverride(EntityUid uid, ICommonSession session) /// /// Forces the entity, all of its parents, and all of its children to ignore normal PVS range limitations, - /// causing them to always be sent to all clients. + /// causing them to always be sent to the specified clients. This will still respect visibility masks, it only + /// overrides the range. /// public void AddSessionOverrides(EntityUid uid, Filter filter) { + _hasOverride.Add(uid); foreach (var session in filter.Recipients) { - AddSessionOverride(uid, session); + SessionOverrides.GetOrNew(session).Add(uid); } } diff --git a/Robust.Server/GameStates/PvsSystem.Overrides.cs b/Robust.Server/GameStates/PvsSystem.Overrides.cs index badd3964145..550e61e5d78 100644 --- a/Robust.Server/GameStates/PvsSystem.Overrides.cs +++ b/Robust.Server/GameStates/PvsSystem.Overrides.cs @@ -19,14 +19,15 @@ internal sealed partial class PvsSystem private void AddAllOverrides(PvsSession session) { - var mask = session.VisMask; var fromTick = session.FromTick; - RaiseExpandEvent(session, fromTick); + var mask = RaiseExpandEvent(session, fromTick); foreach (ref var ent in CollectionsMarshal.AsSpan(_cachedGlobalOverride)) { ref var meta = ref _metadataMemory.GetRef(ent.Ptr.Index); meta.Validate(ent.Meta); + + // PVS overrides still respect visibility masks if ((mask & meta.VisMask) == meta.VisMask) AddEntity(session, ref ent, ref meta, fromTick); } @@ -36,7 +37,7 @@ private void AddAllOverrides(PvsSession session) foreach (var uid in sessionOverrides) { - RecursivelyAddOverride(session, uid, fromTick, addChildren: true); + RecursivelyAddOverride(session, uid, fromTick, addChildren: true, mask); } } @@ -45,22 +46,23 @@ private void AddAllOverrides(PvsSession session) /// private void AddForcedEntities(PvsSession session) { + // Forced overrides do not respect visibility masks, so we set all bits. + var mask = -1; + // Ignore PVS budgets session.Budget = new() {NewLimit = int.MaxValue, EnterLimit = int.MaxValue}; - var mask = session.VisMask; var fromTick = session.FromTick; foreach (ref var ent in CollectionsMarshal.AsSpan(_cachedForceOverride)) { ref var meta = ref _metadataMemory.GetRef(ent.Ptr.Index); meta.Validate(ent.Meta); - if ((mask & meta.VisMask) == meta.VisMask) - AddEntity(session, ref ent, ref meta, fromTick); + AddEntity(session, ref ent, ref meta, fromTick); } foreach (var uid in session.Viewers) { - RecursivelyAddOverride(session, uid, fromTick, addChildren: false); + RecursivelyAddOverride(session, uid, fromTick, addChildren: false, mask); } if (!_pvsOverride.SessionForceSend.TryGetValue(session.Session, out var sessionForce)) @@ -68,13 +70,13 @@ private void AddForcedEntities(PvsSession session) foreach (var uid in sessionForce) { - RecursivelyAddOverride(session, uid, fromTick, addChildren: false); + RecursivelyAddOverride(session, uid, fromTick, addChildren: false, mask); } } - private void RaiseExpandEvent(PvsSession session, GameTick fromTick) + private int RaiseExpandEvent(PvsSession session, GameTick fromTick) { - var expandEvent = new ExpandPvsEvent(session.Session); + var expandEvent = new ExpandPvsEvent(session.Session, session.VisMask); if (session.Session.AttachedEntity != null) RaiseLocalEvent(session.Session.AttachedEntity.Value, ref expandEvent, true); @@ -85,23 +87,25 @@ private void RaiseExpandEvent(PvsSession session, GameTick fromTick) { foreach (var uid in expandEvent.Entities) { - RecursivelyAddOverride(session, uid, fromTick, addChildren: false); + RecursivelyAddOverride(session, uid, fromTick, addChildren: false, expandEvent.VisMask); } } if (expandEvent.RecursiveEntities == null) - return; + return expandEvent.VisMask; foreach (var uid in expandEvent.RecursiveEntities) { - RecursivelyAddOverride(session, uid, fromTick, addChildren: true); + RecursivelyAddOverride(session, uid, fromTick, addChildren: true, expandEvent.VisMask); } + + return expandEvent.VisMask; } /// /// Recursively add an entity and all of its parents to the to-send set. This optionally also adds all children. /// - private bool RecursivelyAddOverride(PvsSession session, EntityUid uid, GameTick fromTick, bool addChildren) + private bool RecursivelyAddOverride(PvsSession session, EntityUid uid, GameTick fromTick, bool addChildren, int mask) { if (!_xformQuery.TryGetComponent(uid, out var xform)) { @@ -116,17 +120,20 @@ private bool RecursivelyAddOverride(PvsSession session, EntityUid uid, GameTick // to the toSend set, it doesn't guarantee that its parents have been. E.g., if a player ghost just teleported // to follow a far away entity, the player's own entity is still being sent, but we need to ensure that we also // send the new parents, which may otherwise be delayed because of the PVS budget. - if (parent.IsValid() && !RecursivelyAddOverride(session, parent, fromTick, false)) + if (parent.IsValid() && !RecursivelyAddOverride(session, parent, fromTick, false, mask)) return false; if (!_metaQuery.TryGetComponent(uid, out var meta)) return false; + if ((mask & meta.VisibilityMask) != meta.VisibilityMask) + return false; + if (!AddEntity(session, (uid, meta), fromTick)) return false; if (addChildren) - RecursivelyAddChildren(session, xform, fromTick); + RecursivelyAddChildren(session, xform, fromTick, mask); return true; } @@ -134,7 +141,7 @@ private bool RecursivelyAddOverride(PvsSession session, EntityUid uid, GameTick /// /// Recursively add an entity and all of its children to the to-send set. /// - private void RecursivelyAddChildren(PvsSession session, TransformComponent xform, GameTick fromTick) + private void RecursivelyAddChildren(PvsSession session, TransformComponent xform, GameTick fromTick, int mask) { foreach (var child in xform._children) { @@ -145,10 +152,14 @@ private void RecursivelyAddChildren(PvsSession session, TransformComponent xform } var metadata = _metaQuery.GetComponent(child); + + if ((mask & metadata.VisibilityMask) != metadata.VisibilityMask) + continue; + if (!AddEntity(session, (child, metadata), fromTick)) - return; + return; // Budget was exceeded (or some error occurred) -> return instead of continue. - RecursivelyAddChildren(session, childXform, fromTick); + RecursivelyAddChildren(session, childXform, fromTick, mask); } } diff --git a/Robust.Server/GameStates/PvsSystem.ToSendSet.cs b/Robust.Server/GameStates/PvsSystem.ToSendSet.cs index 5dfa5b949bd..99e3e86b5c0 100644 --- a/Robust.Server/GameStates/PvsSystem.ToSendSet.cs +++ b/Robust.Server/GameStates/PvsSystem.ToSendSet.cs @@ -142,18 +142,32 @@ private bool AddEntity(PvsSession session, Entity entity, Gam if (meta.EntityLifeStage >= EntityLifeStage.Terminating) { - var rep = new EntityStringRepresentation(entity); - Log.Error($"Attempted to add a deleted entity to PVS send set: '{rep}'. Deletion queued: {EntityManager.IsQueuedForDeletion(uid)}. Trace:\n{Environment.StackTrace}"); - // This can happen if some entity was some removed from it's parent while that parent was being deleted. // As a result the entity was marked for deletion but was never actually properly deleted. - EntityManager.QueueDeleteEntity(uid); + + bool queued; + lock (_toDelete) + { + queued = EntityManager.IsQueuedForDeletion(uid) || _toDelete.Contains(uid); + if (!queued) + _toDelete.Add(uid); + } + + var rep = new EntityStringRepresentation(entity); + Log.Error($"Attempted to add a deleted entity to PVS send set: '{rep}'. Deletion queued: {queued}. Trace:\n{Environment.StackTrace}"); return false; } data.LastSeen = _gameTiming.CurTick; session.ToSend!.Add(entity.Comp.PvsData); + // TODO PVS PERFORMANCE + // Investigate whether its better to defer actually creating the entity state & populating session.States here? + // I.e., should be be constructing the to-send list & to-get-states lists, and then separately getting all states + // after we have gotten all entities? If the CPU can focus on only processing data in session.DataMemory without + // having to access miscellaneous component info, maybe it will be faster? + // Though for that to work I guess it also has to avoid accessing the metadata component's lifestage? + if (session.RequestedFull) { var state = GetFullEntityState(session.Session, uid, meta); diff --git a/Robust.Server/GameStates/PvsSystem.cs b/Robust.Server/GameStates/PvsSystem.cs index bb63968cd1b..8a38eb13f19 100644 --- a/Robust.Server/GameStates/PvsSystem.cs +++ b/Robust.Server/GameStates/PvsSystem.cs @@ -94,6 +94,8 @@ internal sealed partial class PvsSystem : EntitySystem /// private readonly List _deletedTick = new(); + private readonly HashSet _toDelete = new(); + /// /// The sessions that are currently being processed. Note that this is in general used by parallel & async tasks. /// Hence player disconnection processing is deferred and only run via . @@ -195,6 +197,12 @@ internal void SendGameStates(ICommonSession[] players) // Construct & serialize the game state for each player (and for the replay). SerializeStates(); + foreach (var uid in _toDelete) + { + EntityManager.QueueDeleteEntity(uid); + } + _toDelete.Clear(); + // Compress & send the states. SendStates(); @@ -461,18 +469,27 @@ private void AfterSerializeStates() } [ByRefEvent] -public struct ExpandPvsEvent(ICommonSession session) +public struct ExpandPvsEvent(ICommonSession session, int mask) { public readonly ICommonSession Session = session; /// - /// List of entities that will get added to this session's PVS set. + /// List of entities that will get added to this session's PVS set. This will still respect visibility masks. /// public List? Entities; /// /// List of entities that will get added to this session's PVS set. Unlike this will also - /// recursively add all children of the given entity. + /// recursively add all children of the given entity. This will still respect visibility masks. /// public List? RecursiveEntities; + + /// + /// Visibility mask to use when adding entities. Defaults to the usual visibility mask for that client. + /// + /// + /// Note that this mask will affect all global & session overrides from for this + /// client, not just the entities in and . + /// + public int VisMask = mask; }