refactor: fold hand-written serializable property setters into field hooks (#2587)

## Summary

Folds **113** hand-written `[SerializableProperty]` members into plain `[SerializableField]` declarations using the v4 setter hooks — value coercion/vetoes via `allowFieldChange`, post-change side effects via `fieldChanged` (whose `oldValue` parameter covers the old-house/old-sender unsubscribe patterns). Net **-450 lines** of setter boilerplate.

```cs
// before
[SerializableProperty(1)]
[CommandProperty(AccessLevel.GameMaster)]
public int Charges
{
    get => _charges;
    set
    {
        _charges = Math.Clamp(value, 0, MaxCharges);
        InvalidateProperties();
        this.MarkDirty();
    }
}

// after
[SerializableField(1, allowFieldChange: nameof(AllowChargesChange))]
[SerializedCommandProperty(AccessLevel.GameMaster)]
[InvalidateProperties]
private int _charges;

private bool AllowChargesChange(ref int value)
{
    value = Math.Clamp(value, 0, MaxCharges);
    return true;
}
```

## How sites were selected

A classifier parsed all 204 `[SerializableProperty]` sites and converted only those matching strict shapes: getter is exactly `get => _field;`, the assignment comes first (after at most an equality guard), and relocated side effects contain no `return`, no `value` mutation, and no field re-assignment. Everything else was left alone deliberately:

- **~34 custom getters** (fallback defaults like `_x == -1 ? Default : _x`, self-healing refs) — no setter hook can express these.
- **~35 pre-assignment logic** (durability Unscale/Scale sandwiches, old-state captures like PotionKeg's pile weight).
- **virtual/override members, name-mismatched backing fields (`m_`), exotic semantics** (guards' `Focus` does work on *equal* assignment; `ChampionSpawn.Active` never assigns its field).

Five sites the classifier refused were converted by hand where the hooks fit cleanly: `ReceiverCrystal.Sender`, `PlayerVendor.House`, `PlayerBarkeeper.House` (old-value unsubscribe via `oldValue`), `BaseSuit.AccessLevel` (its existing virtual `OnAccessLevelChanged` already had the exact callback shape), and `DyeTub.DyedHue` (a true veto: `AllowDyedHueChange(ref int value) => _redyable`).

## Verification

- Build: **0 errors, 0 warnings**.
- **Schema regeneration produces zero Migrations changes** — the conversion is wire- and schema-neutral by construction (same orders, types, and property names), and CI's schema diff check enforces it.
- **835 + 708 tests green.**

## Behavioral notes (all strict improvements, called out for review)

- Generated setters skip everything when the incoming value equals the current one; a few converted setters previously re-ran side effects on equal assignment (redundant `Update()`-style refreshes).
- Generated setters always `MarkDirty()` on change; several converted setters never did (e.g. `DyeTub.DyedHue`, `MorphItem` ranges) — their changes only persisted if something else dirtied the entity. Those latent persistence bugs are fixed by construction.
This commit is contained in:
Kamron Batman 2026-08-22 18:20:39 -07:00 • committed by GitHub
parent 73f9688083
commit b042edcf0b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
73 changed files with 893 additions and 1340 deletions

View file

@ -21,22 +21,14 @@ public partial class Board : Item, ICommodity
Hue = CraftResources.GetHue(resource);
}
[SerializableProperty(0)]
[CommandProperty(AccessLevel.GameMaster)]
public CraftResource Resource
{
get => _resource;
set
{
if (_resource != value)
{
_resource = value;
Hue = CraftResources.GetHue(value);
[SerializableField(0, fieldChanged: nameof(OnResourceChanged))]
[SerializedCommandProperty(AccessLevel.GameMaster)]
[InvalidateProperties]
private CraftResource _resource;
InvalidateProperties();
this.MarkDirty();
}
}
private void OnResourceChanged(CraftResource oldValue, CraftResource newValue)
{
Hue = CraftResources.GetHue(newValue);
}
int ICommodity.DescriptionNumber

View file

@ -25,16 +25,14 @@ public partial class MessageInABottle : Item
public override int LabelNumber => 1041080; // a message in a bottle
[SerializableProperty(0)]
[CommandProperty(AccessLevel.GameMaster)]
public int Level
[SerializableField(0, allowFieldChange: nameof(AllowLevelChange))]
[SerializedCommandProperty(AccessLevel.GameMaster)]
private int _level;
private bool AllowLevelChange(ref int value)
{
get => _level;
set
{
_level = Math.Max(1, Math.Min(value, 4));
this.MarkDirty();
}
value = Math.Max(1, Math.Min(value, 4));
return true;
}
public static int GetRandomLevel()

View file

@ -103,18 +103,20 @@ public partial class SOS : Item
[CommandProperty(AccessLevel.GameMaster)]
public bool IsAncient => _level >= 4;
[SerializableProperty(0)]
[CommandProperty(AccessLevel.GameMaster)]
public int Level
[SerializableField(0, fieldChanged: nameof(OnLevelChanged), allowFieldChange: nameof(AllowLevelChange))]
[SerializedCommandProperty(AccessLevel.GameMaster)]
[InvalidateProperties]
private int _level;
private bool AllowLevelChange(ref int value)
{
get => _level;
set
{
_level = Math.Max(1, Math.Min(value, 4));
UpdateHue();
InvalidateProperties();
this.MarkDirty();
}
value = Math.Max(1, Math.Min(value, 4));
return true;
}
private void OnLevelChanged(int oldValue, int newValue)
{
UpdateHue();
}
public void UpdateHue() => Hue = IsAncient ? 0x481 : 0;

View file

@ -28,22 +28,14 @@ public partial class Log : Item, ICommodity, IAxe
public override double DefaultWeight => 2.0;
[SerializableProperty(0)]
[CommandProperty(AccessLevel.GameMaster)]
public CraftResource Resource
{
get => _resource;
set
{
if (_resource != value)
{
_resource = value;
Hue = CraftResources.GetHue(value);
[SerializableField(0, fieldChanged: nameof(OnResourceChanged))]
[SerializedCommandProperty(AccessLevel.GameMaster)]
[InvalidateProperties]
private CraftResource _resource;
InvalidateProperties();
this.MarkDirty();
}
}
private void OnResourceChanged(CraftResource oldValue, CraftResource newValue)
{
Hue = CraftResources.GetHue(newValue);
}
public virtual bool Axe(Mobile from, BaseAxe axe) => TryCreateBoards(from, 0, new Board());

View file

@ -47,36 +47,24 @@ public partial class RecallRune : Item
}
}
[SerializableProperty(2)]
[CommandProperty(AccessLevel.Counselor, AccessLevel.GameMaster)]
public bool Marked
[SerializableField(2, fieldChanged: nameof(OnMarkedChanged))]
[SerializedCommandProperty(AccessLevel.Counselor, AccessLevel.GameMaster)]
[InvalidateProperties]
private bool _marked;
private void OnMarkedChanged(bool oldValue, bool newValue)
{
get => _marked;
set
{
if (_marked != value)
{
_marked = value;
CalculateHue();
InvalidateProperties();
}
}
CalculateHue();
}
[SerializableProperty(4)]
[CommandProperty(AccessLevel.Counselor, AccessLevel.GameMaster)]
public Map TargetMap
[SerializableField(4, fieldChanged: nameof(OnTargetMapChanged))]
[SerializedCommandProperty(AccessLevel.Counselor, AccessLevel.GameMaster)]
[InvalidateProperties]
private Map _targetMap;
private void OnTargetMapChanged(Map oldValue, Map newValue)
{
get => _targetMap;
set
{
if (_targetMap != value)
{
_targetMap = value;
CalculateHue();
InvalidateProperties();
}
}
CalculateHue();
}
private void Deserialize(IGenericReader reader, int version)

View file

@ -133,28 +133,19 @@ public partial class Spellbook : Item, ICraftable, ISlayer, IAosItem
public virtual int BookOffset => 0;
public virtual int BookCount => 64;
[CommandProperty(AccessLevel.GameMaster)]
[SerializableProperty(7)]
public ulong Content
[SerializableField(7, fieldChanged: nameof(OnContentChanged))]
[SerializedCommandProperty(AccessLevel.GameMaster)]
[InvalidateProperties]
private ulong _content;
private void OnContentChanged(ulong oldValue, ulong newValue)
{
get => _content;
set
// This assignment will mark it as dirty
SpellCount = 0;
while (newValue > 0)
{
if (_content != value)
{
_content = value;
// This assignment will mark it as dirty
SpellCount = 0;
while (value > 0)
{
_spellCount += (int)(value & 0x1);
value >>= 1;
}
InvalidateProperties();
}
_spellCount += (int)(newValue & 0x1);
newValue >>= 1;
}
}

View file

@ -62,17 +62,15 @@ public partial class RepairDeed : Item
public override bool DisplayLootType => false;
[CommandProperty(AccessLevel.GameMaster)]
[SerializableProperty(1)]
public double SkillLevel
[SerializableField(1, allowFieldChange: nameof(AllowSkillLevelChange))]
[SerializedCommandProperty(AccessLevel.GameMaster)]
[InvalidateProperties]
private double _skillLevel;
private bool AllowSkillLevelChange(ref double value)
{
get => _skillLevel;
set
{
_skillLevel = Math.Clamp(value, 0, 120.0);
InvalidateProperties();
this.MarkDirty();
}
value = Math.Clamp(value, 0, 120.0);
return true;
}
public override void AddNameProperty(IPropertyList list)

View file

@ -74,16 +74,13 @@ public abstract partial class BaseInstrument : Item, ICraftable, ISlayer
}
}
[SerializableProperty(1)]
[CommandProperty(AccessLevel.GameMaster)]
public DateTime LastReplenished
[SerializableField(1, fieldChanged: nameof(OnLastReplenishedChanged))]
[SerializedCommandProperty(AccessLevel.GameMaster)]
private DateTime _lastReplenished;
private void OnLastReplenishedChanged(DateTime oldValue, DateTime newValue)
{
get => _lastReplenished;
set
{
_lastReplenished = value;
CheckReplenishUses();
}
CheckReplenishUses();
}
[SerializableProperty(3)]

View file

@ -41,20 +41,13 @@ namespace Server.Items
public virtual bool AllowDyables => true;
[SerializableProperty(2)]
[CommandProperty(AccessLevel.GameMaster)]
public int DyedHue
{
get => _dyedHue;
set
{
if (_redyable)
{
_dyedHue = value;
Hue = value;
}
}
}
[SerializableField(2, allowFieldChange: nameof(AllowDyedHueChange), fieldChanged: nameof(OnDyedHueChanged))]
[SerializedCommandProperty(AccessLevel.GameMaster)]
private int _dyedHue;
private bool AllowDyedHueChange(ref int value) => _redyable;
private void OnDyedHueChanged(int oldValue, int newValue) => Hue = newValue;
// Three metallic tubs now.
public virtual bool MetallicHues => false;

View file

@ -60,17 +60,14 @@ public abstract partial class BaseRunicTool : BaseTool
public BaseRunicTool(CraftResource resource, int uses, int itemID) : base(uses, itemID) => _resource = resource;
[SerializableProperty(0)]
[CommandProperty(AccessLevel.GameMaster)]
public CraftResource Resource
[SerializableField(0, fieldChanged: nameof(OnResourceChanged))]
[SerializedCommandProperty(AccessLevel.GameMaster)]
[InvalidateProperties]
private CraftResource _resource;
private void OnResourceChanged(CraftResource oldValue, CraftResource newValue)
{
get => _resource;
set
{
_resource = value;
Hue = CraftResources.GetHue(_resource);
InvalidateProperties();
}
Hue = CraftResources.GetHue(_resource);
}
private void Deserialize(IGenericReader reader, int version)