RobustToolbox icon indicating copy to clipboard operation
RobustToolbox copied to clipboard
verified publisher icon space-wizards

We need WeakEntityReference or another solution to solve PVS error spam

Open slarticodefast opened this issue 4 months ago • 1 comments

The problem:

Example error message:

Can't resolve "Robust.Shared.GameObjects.MetaDataComponent" on entity 5233063!
   at System.Environment.get_StackTrace()
   at Robust.Shared.GameObjects.EntityManager.GetNetEntity(EntityUid uid, MetaDataComponent metadata) in /home/runner/work/space-station-14/space-station-14/RobustToolbox/Robust.Shared/GameObjects/EntityManager.Network.cs:line 186
   at Content.Shared.Audio.Jukebox.JukeboxComponent.JukeboxComponent_AutoNetworkSystem.OnGetState(EntityUid uid, JukeboxComponent component, ComponentGetState& args) in /home/runner/work/space-station-14/space-station-14/Content.Shared/obj/Release/Robust.Shared.CompNetworkGenerator/Robust.Shared.CompNetworkGenerator.ComponentNetworkGenerator/JukeboxComponent_CompNetwork.g.cs:line 45
   at Robust.Shared.GameObjects.EntityManager.GetComponentState(IEventBus eventBus, IComponent component, ICommonSession session, GameTick fromTick) in /home/runner/work/space-station-14/space-station-14/RobustToolbox/Robust.Shared/GameObjects/EntityManager.Components.cs:line 1704
   at Robust.Server.GameStates.PvsSystem.GetEntityState(ICommonSession player, EntityUid entityUid, GameTick fromTick, MetaDataComponent meta) in /home/runner/work/space-station-14/space-station-14/RobustToolbox/Robust.Server/GameStates/PvsSystem.GetStates.cs:line 52
   at Robust.Server.GameStates.PvsSystem.AddPvsChunk(PvsChunk chunk, Single distance, PvsSession session) in /home/runner/work/space-station-14/space-station-14/RobustToolbox/Robust.Server/GameStates/PvsSystem.ToSendSet.cs:line 62
   at Robust.Server.GameStates.PvsSystem.GetEntityStates(PvsSession session) in /home/runner/work/space-station-14/space-station-14/RobustToolbox/Robust.Server/GameStates/PvsSystem.cs:line 311
   at Robust.Server.GameStates.PvsSystem.SerializeSessionState(PvsSession data) in /home/runner/work/space-station-14/space-station-14/RobustToolbox/Robust.Server/GameStates/PvsSystem.Serialize.cs:line 59
   at Robust.Server.GameStates.PvsSystem.SerializeState(Int32 i) in /home/runner/work/space-station-14/space-station-14/RobustToolbox/Robust.Server/GameStates/PvsSystem.Serialize.cs:line 41
   at System.Threading.Tasks.Parallel.<>c__DisplayClass19_0`2.<ForWorker>b__1(RangeWorker& currentWorker, Int64 timeout, Boolean& replicationDelegateYieldedBeforeCompletion)
   at System.Threading.Tasks.TaskReplicator.Replica.Execute()
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread)
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading.PortableThreadPool.WorkerThread.WorkerThreadStart()
 Sawmill=resolve

The JukeboxComponent is just one of the many cases of this error happening, and we had over 24000 such errors in the last 24 hours, which is highly problematic since it drowns out other important error messages. For example we had admin logs failing to save for over 2 weeks and we did not notice. Additionally the huge amounts of errors being logged at once can cause major lag spikes.

Image

Technical Explanation:

The current convention is that an EntityUid should always be a reference to a non-deleted entity.

The problem occurrs when:

  • You have an autonetworked datafield of type EntityUid or EntityUid?
  • That datafield gets a value assigned
  • The referenced entity is deleted (for example a player gets gibbed or eaten by a singulo)
  • The component is dirtied
  • The autonetworking source generator will try to convert the EntityUid into a NetEntity by reading its MetaDataComponent, but fails as the entity no longer exists

Workaround

Add a marker component to the referenced entity the resets the reference to null when the marker component shuts down. This is problematic as it adds a giant amount of boilerplate to every single component containing an EntityUid datafield. This will be even more complicated in cases where multiple components can store a reference the same target. If we want to fix this this way we would have to add such a marker for hundreds of existing components in the content repo, that currently all cause errors. Also this solution is problematic regarding cheat clients, as such a marker component has to be networked as well and can be used to obtain secret information about the round, for example when marking the target player of a kill objective.

Possible solutions

WeakEntityReference

A few PRs were made in an attempt to support referencing deleted entities using a new wrapper struct:

  • https://github.com/space-wizards/RobustToolbox/pull/5577
  • The above PR was reverted because the implementation was not the correct approach https://github.com/space-wizards/RobustToolbox/pull/6112
  • Another attempt was made with https://github.com/space-wizards/RobustToolbox/pull/6114

The downside of adding WeakEntityReference would be that the content repo will need a major cleanup by converting hundreds of components to use it.

Treat deleted entities as null when networked

https://github.com/space-wizards/RobustToolbox/pull/6123 This was declined as well, but something in this direction where we simply adjust the source generator instead of adding a new wrapper would have the advantage that we don't need to fix the content repo, the errors should just go away. The downside might be that we may have to fix some edgecases with for example datafields of type List<EntityUid> or Dictionary<EntityUid, Object>. These could be adjusted to automatically delete the entries of deleted entities, but in some cases their count will be relevant for the client (for example: List of players you have killed, which would get lower again in case they get gibbed), which then needs fixing.

slarticodefast avatar Aug 18 '25 09:08 slarticodefast

I know this issue is mostly about how to fix the problem in general, not the specific components causing problems, but I had a look through grafana for components where this seems to be a problem. From the last 12 hours of logs theres:

  • StepTriggerComponent
  • DeviceListComponent
  • InputMoverComponent
  • JukeboxComponent
  • PortalTimeoutComponent
  • DoorComponent
  • GrapplingGunComponent
  • TargetedProjectileComponent
  • ThrownItemComponent
  • ProjectileComponent

I haven't looked into them in detail, but some of these are probably bugs that need to be fixed, where the fields on components should be getting updated as entities are deleted.

  • IIRC StepTriggerComponent should be getting updated via collision events, so there might be a bug somewhere there.
  • InputMoverComponent probably shouldn't be referencing deleted entities
  • I know DeviceListComponent is already meant to be removing deleted entities, so theres some bug there

ElectroJr avatar Oct 07 '25 02:10 ElectroJr