feat: scope container child removal, skip-serialize and linearly delete container children (#2676)

## Summary

Removing an item from a container now reaches only the clients that were sent the item. This PR also adds container-scoped skip serialization, makes container deletion linear, and closes a lift-rejection resync gap. It is the groundwork for the NPC vendor buyback fix (#2677), and it is safe to merge by itself.

### Container child removal reaches only the clients that were sent the item

Adds and updates for an item inside a private container have always gone only to the root mobile, the secure-trade partner, and the container's `Openers` (`ProcessDelta`, the same as RunUO). Removes, though, were broadcast to every client in range. That too is inherited from RunUO; #326 only converted the packets. Two costs:
- **Bandwidth.** One packet per nearby client for every reagent, bandage or arrow stack that runs out, and every container-to-container move.
- **Disclosure.** It sent the serial and timing of items moved in someone else's pack to every nearby client.

Now:
- **`Item.SendRemovePacket`:** a private container child's remove goes only to the root mobile, both secure-trade parties (found by walking up to the trade container, as `ProcessDelta` does), and the container's `Openers`. Each recipient is deduplicated and must be on the same map and in range.
- **`Container.IsChildPublic(Item child)`** (default `IsPublicContainer`) marks children that were sent to every client in range. Their removal still broadcasts. Overrides can only widen: a public container's children always broadcast.
- **`Corpse`** marks its worn gear public on human corpses. `SendInfoTo` shows that gear to everyone who sees the corpse, so bystanders still see looted gear disappear. Loot stays private, and corpses are not made public.
- **`IsPublicContainer` is unchanged** and remains the escape hatch.
- **Unchanged:** items equipped on mobiles, children of non-container items (spellbooks, bulletin boards), and ground items.
- **`Item.Map` no longer sends removes for contained items.** A contained item only changes map along with its parent or root, and that ancestor's own remove already clears the subtree for everyone who knew it. The owner keeps their nested bags across a facet change, as before, and bystanders no longer get a remove per nested item.

**For forks:** a custom container that sends its contents to clients other than the root, trade parties and openers must override `IsChildPublic` or `IsPublicContainer`. Otherwise those clients keep the item until they reopen the container, it leaves their range, or they log out.

- **`Item.MoveToWorld` no longer sends a second remove for an item that had a parent.** It used to detach through the parent's `RemoveItem` (correctly scoped), then broadcast another remove from the old location. Every lift goes through `Internalize()`, so every lift from a pack or container broadcast the item's serial. Ground items are unchanged.
- **Property-only updates** (`ItemDelta.Properties` without `Update`) of a private child now send the object property list revision only to the same recipients, not every client in range. Gump tooltips that reference such an item without the client holding it no longer get the revision nudge; their cached tooltip refreshes when re-requested.

### Lift rejection resyncs only items the requester was sent

A rejected lift re-sent the item's full data to the requester without checking that their client had ever been sent it. `Item.IsSentTo(Mobile)` now applies the same rule as the adds, including `CanSee`:
- **Private container children:** the root mobile, the trade parties, or the openers.
- **Everything else:** visible and in update range.

`Mobile.Lift` still always sends the lift-reject packet, but resyncs the item only when `IsSentTo` holds.

### Container content packets stay within the protocol limit

`SendContainerContent` stack-allocated `items.Count × 20` bytes, which overflowed the stack for a container with hundreds of thousands of items. The `0x3C` length and count fields are 16-bit anyway. It now caps entries so the packet fits (3448 entries, or 3276 with grid lines), and rents a pooled buffer above a small size. Output is byte-identical for normal containers.

### Container-scoped skip serialization

**`Container.SkipsChildSerialization`** (default `false`). When a container returns true:
- its direct children are left out of world saves (the base `Item.SkipSerialization` checks the parent);
- it writes no child list, so a skipped child's serial can never resolve to an unrelated item at load;
- an item that loads naming such a container as its parent is deleted by the existing missing-parent path.

**Known limitation:** `SkipSerialization` is read while the save files are written, after the world unfreezes. An item that enters an opted-in container in that window can be left out of that save. The next save is correct. Moving the decision into the freeze is a follow-up.

### Linear container deletion

`Item.Delete` removes children back to front, but `RemoveItem` searched from the front with `List.Remove`, so deleting a container of *n* items was O(n²). `RemoveItem` now checks the last entry first.
- 100,000 children took 8.0s before and 162ms after.
- Other removals cost the same as before.

**Save format unchanged for every existing type** (the schema generator produces no diff).

## Test plan

- [x] `ContainerChildRemovalTests`:
  - a private child's removal isn't broadcast;
  - openers, the root owner's client and a trade party (who never opened the pouch) still receive it;
  - public containers and per-child `IsChildPublic` overrides still broadcast;
  - an override can't narrow a public container;
  - no duplicate packets when root and opener overlap;
  - deleting the owner sends no remove per child;
  - a facet change sends no remove for nested children to the owner or bystanders;
  - lifting from a backpack or moving a contained item to the ground sends no remove to bystanders;
  - equipped items and ground items keep their broadcast.
- [x] `PrivateChildPropertiesTests`: a property-only update of a private child reaches the owner and openers, not bystanders; public-container children and ground items still broadcast.
- [x] `CorpseChildPublicTests`: human corpse worn gear is public and loot isn't, bones and non-human bodies aren't; end to end, worn gear leaving the corpse reaches a bystander and deleted loot doesn't.
- [x] `LiftRejectResyncTests`:
  - a bystander and an out-of-range requester get no resync;
  - the owner, an opener and a trade party do;
  - an invisible item isn't resynced to a non-staff owner, but is to staff;
  - equipment on a hidden mobile and an invisible ground item aren't resynced.
- [x] `ContainerPacketTests`: large containers are capped at the protocol limit (with and without grid lines) and the pooled-buffer path is byte-identical.
- [x] `ContainerChildSkipTests`, `ContainerBulkRemovalTests`.
- [x] Server.Tests (944) and UOContent.Tests (1203) green on current `main`; schema generator clean.
- [x] Manual in-game pass with three clients:
  - consumption;
  - snooping;
  - co-opened containers;
  - bank;
  - trade with and without opening the pouch;
  - facet change with nested bags;
  - pack animals;
  - player vendors;
  - human corpse looting by drag and by script;
  - every lift-rejection reason.

  Verified with a ClassicUO build instrumented to count:
  - removes for unknown serials;
  - adds dropped for an unknown parent;
  - items learned inside containers the client never opened;
  - lift resyncs of items the client never had.
This commit is contained in:
Kamron Batman 2026-10-09 22:21:50 -07:00 • committed by GitHub
parent 1992c9d1b5
commit 89a86d81b4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 1808 additions and 37 deletions

View file

@ -0,0 +1,58 @@
using Server.Items;
using Xunit;
namespace Server.Tests;
[Collection("Sequential Server Tests")]
public class ContainerBulkRemovalTests
{
[Fact]
public void DeletingLargeContainer_DeletesAllChildren_AndTotalsStayConsistent()
{
var outer = new Container(0xE75);
var pack = new Container(0xE75);
outer.DropItem(pack);
var children = new Item[100_000];
for (var i = 0; i < children.Length; i++)
{
children[i] = new Item(0x1234);
pack.DropItem(children[i]);
}
Assert.Equal(children.Length + 1, outer.TotalItems);
pack.Delete();
for (var i = 0; i < children.Length; i++)
{
Assert.True(children[i].Deleted);
}
Assert.Empty(outer.Items);
Assert.Equal(0, outer.TotalItems);
outer.Delete();
}
// The tail fast path must not change which entry is removed when the item is elsewhere.
[Fact]
public void RemovingFromFrontMiddleAndEnd_RemovesExactlyThatItem()
{
var pack = new Container(0xE75);
var items = new Item[5];
for (var i = 0; i < items.Length; i++)
{
items[i] = new Item(0x1234);
pack.DropItem(items[i]);
}
items[0].Delete();
items[2].Delete();
items[4].Delete();
Assert.Equal(2, pack.Items.Count);
Assert.Same(items[1], pack.Items[0]);
Assert.Same(items[3], pack.Items[1]);
pack.Delete();
}
}

View file

@ -0,0 +1,462 @@
using System;
using Server.Items;
using Server.Network;
using Server.Tests.Network;
using Xunit;
namespace Server.Tests;
[Collection("Sequential Server Tests")]
public class ContainerChildRemovalTests
{
private class PublicContainer : Container
{
public PublicContainer() : base(0xE75)
{
}
public override bool IsPublicContainer => true;
}
private class OneChildPublicContainer : Container
{
public OneChildPublicContainer() : base(0xE75)
{
}
public Item PublicChild { get; set; }
public override bool IsChildPublic(Item child) => child == PublicChild;
}
// An override that narrows below IsPublicContainer instead of only widening it, exercising the guard
// that keeps a public container's removals broadcast regardless of what IsChildPublic returns.
private class PublicContainerWithNarrowingOverride : Container
{
public PublicContainerWithNarrowingOverride() : base(0xE75)
{
}
public override bool IsPublicContainer => true;
public override bool IsChildPublic(Item child) => false;
}
private static readonly Point3D _ownerLoc = new(1500, 1500, 0);
private static (NetState, Mobile) CreateClient(Point3D location)
{
var ns = PacketTestUtilities.CreateTestNetState();
ns.Account = new MockAccount();
var mobile = new Mobile(World.NewMobile);
mobile.DefaultMobileInit();
ns.Mobile = mobile;
mobile.NetState = ns;
mobile.MoveToWorld(location, Map.Felucca);
return (ns, mobile);
}
private static Mobile CreateOwnerWith(Container pack)
{
var owner = new Mobile(World.NewMobile);
owner.DefaultMobileInit();
owner.MoveToWorld(_ownerLoc, Map.Felucca);
pack.Layer = Layer.ShopBuy;
pack.Visible = false;
owner.AddItem(pack);
return owner;
}
private static bool ReceivedRemove(NetState ns, Serial serial)
{
Span<byte> expected = stackalloc byte[OutgoingEntityPackets.RemoveEntityLength];
OutgoingEntityPackets.CreateRemoveEntity(expected, serial);
return ns.SendBuffer.GetReadSpan().IndexOf(expected) >= 0;
}
private static int CountRemoves(NetState ns, Serial serial)
{
Span<byte> expected = stackalloc byte[OutgoingEntityPackets.RemoveEntityLength];
OutgoingEntityPackets.CreateRemoveEntity(expected, serial);
var span = ns.SendBuffer.GetReadSpan();
var count = 0;
while (true)
{
var index = span.IndexOf(expected);
if (index < 0)
{
break;
}
count++;
span = span[(index + expected.Length)..];
}
return count;
}
private static void DisposeClient(NetState ns, Mobile mobile)
{
ns.Mobile = null;
ns.Dispose();
mobile.Delete();
}
private static void Cleanup(NetState ns, Mobile client, Mobile owner)
{
DisposeClient(ns, client);
owner.Delete();
}
[Fact]
public void PrivateChildDelete_IsNotBroadcastToNearbyClients()
{
var (ns, client) = CreateClient(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0));
var owner = CreateOwnerWith(new Container(0xE75));
var child = new Item(0x1234);
((Container)owner.FindItemOnLayer(Layer.ShopBuy)).DropItem(child);
try
{
child.Delete();
Assert.False(ReceivedRemove(ns, child.Serial));
}
finally
{
Cleanup(ns, client, owner);
}
}
[Fact]
public void PrivateChildDelete_ReachesOpeners()
{
var (ns, client) = CreateClient(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0));
var pack = new Container(0xE75);
var owner = CreateOwnerWith(pack);
var child = new Item(0x1234);
pack.DropItem(child);
pack.Openers = [client];
try
{
child.Delete();
Assert.True(ReceivedRemove(ns, child.Serial));
}
finally
{
Cleanup(ns, client, owner);
}
}
[Fact]
public void PrivateChildDelete_ReachesTheRootOwnersClient()
{
var (ownerNs, owner) = CreateClient(_ownerLoc);
var (bystanderNs, bystander) = CreateClient(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0));
var pack = new Container(0xE75)
{
Layer = Layer.Backpack,
Visible = false
};
owner.AddItem(pack);
var child = new Item(0x1234);
pack.DropItem(child);
try
{
child.Delete();
Assert.True(ReceivedRemove(ownerNs, child.Serial));
Assert.False(ReceivedRemove(bystanderNs, child.Serial));
}
finally
{
DisposeClient(ownerNs, owner);
DisposeClient(bystanderNs, bystander);
}
}
[Fact]
public void PublicContainerChildDelete_IsBroadcast()
{
var (ns, client) = CreateClient(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0));
var owner = CreateOwnerWith(new PublicContainer());
var child = new Item(0x1234);
((Container)owner.FindItemOnLayer(Layer.ShopBuy)).DropItem(child);
try
{
child.Delete();
Assert.True(ReceivedRemove(ns, child.Serial));
}
finally
{
Cleanup(ns, client, owner);
}
}
[Fact]
public void PublicContainer_NarrowingChildOverride_StillBroadcastsRemoval()
{
var (ns, client) = CreateClient(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0));
var owner = CreateOwnerWith(new PublicContainerWithNarrowingOverride());
var child = new Item(0x1234);
((Container)owner.FindItemOnLayer(Layer.ShopBuy)).DropItem(child);
try
{
child.Delete();
Assert.True(ReceivedRemove(ns, child.Serial));
}
finally
{
Cleanup(ns, client, owner);
}
}
[Fact]
public void IsChildPublic_BroadcastsOnlyThatChild()
{
var (ns, client) = CreateClient(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0));
var pack = new OneChildPublicContainer();
var owner = CreateOwnerWith(pack);
var publicChild = new Item(0x1234);
var privateChild = new Item(0x1235);
pack.DropItem(publicChild);
pack.DropItem(privateChild);
pack.PublicChild = publicChild;
try
{
privateChild.Delete();
Assert.False(ReceivedRemove(ns, privateChild.Serial));
publicChild.Delete();
Assert.True(ReceivedRemove(ns, publicChild.Serial));
}
finally
{
Cleanup(ns, client, owner);
}
}
[Fact]
public void DeletingOwner_SendsNoRemoveForPrivateChildren()
{
var (ns, client) = CreateClient(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0));
var pack = new Container(0xE75);
var owner = CreateOwnerWith(pack);
var children = new Item[50];
for (var i = 0; i < children.Length; i++)
{
children[i] = new Item(0x1234);
pack.DropItem(children[i]);
}
try
{
owner.Delete();
for (var i = 0; i < children.Length; i++)
{
Assert.True(children[i].Deleted);
Assert.False(ReceivedRemove(ns, children[i].Serial));
}
}
finally
{
Cleanup(ns, client, owner);
}
}
[Fact]
public void TradePartner_ReceivesRemoveForNestedItem()
{
var (nsA, a) = CreateClient(_ownerLoc);
var (nsB, b) = CreateClient(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0));
var trade = new SecureTrade(a, b);
var pouch = new Container(0xE75);
trade.From.Container.DropItem(pouch);
var item = new Item(0x1234);
pouch.DropItem(item);
try
{
item.Delete();
Assert.True(ReceivedRemove(nsB, item.Serial));
}
finally
{
trade.From.Container.Delete();
trade.To.Container.Delete();
DisposeClient(nsA, a);
DisposeClient(nsB, b);
}
}
[Fact]
public void FacetChange_DoesNotRemoveNestedContainerChildren()
{
var (ownerNs, owner) = CreateClient(_ownerLoc);
var (bystanderNs, bystander) = CreateClient(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0));
var backpack = new Container(0xE75) { Layer = Layer.Backpack };
owner.AddItem(backpack);
var pouch = new Container(0xE75);
backpack.DropItem(pouch);
var reagent = new Item(0xF7A);
pouch.DropItem(reagent);
try
{
owner.MoveToWorld(_ownerLoc, Map.Trammel);
Assert.False(ReceivedRemove(ownerNs, pouch.Serial));
Assert.False(ReceivedRemove(ownerNs, reagent.Serial));
Assert.False(ReceivedRemove(bystanderNs, pouch.Serial));
Assert.False(ReceivedRemove(bystanderNs, reagent.Serial));
}
finally
{
DisposeClient(ownerNs, owner);
DisposeClient(bystanderNs, bystander);
}
}
[Fact]
public void NoDuplicateRemove_WhenRootIsAlsoAnOpener()
{
var (ownerNs, owner) = CreateClient(_ownerLoc);
var pack = new Container(0xE75)
{
Layer = Layer.Backpack,
Visible = false
};
owner.AddItem(pack);
pack.Openers = [owner];
var child = new Item(0x1234);
pack.DropItem(child);
try
{
child.Delete();
Assert.Equal(1, CountRemoves(ownerNs, child.Serial));
}
finally
{
DisposeClient(ownerNs, owner);
}
}
[Fact]
public void LiftingFromOwnBackpack_DoesNotBroadcastRemove()
{
var (ownerNs, owner) = CreateClient(_ownerLoc);
var (bystanderNs, bystander) = CreateClient(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0));
var pack = new Container(0xE75) { Layer = Layer.Backpack };
owner.AddItem(pack);
var item = new Item(0x1234);
pack.DropItem(item);
try
{
item.Internalize();
Assert.True(ReceivedRemove(ownerNs, item.Serial));
Assert.False(ReceivedRemove(bystanderNs, item.Serial));
}
finally
{
item.Delete();
DisposeClient(ownerNs, owner);
DisposeClient(bystanderNs, bystander);
}
}
[Fact]
public void MovingPrivateChildToGroundSameMap_DoesNotSendOldLocationRemoveToBystanders()
{
var (ownerNs, owner) = CreateClient(_ownerLoc);
var (bystanderNs, bystander) = CreateClient(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0));
var pack = new Container(0xE75) { Layer = Layer.Backpack };
owner.AddItem(pack);
var item = new Item(0x1234);
pack.DropItem(item);
var farLocation = new Point3D(_ownerLoc.X + 100, _ownerLoc.Y, 0);
try
{
item.MoveToWorld(farLocation, Map.Felucca);
Assert.False(ReceivedRemove(bystanderNs, item.Serial));
}
finally
{
item.Delete();
DisposeClient(ownerNs, owner);
DisposeClient(bystanderNs, bystander);
}
}
[Fact]
public void LiftingEquippedItem_StillBroadcastsRemove()
{
var (ownerNs, owner) = CreateClient(_ownerLoc);
var (bystanderNs, bystander) = CreateClient(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0));
var weapon = new Item(0x1234) { Layer = Layer.OneHanded };
owner.AddItem(weapon);
try
{
weapon.Internalize();
Assert.True(ReceivedRemove(bystanderNs, weapon.Serial));
}
finally
{
weapon.Delete();
DisposeClient(ownerNs, owner);
DisposeClient(bystanderNs, bystander);
}
}
[Fact]
public void GroundItemMove_StillSendsOldLocationRemove()
{
var (bystanderNs, bystander) = CreateClient(_ownerLoc);
var groundLocation = new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0);
var item = new Item(0x1234);
item.MoveToWorld(groundLocation, Map.Felucca);
var farLocation = new Point3D(_ownerLoc.X + 100, _ownerLoc.Y, 0);
try
{
item.MoveToWorld(farLocation, Map.Felucca);
Assert.True(ReceivedRemove(bystanderNs, item.Serial));
}
finally
{
item.Delete();
DisposeClient(bystanderNs, bystander);
}
}
}

View file

@ -0,0 +1,146 @@
using System;
using Server.Items;
using Xunit;
namespace Server.Tests;
[Collection("Sequential Server Tests")]
public class ContainerChildSkipTests
{
private class SkippingContainer : Container
{
public SkippingContainer() : base(0xE75)
{
}
public SkippingContainer(Serial serial) : base(serial)
{
}
public override bool SkipsChildSerialization => true;
}
[Fact]
public void ChildOfSkippingContainer_SkipsUntilMovedOut()
{
var pack = new SkippingContainer();
var normal = new Container(0xE75);
var item = new Item(0x1234);
try
{
pack.DropItem(item);
Assert.True(item.SkipSerialization);
Assert.False(pack.SkipSerialization);
normal.DropItem(item);
Assert.False(item.SkipSerialization);
}
finally
{
item.Delete();
pack.Delete();
normal.Delete();
}
}
[Fact]
public void GrandchildOfSkippingContainer_IsNotSkipped()
{
var pack = new SkippingContainer();
var inner = new Container(0xE75);
var item = new Item(0x1234);
try
{
pack.DropItem(inner);
inner.DropItem(item);
Assert.True(inner.SkipSerialization);
Assert.False(item.SkipSerialization);
}
finally
{
pack.Delete();
}
}
// A skipped child is not saved, so the container must load without it.
[Fact]
public void SkippingContainer_LoadsWithoutItsSkippedChildren()
{
var pack = new SkippingContainer();
var item = new Item(0x1234);
pack.DropItem(item);
var writer = new BufferWriter(true);
pack.Serialize(writer);
var buffer = new byte[writer.Position];
writer.Buffer.AsSpan(0, (int)writer.Position).CopyTo(buffer);
item.Delete();
pack.Delete();
var copy = new SkippingContainer(World.NewItem);
copy.Deserialize(new BufferReader(buffer));
Assert.Empty(copy.Items);
}
// A written child serial could resolve at load to an unrelated item that was handed the same serial.
[Fact]
public void SkippingContainer_DoesNotWriteAliveChildSerial()
{
var pack = new SkippingContainer();
var item = new Item(0x1234);
pack.DropItem(item);
var writer = new BufferWriter(true);
pack.Serialize(writer);
var buffer = new byte[writer.Position];
writer.Buffer.AsSpan(0, (int)writer.Position).CopyTo(buffer);
// item stays alive and in pack's list past this point, so its serial is still resolvable
// when copy deserializes below - deleting pack cascades to item, so both go together.
try
{
var copy = new SkippingContainer(World.NewItem);
copy.Deserialize(new BufferReader(buffer));
Assert.Empty(copy.Items);
}
finally
{
pack.Delete();
}
}
// An item can be saved naming a skipping container as parent while absent from its list; it must load
// as parentless so the missing-parent path deletes it rather than leaving an unreachable orphan.
[Fact]
public void ItemLoadingWithSkippingContainerAsParent_ClearsParent()
{
var pack = new SkippingContainer();
var item = new Item(0x1234);
pack.DropItem(item);
var writer = new BufferWriter(true);
item.Serialize(writer);
var buffer = new byte[writer.Position];
writer.Buffer.AsSpan(0, (int)writer.Position).CopyTo(buffer);
item.Delete();
try
{
var copy = new Item(World.NewItem);
copy.Deserialize(new BufferReader(buffer));
Assert.Null(copy.Parent);
}
finally
{
pack.Delete();
}
}
}

View file

@ -0,0 +1,311 @@
using System;
using System.Buffers;
using Server.Items;
using Server.Network;
using Server.Tests.Network;
using Xunit;
namespace Server.Tests;
[Collection("Sequential Server Tests")]
public class LiftRejectResyncTests
{
private static readonly Point3D _baseLoc = new(2200, 2200, 0);
private static (NetState, Mobile) CreateClient(Point3D location)
{
var ns = PacketTestUtilities.CreateTestNetState();
ns.Account = new MockAccount();
var mobile = new Mobile(World.NewMobile);
mobile.DefaultMobileInit();
ns.Mobile = mobile;
mobile.NetState = ns;
mobile.MoveToWorld(location, Map.Felucca);
return (ns, mobile);
}
private static void DisposeClient(NetState ns, Mobile mobile)
{
ns.Mobile = null;
ns.Dispose();
mobile.Delete();
}
private static bool ReceivedContentUpdate(NetState ns, Serial serial)
{
Span<byte> expected = stackalloc byte[5];
var writer = new SpanWriter(expected);
writer.Write((byte)0x25); // ContainerContentUpdate packet ID
writer.Write(serial);
return ns.SendBuffer.GetReadSpan().IndexOf(expected) >= 0;
}
private static bool ReceivedWorldItem(NetState ns, Item item)
{
Span<byte> expected = stackalloc byte[OutgoingEntityPackets.MaxWorldEntityPacketLength];
var length = OutgoingItemPackets.CreateWorldItem(expected, item);
return ns.SendBuffer.GetReadSpan().IndexOf(expected[..length]) >= 0;
}
private static bool ReceivedEquipUpdate(NetState ns, Serial serial)
{
Span<byte> expected = stackalloc byte[5];
var writer = new SpanWriter(expected);
writer.Write((byte)0x2E); // EquipUpdate packet ID
writer.Write(serial);
return ns.SendBuffer.GetReadSpan().IndexOf(expected) >= 0;
}
[Fact]
public void PrivateNestedItem_RejectedLift_DoesNotResyncBystander()
{
var (ownerNs, owner) = CreateClient(_baseLoc);
var (bystanderNs, bystander) = CreateClient(new Point3D(_baseLoc.X + 1, _baseLoc.Y, 0));
var backpack = new Container(0xE75) { Layer = Layer.Backpack };
owner.AddItem(backpack);
var pouch = new Container(0xE75);
backpack.DropItem(pouch);
var item = new Item(0x1234);
pouch.DropItem(item);
try
{
bystander.Lift(item, item.Amount, out var rejected, out _);
// Whichever check trips first (CheckNonlocalLift, accessibility, ...), the bystander was
// never sent this private child, so the rejection must not resynchronize it to them.
Assert.True(rejected);
Assert.False(ReceivedContentUpdate(bystanderNs, item.Serial));
}
finally
{
DisposeClient(ownerNs, owner);
DisposeClient(bystanderNs, bystander);
}
}
[Fact]
public void OwnerAlreadyHolding_RejectedLift_ResyncsPrivateChildToOwner()
{
var (ownerNs, owner) = CreateClient(_baseLoc);
var backpack = new Container(0xE75) { Layer = Layer.Backpack };
owner.AddItem(backpack);
var item = new Item(0x1234);
backpack.DropItem(item);
owner.Holding = new Item(0x1);
try
{
owner.Lift(item, item.Amount, out var rejected, out var reject);
Assert.True(rejected);
Assert.Equal(LRReason.AreHolding, reject);
Assert.True(ReceivedContentUpdate(ownerNs, item.Serial));
}
finally
{
owner.Holding?.Delete();
DisposeClient(ownerNs, owner);
}
}
[Fact]
public void GroundItemOutOfRange_RejectedLift_DoesNotResyncRequester()
{
var (requesterNs, requester) = CreateClient(_baseLoc);
var item = new Item(0x1234);
item.MoveToWorld(new Point3D(_baseLoc.X, _baseLoc.Y + 100, 0), Map.Felucca);
try
{
requester.Lift(item, item.Amount, out var rejected, out var reject);
Assert.True(rejected);
Assert.Equal(LRReason.OutOfRange, reject);
Assert.False(ReceivedWorldItem(requesterNs, item));
}
finally
{
DisposeClient(requesterNs, requester);
item.Delete();
}
}
[Fact]
public void GroundContainerOpener_ForcedRejection_ResyncsPrivateChildToOpener()
{
var (requesterNs, requester) = CreateClient(_baseLoc);
var container = new Container(0xE75);
container.MoveToWorld(new Point3D(_baseLoc.X + 1, _baseLoc.Y, 0), Map.Felucca);
var item = new Item(0x1234);
container.DropItem(item);
container.Openers = [requester];
requester.Holding = new Item(0x1);
try
{
requester.Lift(item, item.Amount, out var rejected, out var reject);
Assert.True(rejected);
Assert.Equal(LRReason.AreHolding, reject);
Assert.True(ReceivedContentUpdate(requesterNs, item.Serial));
}
finally
{
requester.Holding?.Delete();
DisposeClient(requesterNs, requester);
container.Delete();
}
}
[Fact]
public void TradePartner_RejectedLift_ResyncsNestedItemToTradePartner()
{
var (aNs, a) = CreateClient(_baseLoc);
var (bNs, b) = CreateClient(new Point3D(_baseLoc.X + 1, _baseLoc.Y, 0));
var trade = new SecureTrade(a, b);
var pouch = new Container(0xE75);
trade.From.Container.DropItem(pouch);
var item = new Item(0x1234);
pouch.DropItem(item);
b.Holding = new Item(0x1);
try
{
// b is a's trade partner: not the item's root and never an Opener of the pouch, but a
// sanctioned recipient of this private child through the trade relationship.
b.Lift(item, item.Amount, out var rejected, out var reject);
Assert.True(rejected);
Assert.Equal(LRReason.AreHolding, reject);
Assert.True(ReceivedContentUpdate(bNs, item.Serial));
}
finally
{
b.Holding?.Delete();
trade.From.Container.Delete();
trade.To.Container.Delete();
DisposeClient(aNs, a);
DisposeClient(bNs, b);
}
}
[Fact]
public void InvisibleItem_RejectedLift_DoesNotResyncToNonStaffOwner()
{
var (ownerNs, owner) = CreateClient(_baseLoc);
var backpack = new Container(0xE75) { Layer = Layer.Backpack };
owner.AddItem(backpack);
var item = new Item(0x1234) { Visible = false };
backpack.DropItem(item);
owner.Holding = new Item(0x1);
try
{
owner.Lift(item, item.Amount, out var rejected, out var reject);
// ProcessDelta never sends an invisible item to a non-staff owner (CanSee gates every
// private recipient); the rejection resync must honor the same gate.
Assert.True(rejected);
Assert.Equal(LRReason.AreHolding, reject);
Assert.False(ReceivedContentUpdate(ownerNs, item.Serial));
}
finally
{
owner.Holding?.Delete();
DisposeClient(ownerNs, owner);
}
}
[Fact]
public void InvisibleItem_RejectedLift_ResyncsToStaffOwner()
{
var (ownerNs, owner) = CreateClient(_baseLoc);
owner.AccessLevel = AccessLevel.GameMaster;
var backpack = new Container(0xE75) { Layer = Layer.Backpack };
owner.AddItem(backpack);
var item = new Item(0x1234) { Visible = false };
backpack.DropItem(item);
owner.Holding = new Item(0x1);
try
{
owner.Lift(item, item.Amount, out var rejected, out var reject);
Assert.True(rejected);
Assert.Equal(LRReason.AreHolding, reject);
Assert.True(ReceivedContentUpdate(ownerNs, item.Serial));
}
finally
{
owner.Holding?.Delete();
DisposeClient(ownerNs, owner);
}
}
[Fact]
public void EquippedItemOnHiddenMobile_RejectedLift_NoEquipResync()
{
var (ownerNs, owner) = CreateClient(_baseLoc);
var (requesterNs, requester) = CreateClient(new Point3D(_baseLoc.X + 1, _baseLoc.Y, 0));
owner.Hidden = true;
var weapon = new Item(0x1234) { Layer = Layer.OneHanded };
owner.AddItem(weapon);
try
{
requester.Lift(weapon, weapon.Amount, out var rejected, out var reject);
// The requester can't see a hidden, non-staff owner: the lift is rejected OutOfSight and
// IsSentTo's CanSee gate must suppress the equip resync.
Assert.True(rejected);
Assert.Equal(LRReason.OutOfSight, reject);
Assert.False(ReceivedEquipUpdate(requesterNs, weapon.Serial));
}
finally
{
weapon.Delete();
DisposeClient(ownerNs, owner);
DisposeClient(requesterNs, requester);
}
}
[Fact]
public void InvisibleGroundItemInRange_RejectedLift_NoWorldResync()
{
var (requesterNs, requester) = CreateClient(_baseLoc);
var item = new Item(0x1234) { Visible = false };
item.MoveToWorld(new Point3D(_baseLoc.X + 1, _baseLoc.Y, 0), Map.Felucca);
try
{
requester.Lift(item, item.Amount, out var rejected, out var reject);
// In range this time (unlike GroundItemOutOfRange above): only Visible gates it, so the
// rejection resync must not hand the requester a world packet for an item they can't see.
Assert.True(rejected);
Assert.Equal(LRReason.OutOfSight, reject);
Assert.False(ReceivedWorldItem(requesterNs, item));
}
finally
{
DisposeClient(requesterNs, requester);
item.Delete();
}
}
}

View file

@ -0,0 +1,187 @@
using System;
using System.Buffers;
using Server.Items;
using Server.Network;
using Server.Tests.Network;
using Xunit;
namespace Server.Tests;
[Collection("Sequential Server Tests")]
public class PrivateChildPropertiesTests
{
private class PublicContainer : Container
{
public PublicContainer() : base(0xE75)
{
}
public override bool IsPublicContainer => true;
}
private static readonly Point3D _ownerLoc = new(1600, 1600, 0);
private static (NetState, Mobile) CreateClient(Point3D location)
{
var ns = PacketTestUtilities.CreateTestNetState();
ns.Account = new MockAccount();
var mobile = new Mobile(World.NewMobile);
mobile.DefaultMobileInit();
ns.Mobile = mobile;
mobile.NetState = ns;
mobile.MoveToWorld(location, Map.Felucca);
return (ns, mobile);
}
private static void DisposeClient(NetState ns, Mobile mobile)
{
ns.Mobile = null;
ns.Dispose();
mobile.Delete();
}
private static bool ReceivedOplInfo(NetState ns, Serial serial)
{
Span<byte> expected = stackalloc byte[5];
var writer = new SpanWriter(expected);
writer.Write((byte)0xDC); // OPL info packet ID
writer.Write(serial);
return ns.SendBuffer.GetReadSpan().IndexOf(expected) >= 0;
}
// AddItem/MoveToWorld already queue their own Update (+Properties) delta as a side effect of the
// Map setter. Draining it first isolates the properties-only delta the test actually exercises;
// otherwise the leftover Update flag routes through the Update branch and the test would pass or
// fail for the wrong reason.
private static void DrainPendingDelta(Item item) => item.ProcessDelta();
private static void InvalidateAndFlush(Item item)
{
// InvalidateProperties() only queues a delta when the OPL hash actually changes, which the
// drain above already stabilized. Queue ItemDelta.Properties directly so the test deterministically
// exercises the properties-only (no Update) branch under test.
item.Delta(ItemDelta.Properties);
item.ProcessDelta();
}
[Fact]
public void PrivateChildInOwnerBackpack_PropertiesOnlyUpdate_ReachesOwnerNotBystander()
{
var wasEnabled = ObjectPropertyList.Enabled;
ObjectPropertyList.Enabled = true;
var (ownerNs, owner) = CreateClient(_ownerLoc);
var (bystanderNs, bystander) = CreateClient(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0));
var pack = new Container(0xE75) { Layer = Layer.Backpack };
owner.AddItem(pack);
var child = new Item(0x1234);
pack.DropItem(child);
try
{
DrainPendingDelta(child);
InvalidateAndFlush(child);
Assert.True(ReceivedOplInfo(ownerNs, child.Serial));
Assert.False(ReceivedOplInfo(bystanderNs, child.Serial));
}
finally
{
child.Delete();
DisposeClient(ownerNs, owner);
DisposeClient(bystanderNs, bystander);
ObjectPropertyList.Enabled = wasEnabled;
}
}
[Fact]
public void PrivateGroundContainer_PropertiesOnlyUpdate_ReachesOpenerNotNonOpener()
{
var wasEnabled = ObjectPropertyList.Enabled;
ObjectPropertyList.Enabled = true;
var (openerNs, opener) = CreateClient(_ownerLoc);
var (bystanderNs, bystander) = CreateClient(new Point3D(_ownerLoc.X + 2, _ownerLoc.Y, 0));
var container = new Container(0xE75);
container.MoveToWorld(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0), Map.Felucca);
container.Openers = [opener];
var child = new Item(0x1234);
container.DropItem(child);
try
{
DrainPendingDelta(child);
InvalidateAndFlush(child);
Assert.True(ReceivedOplInfo(openerNs, child.Serial));
Assert.False(ReceivedOplInfo(bystanderNs, child.Serial));
}
finally
{
container.Delete();
DisposeClient(openerNs, opener);
DisposeClient(bystanderNs, bystander);
ObjectPropertyList.Enabled = wasEnabled;
}
}
[Fact]
public void PublicContainerChild_PropertiesOnlyUpdate_StillReachesBystander()
{
var wasEnabled = ObjectPropertyList.Enabled;
ObjectPropertyList.Enabled = true;
var (bystanderNs, bystander) = CreateClient(_ownerLoc);
var pack = new PublicContainer();
pack.MoveToWorld(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0), Map.Felucca);
var child = new Item(0x1234);
pack.DropItem(child);
try
{
DrainPendingDelta(child);
InvalidateAndFlush(child);
Assert.True(ReceivedOplInfo(bystanderNs, child.Serial));
}
finally
{
pack.Delete();
DisposeClient(bystanderNs, bystander);
ObjectPropertyList.Enabled = wasEnabled;
}
}
[Fact]
public void GroundItem_PropertiesOnlyUpdate_StillReachesBystander()
{
var wasEnabled = ObjectPropertyList.Enabled;
ObjectPropertyList.Enabled = true;
var (bystanderNs, bystander) = CreateClient(_ownerLoc);
var item = new Item(0x1234);
item.MoveToWorld(new Point3D(_ownerLoc.X + 1, _ownerLoc.Y, 0), Map.Felucca);
try
{
DrainPendingDelta(item);
InvalidateAndFlush(item);
Assert.True(ReceivedOplInfo(bystanderNs, item.Serial));
}
finally
{
item.Delete();
DisposeClient(bystanderNs, bystander);
ObjectPropertyList.Enabled = wasEnabled;
}
}
}

View file

@ -1,3 +1,4 @@
using System.Buffers.Binary;
using Server.Items;
using Server.Network;
using Xunit;
@ -196,4 +197,135 @@ public class ContainerPacketTests
var result = ns.SendBuffer.GetReadSpan();
AssertThat.Equal(result, expected);
}
[Fact]
public void TestContainerContent_CapsAtProtocolLimit()
{
var cont = new Container(World.NewItem);
// Past the 3448-entry cap (19-byte entries) so truncation engages.
const int itemCount = 5000;
for (var i = 0; i < itemCount; i++)
{
cont.AddItem(new Item(World.NewItem));
}
cont.Map = Map.Felucca;
var m = new Mobile((Serial)0x1);
m.DefaultMobileInit();
m.AccessLevel = AccessLevel.Administrator;
m.Map = Map.Felucca;
try
{
using var ns = PacketTestUtilities.CreateTestNetState();
// A ~64KB packet exceeds the 4KB pre-auth send buffer; an account lets Send() promote it
// on demand instead of exhausting the buffer and disconnecting.
ns.Account = new MockAccount();
var ex = Record.Exception(() => ns.SendContainerContent(m, cont));
Assert.Null(ex);
var span = ns.SendBuffer.GetReadSpan();
Assert.Equal((byte)0x3C, span[0]);
var length = BinaryPrimitives.ReadUInt16BigEndian(span[1..3]);
var count = BinaryPrimitives.ReadUInt16BigEndian(span[3..5]);
const int entrySize = 19; // no grid lines negotiated on this NetState
var expectedMaxEntries = (ushort.MaxValue - 5) / entrySize;
Assert.Equal(65517, length);
Assert.Equal(length, span.Length);
Assert.Equal(expectedMaxEntries, count);
}
finally
{
cont.Delete();
m.Delete();
}
}
[Fact]
public void TestContainerContent_CapsAtProtocolLimit_WithGridLines()
{
var cont = new Container(World.NewItem);
// Past the 3276-entry cap (20-byte entries with grid lines) so truncation engages.
const int itemCount = 5000;
for (var i = 0; i < itemCount; i++)
{
cont.AddItem(new Item(World.NewItem));
}
cont.Map = Map.Felucca;
var m = new Mobile((Serial)0x1);
m.DefaultMobileInit();
m.AccessLevel = AccessLevel.Administrator;
m.Map = Map.Felucca;
try
{
using var ns = PacketTestUtilities.CreateTestNetState();
ns.ProtocolChanges |= ProtocolChanges.ContainerGridLines;
ns.Account = new MockAccount();
var ex = Record.Exception(() => ns.SendContainerContent(m, cont));
Assert.Null(ex);
var span = ns.SendBuffer.GetReadSpan();
Assert.Equal((byte)0x3C, span[0]);
var length = BinaryPrimitives.ReadUInt16BigEndian(span[1..3]);
var count = BinaryPrimitives.ReadUInt16BigEndian(span[3..5]);
Assert.Equal(65525, length);
Assert.Equal(length, span.Length);
Assert.Equal(3276, count);
}
finally
{
cont.Delete();
m.Delete();
}
}
[Fact]
public void TestContainerContent_RentedBuffer_MatchesReferencePacket()
{
var cont = new Container(World.NewItem);
// Below the entry cap but past the 1024-byte stackalloc threshold, so SendContainerContent
// rents its buffer instead of using the stack.
const int itemCount = 100;
for (var i = 0; i < itemCount; i++)
{
cont.AddItem(new Item(World.NewItem));
}
cont.Map = Map.Felucca;
var m = new Mobile((Serial)0x1);
m.DefaultMobileInit();
m.AccessLevel = AccessLevel.Administrator;
m.Map = Map.Felucca;
try
{
var expected = new ContainerContent(m, cont).Compile();
using var ns = PacketTestUtilities.CreateTestNetState();
ns.SendContainerContent(m, cont);
var result = ns.SendBuffer.GetReadSpan();
AssertThat.Equal(result, expected);
}
finally
{
cont.Delete();
m.Delete();
}
}
}