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

Try fix invalid PVS index bug #5422

Merged
merged 9 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,13 @@ END TEMPLATE-->

### Bugfixes

<<<<<<< HEAD
* Fixed equality checks for `MarkupNode` not properly handling attributes.
* Fixed `MarkupNode` not having a `GetHashCode()` implementation.
* Fixed a PVS error that could occur when trying to delete the first entity that gets created in a round.
=======
* Fixed the "to" and "take" toolshed commands not working as intended.
>>>>>>> f5c1d870f904ca1a5d67ae6db20c17e181a26df9

### Other

Expand Down
10 changes: 10 additions & 0 deletions Robust.Server/GameStates/PvsData.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
Expand Down Expand Up @@ -190,6 +191,15 @@ internal struct PvsMetadata
private byte Pad0;
public uint Marker;
#endif

[Conditional("DEBUG")]
public void Validate(MetaDataComponent comp)
{
DebugTools.AssertEqual(NetEntity, comp.NetEntity);
DebugTools.AssertEqual(VisMask, comp.VisibilityMask);
DebugTools.AssertEqual(LifeStage, comp.EntityLifeStage);
DebugTools.Assert(LastModifiedTick == comp.EntityLastModifiedTick || LastModifiedTick.Value == 0);
}
}

[StructLayout(LayoutKind.Sequential, Size = 16)]
Expand Down
9 changes: 3 additions & 6 deletions Robust.Server/GameStates/PvsSystem.DataStorage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,6 @@ private static void FreeSessionDataMemory(PvsSession session)

private void OnEntityAdded(Entity<MetaDataComponent> entity)
{
DebugTools.Assert(entity.Comp.PvsData.Index == default);

AssignEntityPointer(entity.Comp);
}

Expand All @@ -255,6 +253,7 @@ private void OnEntityAdded(Entity<MetaDataComponent> entity)
/// </summary>
private void AssignEntityPointer(MetaDataComponent meta)
{
DebugTools.Assert(meta.PvsData == PvsIndex.Invalid);
if (_dataFreeListHead == PvsIndex.Invalid)
{
ExpandEntityCapacity();
Expand All @@ -267,8 +266,6 @@ private void AssignEntityPointer(MetaDataComponent meta)
ref var freeLink = ref Unsafe.As<PvsMetadata, PvsMetadataFreeLink>(ref metadata);
_dataFreeListHead = freeLink.NextFree;

// TODO: re-introduce this assert.
// DebugTools.AssertEqual(((PvsMetadata*) ptr)->NetEntity, NetEntity.Invalid);
DebugTools.AssertNotEqual(meta.NetEntity, NetEntity.Invalid);

meta.PvsData = index;
Expand All @@ -287,9 +284,9 @@ private void AssignEntityPointer(MetaDataComponent meta)
private void OnEntityDeleted(Entity<MetaDataComponent> entity)
{
var ptr = entity.Comp.PvsData;
entity.Comp.PvsData = default;
entity.Comp.PvsData = PvsIndex.Invalid;

if (ptr == default)
if (ptr == PvsIndex.Invalid)
return;

_incomingReturns.Add(ptr);
Expand Down
2 changes: 1 addition & 1 deletion Robust.Server/GameStates/PvsSystem.Dirty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ private void OnEntityAdd(Entity<MetaDataComponent> e)

private void OnEntityDirty(Entity<MetaDataComponent> uid)
{
if (uid.Comp.PvsData != default)
if (uid.Comp.PvsData != PvsIndex.Invalid)
{
ref var meta = ref _metadataMemory.GetRef(uid.Comp.PvsData.Index);
meta.LastModifiedTick = uid.Comp.EntityLastModifiedTick;
Expand Down
2 changes: 1 addition & 1 deletion Robust.Server/GameStates/PvsSystem.Entity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ private void AssertNullspace(EntityUid uid)

internal void SyncMetadata(MetaDataComponent meta)
{
if (meta.PvsData == default)
if (meta.PvsData == PvsIndex.Invalid)
return;

ref var ptr = ref _metadataMemory.GetRef(meta.PvsData.Index);
Expand Down
3 changes: 3 additions & 0 deletions Robust.Server/GameStates/PvsSystem.Overrides.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Runtime.InteropServices;
using Robust.Shared.GameObjects;
using Robust.Shared.Timing;
using Robust.Shared.Utility;

namespace Robust.Server.GameStates;

Expand All @@ -25,6 +26,7 @@ private void AddAllOverrides(PvsSession session)
foreach (ref var ent in CollectionsMarshal.AsSpan(_cachedGlobalOverride))
{
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);
}
Expand All @@ -51,6 +53,7 @@ private void AddForcedEntities(PvsSession session)
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);
}
Expand Down
4 changes: 2 additions & 2 deletions Robust.Server/GameStates/PvsSystem.ToSendSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ private void AddPvsChunk(PvsChunk chunk, float distance, PvsSession session)
foreach (ref var ent in span)
{
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);
}
Expand All @@ -78,8 +79,7 @@ private void AddEntity(PvsSession session, ref PvsChunk.ChunkEntity ent, ref Pvs

if (meta.LifeStage >= EntityLifeStage.Terminating)
{
Log.Error($"Attempted to send deleted entity: {ToPrettyString(ent.Uid)}, lifestage is {meta.LifeStage}.\n{Environment.StackTrace}");
EntityManager.QueueDeleteEntity(ent.Uid);
Log.Error($"Attempted to send deleted entity: {ToPrettyString(ent.Uid)}, Meta lifestage: {ent.Meta.EntityLifeStage}, PVS lifestage: {meta.LifeStage}.\n{Environment.StackTrace}");
return;
}

Expand Down
5 changes: 4 additions & 1 deletion Robust.Shared/GameObjects/Components/MetaDataComponent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ private protected override void ClearTicks()
/// <summary>
/// Offset into internal PVS data.
/// </summary>
internal PvsIndex PvsData;
internal PvsIndex PvsData = PvsIndex.Invalid;
}

[Flags]
Expand Down Expand Up @@ -254,5 +254,8 @@ internal readonly record struct PvsIndex(int Index)
/// An invalid index. This is also used as a marker value in the free list.
/// </summary>
public static readonly PvsIndex Invalid = new PvsIndex(-1);
// TODO PVS
// Consider making 0 an invalid value.
// it prevents default structs from accidentally being used.
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ public IEnumerable<T> To<T>(
[CommandArgument] ValueRef<T> end
)
where T : INumber<T>
=> Enumerable.Range(int.CreateTruncating(start), int.CreateTruncating(end.Evaluate(ctx)! - start)).Select(T.CreateTruncating);
=> Enumerable.Range(int.CreateTruncating(start), 1 + int.CreateTruncating(end.Evaluate(ctx)! - start)).Select(T.CreateTruncating);
}
8 changes: 4 additions & 4 deletions Robust.Shared/Utility/ResizableMemoryRegion.cs
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,8 @@ public Span<TCast> GetSpan<TCast>() where TCast : unmanaged
public ref T GetRef(int index)
{
// If the memory region is disposed, CurrentSize is 0 and this check always fails.
if (index >= CurrentSize)
ThrowIndexOutOfRangeException();
if (index >= CurrentSize || index < 0)
ThrowIndexOutOfRangeException(CurrentSize, index);

return ref *(BaseAddress + index);
}
Expand Down Expand Up @@ -302,9 +302,9 @@ private static void ThrowNotInitialized()
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowIndexOutOfRangeException()
private static void ThrowIndexOutOfRangeException(int size, int index)
{
throw new IndexOutOfRangeException();
throw new IndexOutOfRangeException($"Index was outside the bounds of the memory region. Size: {size}, Index: {index}");
}

private void ReleaseUnmanagedResources()
Expand Down
Loading