refactor(spawners): subclass-owned entries, lifecycle hooks, per-entry Disabled flag (#2621)

## Summary

Moves spawner entry storage out of the abstract `BaseSpawner` into the concrete owner, so a spawner subclass can store its own entry type while every stock code path keeps working. Motivation: an out-of-tree spawner (ModernSpawner) needs `ModernSpawnerEntry : SpawnerEntry` with extra fields; today `BaseSpawner` owns `List<SpawnerEntry>` and a family of non-virtual members, and the serialization generator constructs list elements from the declared element type, so storage has to live in the class that declares the concrete list.

### What changed

- **`BaseSpawner` v13** no longer owns `_entries`. It reads entries through an abstract view and mutates them through an owner contract (`BaseSpawner.Entries.cs`):
  `Entries` (`IReadOnlyList<SpawnerEntry>`, `[IgnoreDupe]`), `EntrySpan` (`ReadOnlySpan<SpawnerEntry>` for hot loops), `CreateEntry`, `AddEntryCore`, `RemoveEntryCore`, `ClearEntriesCore`, `AdoptEntries`, `CloneEntry`, `TransferSpawned`, plus public `RemoveAllEntries()`, `CopyEntriesTo(target)` and protected `RebuildSpawned()`. Every loop inside `BaseSpawner` is an indexed `for` over `EntrySpan`.
- **`Spawner` v2** owns `[SerializedIgnoreDupe] List<SpawnerEntry> _entryList` (generated `EntryList`, protected). `ProximitySpawner`/`RegionSpawner` inherit it unchanged. The `Spawned` rebuild and timer re-arm moved from the base `[AfterDeserialization]` (which runs before derived fields are read) into `Spawner`'s.
- **Save migration**: `MigrateFrom(V12Content)` (and the v10/v11/legacy readers) hand the old list to the owner via `AdoptEntries`; `Spawner.MigrateFrom(V1Content)` restores its own fields and leaves the adopted list alone. Three v12/v1/v0 save blobs captured before the change are committed as fixtures and loaded by tests.
- **Lifecycle hooks** (`BaseSpawner.Hooks.cs`, all no-op by default): `OnStarted`, `OnStopped`, `OnBeforeSpawn(entry)` veto, `OnConfigureSpawned(entry, spawned)` before positioning, entry-aware `GetSpawnPosition(entry, spawned, map)`, `OnSpawned(entry, spawned)`, `OnSpawnedDeath(entry, spawned, killer)`. `BaseCreature.OnDeath` calls `NotifySpawnedDeath` before base death (which deletes the mobile and unlinks the spawner). `Start()` and the `NextSpawn` setter share one start core so `OnStarted` fires on both.
- **`SpawnerEntry` v2**: per-entry `Disabled` (XmlSpawner's entry "lock"), stored inverted so the common case writes nothing in binary or JSON; skipped by weighted selection, live spawns untouched; toggle button per row in `SpawnerGump`. `SetParent` is public and `Parent` is protected so an out-of-tree entry subclass can adopt and dirty-track.
- **DTO**: `SpawnerDto` loses `Entries`; each concrete record declares its own `entries` at the same JSON order, so `Distribution/Data/Spawns/**` is byte-identical. Import adopts the deserialized entry objects instead of recreating them through `AddEntry`, which is what preserves subtype fields (and `disabled`).

### Breaking changes and behaviour changes

- **API:** `BaseSpawner.Entries` is `IReadOnlyList<SpawnerEntry>` instead of `List<SpawnerEntry>`. The generated `AddToEntries`/`RemoveFromEntries`/`InsertIntoEntries`/`RemoveFromEntriesAt`/`ClearEntries` helpers on `BaseSpawner` are gone; use `AddEntry`/`RemoveEntry`/`RemoveAllEntries`/`CopyEntriesTo`. `RemoveAllEntries()` deletes the entries' live spawns as well as the entries (the old generated `ClearEntries()` only cleared the list), which is why it has a new name rather than the old one.
- `SpawnerControllerGump` "copy entries" now goes through `CopyEntriesTo`, which deletes the target's live spawns (previously it cleared the list and left the spawns orphaned) and is a no-op when source == target (previously that wiped the source).
- `RemoveEntry` with an entry the spawner does not own is now a no-op (previously it deleted that entry's spawns).
- `Respawn()` honours `Disabled` because it calls `Spawn()`; `Spawn(int index)`, `RemoveSpawn`, and `RemoveSpawns` ignore it.
- Copying entries between spawners no longer forces a 1-second first spawn; the target re-arms on its normal delay.
- Subclasses that own a different entry list than `Spawner`'s must call `RebuildSpawned()` from their own `[AfterDeserialization]` (`Spawner`'s call runs before their list is read) and, when converting adopted entries into their own type, carry live spawns across with `TransferSpawned`. The in-repo test subclass demonstrates both.

### Performance

Manual harness (`Benchmark_SpawnPath_Manual`, skipped by default): 100k calls, entries all full so `Spawn()` does selection only.

| Path | Before (4bad0cc9e) | After |
|---|---|---|
| `Spawn()` 1 entry | 48.7 ns | 53.2 ns (within run-to-run noise) |
| `Spawn()` 10 entries | 243.7 ns | 153.1 ns |
| `Spawn()` 50 entries | 1077.6 ns | 645.1 ns |
| `Remove()` 10 entries | 139.6 ns | 83.6 ns |

Hooks are no-ops for stock spawners; `OnMovement` is untouched.

### Tests

- `SpawnerEntryOwnershipTests` (add/remove/clear, start after stop, dupe, copy, self-copy, foreign-entry removal, Disabled binary/JSON)
- `SpawnerHookTests` (hook order for mobiles and items, veto, death notification, `NextSpawn` start, a subclass with its own `List<TestEntry>` round-tripping and duping)
- `SpawnerSaveMigrationTests` (v12 fixtures through `Spawner`, `ProximitySpawner`, `RegionSpawner`; new-format byte-identical round trip with a live spawn reference)
- `SpawnerDtoEntryTests` (compact JSON position of `entries`, `disabled` only when set, import adopts the deserialized objects)
- Existing DTO/JSON/spawn-data tests unchanged. UOContent.Tests 801 passed, Server.Tests 869 passed. Migration schemas regenerated (`BaseSpawner.v13`, `Spawner.v2`, `SpawnerEntry.v2`).

Coverage caveats, stated plainly: v10, v11 and the pre-codegen legacy reader could not be captured as fixtures by the current code; each changed only `_entries = …` → `AdoptEntries(…)` into the same sink the v12 fixtures exercise, and is covered by review. The captured v12 fixtures carry no live spawns, so re-linking live `ISpawnable` references is proven by the new-format round trip, not by a legacy blob. The `SpawnerGump` toggle layout could not be checked in a client; the delete button moved from x=38 to x=46 to make room.
This commit is contained in:
Kamron Batman 2026-09-11 17:41:08 -07:00 • committed by GitHub
parent 75f326bfdd
commit a52ce6ef70
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
29 changed files with 1489 additions and 139 deletions

View file

@ -40,7 +40,7 @@ public enum SpawnPositionMode : byte
Abandoned = 3
}
[SerializationGenerator(12, false)]
[SerializationGenerator(13, false)]
public abstract partial class BaseSpawner : Item, ISpawner
{
private static readonly ILogger logger = LogFactory.GetLogger(typeof(BaseSpawner));
@ -61,15 +61,11 @@ public abstract partial class BaseSpawner : Item, ISpawner
[SerializedCommandProperty(AccessLevel.Developer)]
private bool _returnOnDeactivate;
[SerializedIgnoreDupe]
[SerializableField(2, setter: "private")]
private List<SpawnerEntry> _entries;
private int _walkingRange = -1;
private bool ShouldSerializeWayPoint() => _wayPoint != null;
[SerializableField(4)]
[SerializableField(3)]
[SaveFlag(nameof(ShouldSerializeWayPoint))]
[SerializedCommandProperty(AccessLevel.Developer)]
private WayPoint _wayPoint;
@ -77,7 +73,7 @@ public abstract partial class BaseSpawner : Item, ISpawner
private bool ShouldSerializeGroup() => _group;
[InvalidateProperties]
[SerializableField(5)]
[SerializableField(4)]
[SaveFlag(nameof(ShouldSerializeGroup))]
[SerializedCommandProperty(AccessLevel.Developer)]
private bool _group;
@ -87,7 +83,7 @@ public abstract partial class BaseSpawner : Item, ISpawner
private TimeSpan MinDelayDefault() => DefaultMinDelay;
[InvalidateProperties]
[SerializableField(6)]
[SerializableField(5)]
[SaveFlag(nameof(ShouldSerializeMinDelay), nameof(MinDelayDefault))]
[SerializedCommandProperty(AccessLevel.Developer)]
private TimeSpan _minDelay;
@ -97,7 +93,7 @@ public abstract partial class BaseSpawner : Item, ISpawner
private TimeSpan MaxDelayDefault() => DefaultMaxDelay;
[InvalidateProperties]
[SerializableField(7)]
[SerializableField(6)]
[SaveFlag(nameof(ShouldSerializeMaxDelay), nameof(MaxDelayDefault))]
[SerializedCommandProperty(AccessLevel.Developer)]
private TimeSpan _maxDelay;
@ -105,7 +101,7 @@ public abstract partial class BaseSpawner : Item, ISpawner
private bool ShouldSerializeTeam() => _team != 0;
[InvalidateProperties]
[SerializableField(9)]
[SerializableField(8)]
[SaveFlag(nameof(ShouldSerializeTeam))]
[SerializedCommandProperty(AccessLevel.Developer)]
private int _team;
@ -126,14 +122,14 @@ public abstract partial class BaseSpawner : Item, ISpawner
private bool ShouldSerializeSpawnLocationIsHome() => _spawnLocationIsHome;
[InvalidateProperties]
[SerializableField(11)]
[SerializableField(10)]
[SaveFlag(nameof(ShouldSerializeSpawnLocationIsHome))]
[SerializedCommandProperty(AccessLevel.Developer)]
private bool _spawnLocationIsHome;
private bool ShouldSerializeEnd() => _end != default;
[SerializableField(12)]
[SerializableField(11)]
[SaveFlag(nameof(ShouldSerializeEnd))]
[SerializedCommandProperty(AccessLevel.Developer)]
private DateTime _end;
@ -144,7 +140,7 @@ public abstract partial class BaseSpawner : Item, ISpawner
private bool ShouldSerializeSpawnPositionMode() =>
_spawnPositionMode is not SpawnPositionMode.Automatic and not SpawnPositionMode.Abandoned;
[SerializableField(13)]
[SerializableField(12)]
[SaveFlag(nameof(ShouldSerializeSpawnPositionMode))]
[SerializedCommandProperty(AccessLevel.Developer)]
private SpawnPositionMode _spawnPositionMode;
@ -158,7 +154,7 @@ public abstract partial class BaseSpawner : Item, ISpawner
private int MaxSpawnAttemptsDefault() => DefaultMaxSpawnAttempts;
[SerializableField(14)]
[SerializableField(13)]
[SaveFlag(nameof(ShouldSerializeMaxSpawnAttempts), nameof(MaxSpawnAttemptsDefault))]
[SerializedCommandProperty(AccessLevel.Developer)]
private int _maxSpawnAttempts;
@ -300,7 +296,7 @@ public abstract partial class BaseSpawner : Item, ISpawner
public Dictionary<ISpawnable, SpawnerEntry> Spawned { get; private set; }
[CommandProperty(AccessLevel.Developer)]
[SerializableProperty(3, nameof(_walkingRange))]
[SerializableProperty(2, nameof(_walkingRange))]
public int WalkingRange
{
get => _walkingRange > 0 ? _walkingRange : HomeRange;
@ -312,7 +308,7 @@ public abstract partial class BaseSpawner : Item, ISpawner
}
}
[SerializableField(8, fieldChanged: nameof(OnCountChanged))]
[SerializableField(7, fieldChanged: nameof(OnCountChanged))]
[SerializedCommandProperty(AccessLevel.Developer)]
[InvalidateProperties]
private int _count;
@ -329,7 +325,7 @@ public abstract partial class BaseSpawner : Item, ISpawner
}
}
[SerializableProperty(10)]
[SerializableProperty(9)]
[CommandProperty(AccessLevel.Developer)]
public bool Running
{
@ -356,10 +352,10 @@ public abstract partial class BaseSpawner : Item, ISpawner
get => _running && _timer?.Running == true ? End - Core.Now : TimeSpan.Zero;
set
{
if (!_running && Entries.Count > 0)
if (BeginStart())
{
_running = true;
DoTimer(value);
OnStarted();
}
}
}
@ -641,20 +637,7 @@ public abstract partial class BaseSpawner : Item, ISpawner
{
newSpawner._guid = Guid.NewGuid();
newSpawner.Spawned = new Dictionary<ISpawnable, SpawnerEntry>();
newSpawner.Entries = [];
for (var i = 0; i < Entries.Count; i++)
{
var entry = Entries[i];
newSpawner.AddEntry(
entry.SpawnedName,
entry.SpawnedProbability,
entry.SpawnedMaxCount,
false,
entry.Properties,
entry.Parameters
);
}
CopyEntriesTo(newSpawner);
}
}
@ -698,25 +681,6 @@ public abstract partial class BaseSpawner : Item, ISpawner
}
}
public SpawnerEntry AddEntry(
string creaturename,
int probability = 100,
int amount = 1,
bool dotimer = true,
string properties = null,
string parameters = null
)
{
var entry = new SpawnerEntry(this, creaturename, probability, amount, properties, parameters);
AddToEntries(entry);
if (dotimer)
{
DoTimer(TimeSpan.FromSeconds(1));
}
return entry;
}
public void InitSpawn(int amount, TimeSpan minDelay, TimeSpan maxDelay, int team = 0, Rectangle3D spawnBounds = default)
{
Visible = false;
@ -737,7 +701,7 @@ public abstract partial class BaseSpawner : Item, ISpawner
HomeRange = 4;
}
Entries = [];
ClearEntriesCore();
Spawned = new Dictionary<ISpawnable, SpawnerEntry>();
DoTimer(TimeSpan.FromSeconds(1));
@ -804,26 +768,42 @@ public abstract partial class BaseSpawner : Item, ISpawner
public void Start()
{
if (!_running && Entries.Count > 0)
if (BeginStart())
{
_running = true;
DoTimer();
OnStarted();
}
}
/// <summary>Guards and flips <see cref="Running"/>. Callers arm the timer and fire <see cref="OnStarted"/>.</summary>
private bool BeginStart()
{
if (_running || Entries.Count == 0)
{
return false;
}
_running = true;
return true;
}
public void Stop()
{
var wasRunning = _running;
_timer?.Stop();
_running = false;
if (wasRunning)
{
OnStopped();
}
}
public void Defrag()
{
Entries ??= [];
for (var i = 0; i < Entries.Count; ++i)
var entries = EntrySpan;
for (var i = 0; i < entries.Length; i++)
{
Entries[i].Defrag(this);
entries[i].Defrag(this);
}
}
@ -867,16 +847,18 @@ public abstract partial class BaseSpawner : Item, ISpawner
{
Defrag();
if (Entries.Count <= 0 || IsFull)
var entries = EntrySpan;
if (entries.Length <= 0 || IsFull)
{
return;
}
var probsum = 0;
foreach (var spawnerEntry in Entries)
for (var i = 0; i < entries.Length; i++)
{
if (!spawnerEntry.IsFull)
var spawnerEntry = entries[i];
if (!spawnerEntry.IsFull && !spawnerEntry.Disabled)
{
probsum += spawnerEntry.SpawnedProbability;
}
@ -889,10 +871,10 @@ public abstract partial class BaseSpawner : Item, ISpawner
var rand = Utility.RandomMinMax(1, probsum);
for (var i = 0; i < Entries.Count; i++)
for (var i = 0; i < entries.Length; i++)
{
var entry = Entries[i];
if (entry.IsFull)
var entry = entries[i];
if (entry.IsFull || entry.Disabled)
{
continue;
}
@ -1010,6 +992,11 @@ public abstract partial class BaseSpawner : Item, ISpawner
return false;
}
if (!OnBeforeSpawn(entry))
{
return false;
}
try
{
IEntity entity = null;
@ -1095,6 +1082,11 @@ public abstract partial class BaseSpawner : Item, ISpawner
}
}
if (entity is ISpawnable configured)
{
OnConfigureSpawned(entry, configured);
}
if (entity is Mobile m)
{
Spawned.Add(m, entry);
@ -1102,7 +1094,7 @@ public abstract partial class BaseSpawner : Item, ISpawner
// var spawnLocation = m is BaseVendor ? Location : GetSpawnPosition(m, map);
var spawnLocation = GetSpawnPosition(m, map);
var spawnLocation = GetSpawnPosition(entry, m, map);
m.OnBeforeSpawn(spawnLocation, map);
m.MoveToWorld(spawnLocation, map);
@ -1133,13 +1125,14 @@ public abstract partial class BaseSpawner : Item, ISpawner
m.Spawner = this;
m.OnAfterSpawn();
OnSpawned(entry, m);
}
else if (entity is Item item)
{
Spawned.Add(item, entry);
entry.AddToSpawned(item);
var loc = GetSpawnPosition(item, map);
var loc = GetSpawnPosition(entry, item, map);
item.OnBeforeSpawn(loc, map);
@ -1147,6 +1140,7 @@ public abstract partial class BaseSpawner : Item, ISpawner
item.Spawner = this;
item.OnAfterSpawn();
OnSpawned(entry, item);
}
else
{
@ -1221,27 +1215,6 @@ public abstract partial class BaseSpawner : Item, ISpawner
return entry.Spawned.Count;
}
public void RemoveEntry(SpawnerEntry entry)
{
Defrag();
for (var i = entry.Spawned.Count - 1; i >= 0; i--)
{
var e = entry.Spawned[i];
entry.Spawned.RemoveAt(i);
e?.Delete();
}
Entries.Remove(entry);
if (_running && !IsFull && _timer?.Running != true)
{
DoTimer();
}
InvalidateProperties();
}
public void RemoveSpawn(int index) // Entry
{
if (index >= 0 && index < Entries.Count)
@ -1269,9 +1242,10 @@ public abstract partial class BaseSpawner : Item, ISpawner
{
Defrag();
for (var i = 0; i < Entries.Count; i++)
var entries = EntrySpan;
for (var i = 0; i < entries.Length; i++)
{
var entry = Entries[i];
var entry = entries[i];
for (var j = entry.Spawned.Count - 1; j >= 0; j--)
{
@ -1327,18 +1301,6 @@ public abstract partial class BaseSpawner : Item, ISpawner
256
);
}
Spawned = new Dictionary<ISpawnable, SpawnerEntry>();
foreach (var entry in Entries)
{
foreach (var spawned in entry.Spawned)
{
Spawned.Add(spawned, entry);
}
}
DoTimer(_end - Core.Now);
}
private class InternalTimer : Timer