warrentyexpired/ModernUO
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: Consolidate PlayerConstructed onto Item, stamped by the craft system (#2574)

Browse source 
Follow-up to #2573. That change made `SmallBOD.EndCombine` require a player-crafted item, but it could only read provenance off `BaseArmor`, `BaseWeapon` and `BaseClothing`, because those are the only three classes that track it — hence the hand-enumerated `armor?.PlayerConstructed ?? clothing?.PlayerConstructed ?? weapon?.PlayerConstructed ?? false`.

The gap is structural rather than cosmetic. `PlayerConstructed` is set inside each base's `OnCraft`, so it can only ever reach types implementing `ICraftable`. Most craftables do not — the tinkering catalogue alone is largely plain `Item` subclasses — so any rule keyed on "was this actually crafted" has nothing to key on for those types.

## What changed

Provenance moves to `Item` and is stamped centrally in `CraftItem`, immediately after the item is constructed and before the `ICraftable` dispatch, covering both the AOS and T2A craft paths. The three `OnCraft` overrides drop their now-redundant assignment and inherit `Item`'s property, so no call site outside them changes — `Resmelt` and `SalvageBag` still read `armor.PlayerConstructed` and still compile unchanged. `SmallBOD`'s three-way null-coalescing chain collapses to `item.PlayerConstructed`.

`OnCraft` is only ever invoked from `CraftItem` (the other three call sites are `base.OnCraft` chaining), so removing those assignments has no other reachable effect.

## Storage cost: none

`Item`'s `SaveFlag` word is written as a fixed-width `int`, not an encoded one, so occupying bit `0x08000000` changes no record lengths. Items that are not player-constructed serialize byte for byte as before, and crafted ones differ by a single bit in a field already being written.

`Item` itself needs no version bump: a bare `SaveFlag` bit is self-describing, so records written before it existed lack it and read `false`.

## Version bumps

The three content classes do need one, since removing a serialized field changes their layout:

| Class | Version | Field removed |
|---|---|---|
| `BaseArmor` | 9 → 10 | 24 (was last, nothing renumbered) |
| `BaseClothing` | 7 → 8 | 7 (fields 8–10 shift down) |
| `BaseWeapon` | 10 → 11 | 26 (fields 27–30 shift down) |

Each gets a `MigrateFrom` for its previous version that assigns the old bool to the inherited property, so existing crafted armour, weapons and clothing keep their provenance across the upgrade. `Item.Deserialize` runs first and reads the absent bit as `false`, then the migration overwrites it — the generated `Deserialize` calls `base.Deserialize` before dispatching, so the ordering holds. `BaseWeapon` had no migrations file and gains one.

The renumbering is not stylistic: the generator requires contiguous field ordering and rejects a hole with `SG3005: Expected field 'Crafter' with order 7 but found 8`.

New schema JSONs (`BaseArmor.v10`, `BaseClothing.v8`, `BaseWeapon.v11`) are generated by `ModernUOSchemaGenerator` and committed alongside.

## One thing worth a second opinion

The new property is a plain auto-property on `Item`, so it does not call `this.MarkDirty()` the way the codegen setters it replaces did. `MarkDirty` is currently a no-op (`// TODO: Add dirty tracking back`) and no property in `Item.cs` calls it, so this matches the file as it stands — but it is worth noting if dirty tracking comes back.

## Verification

Full solution builds in Release with 0 errors and 0 warnings; 1516 tests pass (815 `Server.Tests`, 701 `UOContent.Tests`).
This commit is contained in:
Kamron Batman 2026-08-13 18:34:59 -07:00 • committed by GitHub
parent 5ce0f1e92b
commit bd79cb7759
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 675 additions and 52 deletions
Download patch file Download diff file Expand all files Collapse all files

31
Projects/UOContent/Items/Weapons/BaseWeapon.cs
View file

@ -27,7 +27,7 @@ public interface ISlayer
SlayerName Slayer2 { get; set; }
}
[SerializationGenerator(10, false)]
[SerializationGenerator(11, false)]
public abstract partial class BaseWeapon
: Item, IWeapon, IFactionItem, ICraftable, ISlayer, IDurability, IAosItem, IIdentifiable
{
@ -141,53 +141,45 @@ public abstract partial class BaseWeapon
[SerializableFieldDefault(25)]
private AosWeaponAttributes WeaponAttributesDefaultValue() => new(this);
[SerializableField(26)]
[SerializedCommandProperty(AccessLevel.GameMaster)]
private bool _playerConstructed;
[SerializableFieldSaveFlag(26)]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private bool ShouldSerializePlayerConstructed() => _playerConstructed;
[SerializedIgnoreDupe]
[SerializableField(27, setter: "private")]
[SerializableField(26, setter: "private")]
[SerializedCommandProperty(AccessLevel.GameMaster, canModify: true)]
private AosSkillBonuses _skillBonuses;
[SerializableFieldSaveFlag(27)]
[SerializableFieldSaveFlag(26)]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private bool ShouldSerializeSkillBonuses() => !_skillBonuses.IsEmpty;
[SerializableFieldDefault(27)]
[SerializableFieldDefault(26)]
private AosSkillBonuses SkillBonusesDefaultValue() => new(this);
[InvalidateProperties]
[SerializableField(28)]
[SerializableField(27)]
[SerializedCommandProperty(AccessLevel.GameMaster)]
private SlayerName _slayer2;
[SerializableFieldSaveFlag(28)]
[SerializableFieldSaveFlag(27)]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private bool ShouldSerializeSlayer2() => _slayer2 != SlayerName.None;
[SerializedIgnoreDupe]
[SerializableField(29, setter: "private")]
[SerializableField(28, setter: "private")]
[SerializedCommandProperty(AccessLevel.GameMaster, canModify: true)]
private AosElementAttributes _aosElementDamages;
[SerializableFieldSaveFlag(29)]
[SerializableFieldSaveFlag(28)]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private bool ShouldSerializeElementAttributes() => !_aosElementDamages.IsEmpty;
[SerializableFieldDefault(29)]
[SerializableFieldDefault(28)]
private AosElementAttributes AosElementAttributesDefaultValue() => new(this);
[InvalidateProperties]
[SerializableField(30)]
[SerializableField(29)]
[SerializedCommandProperty(AccessLevel.GameMaster)]
private string _engravedText;
[SerializableFieldSaveFlag(30)]
[SerializableFieldSaveFlag(29)]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private bool ShouldSerializeEngravedText() => !string.IsNullOrEmpty(_engravedText);
@ -674,7 +666,6 @@ public abstract partial class BaseWeapon
Crafter = from.RawName;
}
PlayerConstructed = true;
Identified = true;
var resourceType = typeRes ?? craftItem.Resources[0].ItemType;