From 00e274ea388bc4ce1e59d9dc197a1886fe19bb34 Mon Sep 17 00:00:00 2001 From: Leon Friedrich <60421075+ElectroJr@users.noreply.github.com> Date: Mon, 16 Oct 2023 16:54:10 +1100 Subject: [PATCH] Add new entity spawn test & fix misc bugs (#19953) --- Content.IntegrationTests/Tests/EntityTest.cs | 98 ++++++++++++++++++- .../GameTicking/Rules/DeathMatchRuleSystem.cs | 3 +- Content.Server/Gatherable/GatherableSystem.cs | 2 +- .../IdentityManagement/IdentitySystem.cs | 5 +- .../EntitySystems/KitchenSpikeSystem.cs | 3 +- .../Events/StationEventSystem.cs | 7 +- .../EntitySystems/StorageSystem.Fill.cs | 9 +- .../Storage/EntitySystems/StorageSystem.cs | 3 +- .../EntityList/EntityLootTablePrototype.cs | 2 +- .../Components/KitchenSpikeComponent.cs | 2 +- .../Station/SharedStationSpawningSystem.cs | 2 +- Content.Shared/Storage/EntitySpawnEntry.cs | 10 +- .../SharedEntityStorageSystem.cs | 6 +- .../Catalog/Fills/Crates/salvage.yml | 33 ++++--- 14 files changed, 149 insertions(+), 36 deletions(-) diff --git a/Content.IntegrationTests/Tests/EntityTest.cs b/Content.IntegrationTests/Tests/EntityTest.cs index 453796b916..042db1deb3 100644 --- a/Content.IntegrationTests/Tests/EntityTest.cs +++ b/Content.IntegrationTests/Tests/EntityTest.cs @@ -1,6 +1,8 @@ using System.Collections.Generic; using System.Linq; +using Content.Server.Humanoid.Components; using Content.Shared.Coordinates; +using Content.Shared.Prototypes; using Robust.Shared; using Robust.Shared.Configuration; using Robust.Shared.GameObjects; @@ -183,7 +185,7 @@ namespace Content.IntegrationTests.Tests var query = entityMan.AllEntityQueryEnumerator(); while (query.MoveNext(out var uid, out var meta)) yield return (uid, meta); - }; + } var entityMetas = Query(sEntMan).ToList(); foreach (var (uid, meta) in entityMetas) @@ -198,6 +200,100 @@ namespace Content.IntegrationTests.Tests await pair.CleanReturnAsync(); } + /// + /// This test checks that spawning and deleting an entity doesn't somehow create other unrelated entities. + /// + /// + /// Unless an entity is intentionally designed to spawn other entities (e.g., mob spawners), they should + /// generally not spawn unrelated / detached entities. Any entities that do get spawned should be parented to + /// the spawned entity (e.g., in a container). If an entity needs to spawn an entity somewhere in null-space, + /// it should delete that entity when it is no longer required. This test mainly exists to prevent "entity leak" + /// bugs, where spawning some entity starts spawning unrelated entities in null space. + /// + [Test] + public async Task SpawnAndDeleteEntityCountTest() + { + var settings = new PoolSettings { Connected = true, Dirty = true }; + await using var pair = await PoolManager.GetServerClient(settings); + var server = pair.Server; + var client = pair.Client; + + var excluded = new[] + { + "MapGrid", + "StationEvent", + "TimedDespawn", + + // Spawner entities + "DragonRift", + "RandomHumanoidSpawner", + "RandomSpawner", + "ConditionalSpawner", + "GhostRoleMobSpawner", + "NukeOperativeSpawner", + "TimedSpawner", + }; + + Assert.That(server.CfgMan.GetCVar(CVars.NetPVS), Is.False); + + var protoIds = server.ProtoMan + .EnumeratePrototypes() + .Where(p => !p.Abstract) + .Where(p => !pair.IsTestPrototype(p)) + .Where(p => !excluded.Any(p.Components.ContainsKey)) + .Select(p => p.ID) + .ToList(); + + MapCoordinates coords = default; + await server.WaitPost(() => + { + var map = server.MapMan.CreateMap(); + coords = new MapCoordinates(default, map); + }); + + await pair.RunTicksSync(3); + + List badPrototypes = new(); + foreach (var protoId in protoIds) + { + // TODO fix ninja + // Currently ninja fails to equip their own loadout. + if (protoId == "MobHumanSpaceNinja") + continue; + + var count = server.EntMan.EntityCount; + var clientCount = client.EntMan.EntityCount; + EntityUid uid = default; + await server.WaitPost(() => uid = server.EntMan.SpawnEntity(protoId, coords)); + await pair.RunTicksSync(3); + + // If the entity deleted itself, check that it didn't spawn other entities + if (!server.EntMan.EntityExists(uid)) + { + if (server.EntMan.EntityCount != count || client.EntMan.EntityCount != clientCount) + badPrototypes.Add(protoId); + continue; + } + + // Check that the number of entities has increased. + if (server.EntMan.EntityCount <= count || client.EntMan.EntityCount <= clientCount) + { + badPrototypes.Add(protoId); + continue; + } + + await server.WaitPost(() => server.EntMan.DeleteEntity(uid)); + await pair.RunTicksSync(3); + + // Check that the number of entities has gone back to the original value. + if (server.EntMan.EntityCount != count || client.EntMan.EntityCount != clientCount) + badPrototypes.Add(protoId); + } + + Assert.That(badPrototypes, Is.Empty); + await pair.CleanReturnAsync(); + } + [Test] public async Task AllComponentsOneToOneDeleteTest() { diff --git a/Content.Server/GameTicking/Rules/DeathMatchRuleSystem.cs b/Content.Server/GameTicking/Rules/DeathMatchRuleSystem.cs index 042455e75d..82ac755592 100644 --- a/Content.Server/GameTicking/Rules/DeathMatchRuleSystem.cs +++ b/Content.Server/GameTicking/Rules/DeathMatchRuleSystem.cs @@ -1,3 +1,4 @@ +using System.Linq; using Content.Server.Administration.Commands; using Content.Server.GameTicking.Rules.Components; using Content.Server.KillTracking; @@ -95,7 +96,7 @@ public sealed class DeathMatchRuleSystem : GameRuleSystem().ToList(); EntityManager.SpawnEntities(Transform(ev.Entity).MapPosition, spawns); } } diff --git a/Content.Server/Gatherable/GatherableSystem.cs b/Content.Server/Gatherable/GatherableSystem.cs index 5eaff020d5..4f7d19b0a8 100644 --- a/Content.Server/Gatherable/GatherableSystem.cs +++ b/Content.Server/Gatherable/GatherableSystem.cs @@ -70,7 +70,7 @@ public sealed partial class GatherableSystem : EntitySystem continue; } var getLoot = _prototypeManager.Index(table); - var spawnLoot = getLoot.GetSpawns(); + var spawnLoot = getLoot.GetSpawns(_random); var spawnPos = pos.Offset(_random.NextVector2(0.3f)); Spawn(spawnLoot[0], spawnPos); } diff --git a/Content.Server/IdentityManagement/IdentitySystem.cs b/Content.Server/IdentityManagement/IdentitySystem.cs index 6be3a96433..3d4be31435 100644 --- a/Content.Server/IdentityManagement/IdentitySystem.cs +++ b/Content.Server/IdentityManagement/IdentitySystem.cs @@ -35,6 +35,7 @@ public class IdentitySystem : SharedIdentitySystem SubscribeLocalEvent((uid, _, _) => QueueIdentityUpdate(uid)); SubscribeLocalEvent((uid, _, _) => QueueIdentityUpdate(uid)); SubscribeLocalEvent((uid, _, _) => QueueIdentityUpdate(uid)); + SubscribeLocalEvent(OnMapInit); } public override void Update(float frameTime) @@ -53,10 +54,8 @@ public class IdentitySystem : SharedIdentitySystem } // This is where the magic happens - protected override void OnComponentInit(EntityUid uid, IdentityComponent component, ComponentInit args) + private void OnMapInit(EntityUid uid, IdentityComponent component, MapInitEvent args) { - base.OnComponentInit(uid, component, args); - var ident = Spawn(null, Transform(uid).Coordinates); QueueIdentityUpdate(uid); diff --git a/Content.Server/Kitchen/EntitySystems/KitchenSpikeSystem.cs b/Content.Server/Kitchen/EntitySystems/KitchenSpikeSystem.cs index 04a224a3a7..6e563ff45f 100644 --- a/Content.Server/Kitchen/EntitySystems/KitchenSpikeSystem.cs +++ b/Content.Server/Kitchen/EntitySystems/KitchenSpikeSystem.cs @@ -110,7 +110,8 @@ namespace Content.Server.Kitchen.EntitySystems if (args.Handled) return; - if (component.PrototypesToSpawn?.Count > 0) { + if (component.PrototypesToSpawn?.Count > 0) + { _popupSystem.PopupEntity(Loc.GetString("comp-kitchen-spike-knife-needed"), uid, args.User); args.Handled = true; } diff --git a/Content.Server/StationEvents/Events/StationEventSystem.cs b/Content.Server/StationEvents/Events/StationEventSystem.cs index c647c96506..97b96ff7b0 100644 --- a/Content.Server/StationEvents/Events/StationEventSystem.cs +++ b/Content.Server/StationEvents/Events/StationEventSystem.cs @@ -136,12 +136,7 @@ public abstract partial class StationEventSystem : GameRuleSystem where T protected bool TryGetRandomStation([NotNullWhen(true)] out EntityUid? station, Func? filter = null) { - var stations = new ValueList(); - - if (filter == null) - { - stations.EnsureCapacity(Count()); - } + var stations = new ValueList(Count()); filter ??= _ => true; var query = AllEntityQuery(); diff --git a/Content.Server/Storage/EntitySystems/StorageSystem.Fill.cs b/Content.Server/Storage/EntitySystems/StorageSystem.Fill.cs index e05a8f49ff..902ab471f1 100644 --- a/Content.Server/Storage/EntitySystems/StorageSystem.Fill.cs +++ b/Content.Server/Storage/EntitySystems/StorageSystem.Fill.cs @@ -1,6 +1,10 @@ +using Content.Server.Spawners.Components; using Content.Server.Storage.Components; +using Content.Shared.Prototypes; using Content.Shared.Storage; using Content.Shared.Storage.Components; +using Robust.Shared.Prototypes; +using Robust.Shared.Utility; namespace Content.Server.Storage.EntitySystems; @@ -25,10 +29,13 @@ public sealed partial class StorageSystem var spawnItems = EntitySpawnCollection.GetSpawns(component.Contents, Random); foreach (var item in spawnItems) { + // No, you are not allowed to fill a container with entity spawners. + DebugTools.Assert(!_prototype.Index(item) + .HasComponent(typeof(RandomSpawnerComponent))); var ent = EntityManager.SpawnEntity(item, coordinates); // handle depending on storage component, again this should be unified after ECS - if (entityStorageComp != null && EntityStorage.Insert(ent, uid)) + if (entityStorageComp != null && EntityStorage.Insert(ent, uid, entityStorageComp)) continue; if (storageComp != null && Insert(uid, ent, out _, storageComp: storageComp, playSound: false)) diff --git a/Content.Server/Storage/EntitySystems/StorageSystem.cs b/Content.Server/Storage/EntitySystems/StorageSystem.cs index b2d940ffe1..3430449957 100644 --- a/Content.Server/Storage/EntitySystems/StorageSystem.cs +++ b/Content.Server/Storage/EntitySystems/StorageSystem.cs @@ -12,7 +12,7 @@ using Robust.Server.GameObjects; using Robust.Server.Player; using Robust.Shared.Map; using Robust.Shared.Player; -using Robust.Shared.Players; +using Robust.Shared.Prototypes; using Robust.Shared.Utility; namespace Content.Server.Storage.EntitySystems; @@ -20,6 +20,7 @@ namespace Content.Server.Storage.EntitySystems; public sealed partial class StorageSystem : SharedStorageSystem { [Dependency] private readonly IAdminManager _admin = default!; + [Dependency] private readonly IPrototypeManager _prototype = default!; [Dependency] private readonly UserInterfaceSystem _uiSystem = default!; public override void Initialize() diff --git a/Content.Shared/EntityList/EntityLootTablePrototype.cs b/Content.Shared/EntityList/EntityLootTablePrototype.cs index 06b33eb7f6..da535d570c 100644 --- a/Content.Shared/EntityList/EntityLootTablePrototype.cs +++ b/Content.Shared/EntityList/EntityLootTablePrototype.cs @@ -15,7 +15,7 @@ public sealed class EntityLootTablePrototype : IPrototype public ImmutableList Entries = ImmutableList.Empty; /// - public List GetSpawns(IRobustRandom? random = null) + public List GetSpawns(IRobustRandom random) { return EntitySpawnCollection.GetSpawns(Entries, random); } diff --git a/Content.Shared/Kitchen/Components/KitchenSpikeComponent.cs b/Content.Shared/Kitchen/Components/KitchenSpikeComponent.cs index 8b43ab1a85..3057a75a4c 100644 --- a/Content.Shared/Kitchen/Components/KitchenSpikeComponent.cs +++ b/Content.Shared/Kitchen/Components/KitchenSpikeComponent.cs @@ -15,7 +15,7 @@ public sealed partial class KitchenSpikeComponent : Component [DataField("sound")] public SoundSpecifier SpikeSound = new SoundPathSpecifier("/Audio/Effects/Fluids/splat.ogg"); - public List? PrototypesToSpawn; + public List? PrototypesToSpawn; // TODO: Spiking alive mobs? (Replace with uid) (deal damage to their limbs on spiking, kill on first butcher attempt?) public string MeatSource1p = "?"; diff --git a/Content.Shared/Station/SharedStationSpawningSystem.cs b/Content.Shared/Station/SharedStationSpawningSystem.cs index d392cf7bed..8cdcff883b 100644 --- a/Content.Shared/Station/SharedStationSpawningSystem.cs +++ b/Content.Shared/Station/SharedStationSpawningSystem.cs @@ -27,7 +27,7 @@ public abstract class SharedStationSpawningSystem : EntitySystem if (!string.IsNullOrEmpty(equipmentStr)) { var equipmentEntity = EntityManager.SpawnEntity(equipmentStr, EntityManager.GetComponent(entity).Coordinates); - InventorySystem.TryEquip(entity, equipmentEntity, slot.Name, true); + InventorySystem.TryEquip(entity, equipmentEntity, slot.Name, true, force:true); } } } diff --git a/Content.Shared/Storage/EntitySpawnEntry.cs b/Content.Shared/Storage/EntitySpawnEntry.cs index a39c2015a9..96fb9f9f40 100644 --- a/Content.Shared/Storage/EntitySpawnEntry.cs +++ b/Content.Shared/Storage/EntitySpawnEntry.cs @@ -78,12 +78,12 @@ public static class EntitySpawnCollection /// The entity spawn entries. /// Resolve param. /// A list of entity prototypes that should be spawned. - public static List GetSpawns(IEnumerable entries, + public static List GetSpawns(IEnumerable entries, IRobustRandom? random = null) { IoCManager.Resolve(ref random); - var spawned = new List(); + var spawned = new List(); var ungrouped = CollectOrGroups(entries, out var orGroupedSpawns); foreach (var entry in ungrouped) @@ -93,6 +93,9 @@ public static class EntitySpawnCollection if (entry.SpawnProbability != 1f && !random.Prob(entry.SpawnProbability)) continue; + if (entry.PrototypeId == null) + continue; + var amount = (int) entry.GetAmount(random); for (var i = 0; i < amount; i++) @@ -116,6 +119,9 @@ public static class EntitySpawnCollection if (diceRoll > cumulative) continue; + if (entry.PrototypeId == null) + break; + // Dice roll succeeded, add item and break loop var amount = (int) entry.GetAmount(random); diff --git a/Content.Shared/Storage/EntitySystems/SharedEntityStorageSystem.cs b/Content.Shared/Storage/EntitySystems/SharedEntityStorageSystem.cs index 7553fb6c9c..2d85f79e03 100644 --- a/Content.Shared/Storage/EntitySystems/SharedEntityStorageSystem.cs +++ b/Content.Shared/Storage/EntitySystems/SharedEntityStorageSystem.cs @@ -22,6 +22,7 @@ using Robust.Shared.Network; using Robust.Shared.Physics; using Robust.Shared.Physics.Components; using Robust.Shared.Physics.Systems; +using Robust.Shared.Prototypes; using Robust.Shared.Timing; using Robust.Shared.Utility; @@ -260,9 +261,12 @@ public abstract class SharedEntityStorageSystem : EntitySystem } _joints.RecursiveClearJoints(toInsert); + if (!component.Contents.Insert(toInsert, EntityManager)) + return false; + var inside = EnsureComp(toInsert); inside.Storage = container; - return component.Contents.Insert(toInsert, EntityManager); + return true; } public bool Remove(EntityUid toRemove, EntityUid container, SharedEntityStorageComponent? component = null, TransformComponent? xform = null) diff --git a/Resources/Prototypes/Catalog/Fills/Crates/salvage.yml b/Resources/Prototypes/Catalog/Fills/Crates/salvage.yml index cd5daf6f72..feae7a8942 100644 --- a/Resources/Prototypes/Catalog/Fills/Crates/salvage.yml +++ b/Resources/Prototypes/Catalog/Fills/Crates/salvage.yml @@ -92,30 +92,33 @@ id: CratePartsT3 name: tier 3 parts crate description: Contains 5 random tier 3 parts for upgrading machines. - components: - - type: StorageFill - contents: - - id: SalvagePartsT3Spawner - amount: 5 + # TODO add contents. + #components: + #- type: StorageFill + # contents: + # - id: SalvagePartsT3Spawner + # amount: 5 - type: entity parent: CrateGenericSteel id: CratePartsT3T4 name: tier 3/4 parts crate description: Contains 5 random tier 3 or 4 parts for upgrading machines. - components: - - type: StorageFill - contents: - - id: SalvagePartsT3T4Spawner - amount: 5 + # TODO add contents. + #components: + # type: StorageFill + # contents: + # - id: SalvagePartsT3T4Spawner + # amount: 5 - type: entity parent: CrateGenericSteel id: CratePartsT4 name: tier 4 parts crate description: Contains 5 random tier 4 parts for upgrading machines. - components: - - type: StorageFill - contents: - - id: SalvagePartsT4Spawner - amount: 5 + # TODO add contents. + #components: + #- type: StorageFill + # contents: + # - id: SalvagePartsT4Spawner + # amount: 5