diff --git a/Projects/Server.Tests/Tests/Items/ContainerBulkRemovalTests.cs b/Projects/Server.Tests/Tests/Items/ContainerBulkRemovalTests.cs new file mode 100644 index 000000000..c529f8a12 --- /dev/null +++ b/Projects/Server.Tests/Tests/Items/ContainerBulkRemovalTests.cs @@ -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(); + } +} diff --git a/Projects/Server.Tests/Tests/Items/ContainerChildRemovalTests.cs b/Projects/Server.Tests/Tests/Items/ContainerChildRemovalTests.cs new file mode 100644 index 000000000..695838185 --- /dev/null +++ b/Projects/Server.Tests/Tests/Items/ContainerChildRemovalTests.cs @@ -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 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 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); + } + } +} diff --git a/Projects/Server.Tests/Tests/Items/ContainerChildSkipTests.cs b/Projects/Server.Tests/Tests/Items/ContainerChildSkipTests.cs new file mode 100644 index 000000000..5b1cd471c --- /dev/null +++ b/Projects/Server.Tests/Tests/Items/ContainerChildSkipTests.cs @@ -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(); + } + } +} diff --git a/Projects/Server.Tests/Tests/Items/LiftRejectResyncTests.cs b/Projects/Server.Tests/Tests/Items/LiftRejectResyncTests.cs new file mode 100644 index 000000000..97cffbba9 --- /dev/null +++ b/Projects/Server.Tests/Tests/Items/LiftRejectResyncTests.cs @@ -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 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 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 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(); + } + } +} diff --git a/Projects/Server.Tests/Tests/Items/PrivateChildPropertiesTests.cs b/Projects/Server.Tests/Tests/Items/PrivateChildPropertiesTests.cs new file mode 100644 index 000000000..4481b08b2 --- /dev/null +++ b/Projects/Server.Tests/Tests/Items/PrivateChildPropertiesTests.cs @@ -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 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; + } + } +} diff --git a/Projects/Server.Tests/Tests/Network/Packets/Outgoing/ContainerPacketTests.cs b/Projects/Server.Tests/Tests/Network/Packets/Outgoing/ContainerPacketTests.cs index e18b999f0..00dfc4054 100644 --- a/Projects/Server.Tests/Tests/Network/Packets/Outgoing/ContainerPacketTests.cs +++ b/Projects/Server.Tests/Tests/Network/Packets/Outgoing/ContainerPacketTests.cs @@ -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(); + } + } } diff --git a/Projects/Server/Items/Container.cs b/Projects/Server/Items/Container.cs index 18aa27be5..d2fb7b590 100644 --- a/Projects/Server/Items/Container.cs +++ b/Projects/Server/Items/Container.cs @@ -390,6 +390,21 @@ public partial class Container : Item } } + /// + /// Direct children are left out of world saves; the container itself is still saved and + /// loads empty. + /// + public virtual bool SkipsChildSerialization => false; + + /// + /// Whether was sent to every client in range rather than only to the root + /// mobile, the secure-trade partner and openers. Removal of a public child is broadcast; removal of any + /// other child reaches only the clients that were sent it. Overrides may only widen this beyond + /// , never narrow it: a public container's children always broadcast + /// their removal regardless of what an override returns here. + /// + public virtual bool IsChildPublic(Item child) => IsPublicContainer; + [MethodImpl(MethodImplOptions.AggressiveInlining)] public override void OnItemAdded(Item item) { diff --git a/Projects/Server/Items/Item.cs b/Projects/Server/Items/Item.cs index 03ff9d000..4226f100f 100644 --- a/Projects/Server/Items/Item.cs +++ b/Projects/Server/Items/Item.cs @@ -922,7 +922,10 @@ public partial class Item : IHued, IComparable, ISpawnable, IObjectPropert } var info = LookupCompactInfo(); - var items = LookupItems(); + // Skipping containers never write their child serials: those children are not saved as + // top-level records (see SkipSerialization), so a written serial can be handed to an + // unrelated new item during the next load's synchronous deserialize loop. + var items = this is Container { SkipsChildSerialization: true } ? EmptyItems : LookupItems(); if (m_Direction != Direction.North) { @@ -1185,6 +1188,11 @@ public partial class Item : IHued, IComparable, ISpawnable, IObjectPropert // are final yet. LastMoved = Core.Now; + // A parented item's removal already reached every client that was sent it (root, trade + // parties, openers) via RemoveItem below; sending another remove at oldLocation would + // broadcast it to bystanders who never knew about it. + var wasContained = Parent != null; + if (Parent is Mobile mobile) { mobile.RemoveItem(this); @@ -1202,7 +1210,7 @@ public partial class Item : IHued, IComparable, ISpawnable, IObjectPropert { m_Map.OnLeave(this); - if (oldLocation.m_X != 0) + if (!wasContained && oldLocation.m_X != 0) { SendRemovePacket(oldLocation); } @@ -1278,7 +1286,7 @@ public partial class Item : IHued, IComparable, ISpawnable, IObjectPropert } else if (m_Map != null) { - if (oldLocation.m_X != 0) + if (!wasContained && oldLocation.m_X != 0) { var removeEntity = stackalloc byte[OutgoingEntityPackets.RemoveEntityLength].InitializePacket(); @@ -1344,7 +1352,12 @@ public partial class Item : IHued, IComparable, ISpawnable, IObjectPropert m_Map.OnLeave(this); } - SendRemovePacket(); + if (m_Parent is not Container) + { + // A contained item only changes map along with its parent or root; their own + // removal already clears this subtree for every client that knew about it. + SendRemovePacket(); + } } var items = LookupItems(); @@ -1479,6 +1492,43 @@ public partial class Item : IHued, IComparable, ISpawnable, IObjectPropert return; } + // A properties-only invalidation (no Update) on a private child must stay as scoped as every + // other private-child send, otherwise a bystander who was never sent the item learns its OPL hash. + if (!update && ObjectPropertyList.Enabled && (flags & ItemDelta.Properties) != 0 && + TryGetPrivateParent(out var cont)) + { + GetPrivateChildRecipients(cont, out var root, out var tradeFrom, out var tradeTo); + + SendPrivateOplTo(root, worldLoc); + + if (tradeFrom != root) + { + SendPrivateOplTo(tradeFrom, worldLoc); + } + + if (tradeTo != root && tradeTo != tradeFrom) + { + SendPrivateOplTo(tradeTo, worldLoc); + } + + var privateOpeners = cont.Openers; + + if (privateOpeners != null) + { + for (var i = 0; i < privateOpeners.Count; ++i) + { + var mob = privateOpeners[i]; + + if (mob != root && mob != tradeFrom && mob != tradeTo) + { + SendPrivateOplTo(mob, worldLoc); + } + } + } + + return; + } + // A second item sharing an equipment layer is omitted by SendMobileIncoming (0x78); // sending it on its own via EquipUpdate/OPL would leave the client with two items on // one slot. Skip the per-client sends entirely for the dupe. @@ -1581,7 +1631,7 @@ public partial class Item : IHued, IComparable, ISpawnable, IObjectPropert ClearProperties(); } - public virtual bool SkipSerialization => false; + public virtual bool SkipSerialization => m_Parent is Container { SkipsChildSerialization: true }; [IgnoreDupe] public ISpawner Spawner @@ -2926,6 +2976,13 @@ public partial class Item : IHued, IComparable, ISpawnable, IObjectPropert m_Parent = null; } + // A skipping container never wrote its children's serials (see Serialize), so this + // item's frozen parent reference is stale/unreachable: treat it as a missing parent. + if (m_Parent is Container { SkipsChildSerialization: true }) + { + m_Parent = null; + } + if (m_Parent == null && (parent.IsMobile || parent.IsItem)) { Timer.DelayCall(Delete); @@ -3089,6 +3146,13 @@ public partial class Item : IHued, IComparable, ISpawnable, IObjectPropert m_Parent = null; } + // A skipping container never wrote its children's serials (see Serialize), so this + // item's frozen parent reference is stale/unreachable: treat it as a missing parent. + if (m_Parent is Container { SkipsChildSerialization: true }) + { + m_Parent = null; + } + if (m_Parent == null && (parent.IsMobile || parent.IsItem)) { Timer.DelayCall(Delete); @@ -3552,8 +3616,19 @@ public partial class Item : IHued, IComparable, ISpawnable, IObjectPropert { var items = LookupItems(); - if (items.Remove(item)) + // Bulk removal (Delete, purges) walks children back to front; finding the tail in O(1) keeps + // deleting a large container linear instead of quadratic. + var index = items.Count - 1; + + if (index < 0 || !ReferenceEquals(items[index], item)) { + index = items.IndexOf(item); + } + + if (index >= 0) + { + items.RemoveAt(index); + if (this is not Container) { AcquireCompactInfo().Version++; @@ -4018,6 +4093,43 @@ public partial class Item : IHued, IComparable, ISpawnable, IObjectPropert var removeEntity = stackalloc byte[OutgoingEntityPackets.RemoveEntityLength].InitializePacket(); + if (TryGetPrivateParent(out var cont)) + { + // Private children are only ever sent to these recipients (see ProcessDelta); nobody else knows them. + OutgoingEntityPackets.CreateRemoveEntity(removeEntity, Serial); + + GetPrivateChildRecipients(cont, out var root, out var tradeFrom, out var tradeTo); + + SendRemoveTo(root, worldLoc, removeEntity); + + if (tradeFrom != root) + { + SendRemoveTo(tradeFrom, worldLoc, removeEntity); + } + + if (tradeTo != root && tradeTo != tradeFrom) + { + SendRemoveTo(tradeTo, worldLoc, removeEntity); + } + + var openers = cont.Openers; + + if (openers != null) + { + for (var i = 0; i < openers.Count; ++i) + { + var mob = openers[i]; + + if (mob != root && mob != tradeFrom && mob != tradeTo) + { + SendRemoveTo(mob, worldLoc, removeEntity); + } + } + } + + return; + } + foreach (var state in m_Map.GetClientsInRange(worldLoc, GetMaxUpdateRange())) { var m = state.Mobile; @@ -4030,6 +4142,78 @@ public partial class Item : IHued, IComparable, ISpawnable, IObjectPropert } } + private void SendRemoveTo(Mobile m, Point3D worldLoc, ReadOnlySpan removeEntity) + { + if (m?.NetState != null && m.Map == m_Map && m.InRange(worldLoc, GetUpdateRange(m))) + { + m.NetState.Send(removeEntity); + } + } + + // Same recipient gate as SendRemoveTo, plus CanSee: this is an informational OPL push, not + // cleanup of a reference the client may already hold regardless of visibility. + private void SendPrivateOplTo(Mobile m, Point3D worldLoc) + { + var ns = m?.NetState; + + if (ns != null && m.CanSee(this) && m.Map == m_Map && m.InRange(worldLoc, GetUpdateRange(m))) + { + SendOPLPacketTo(ns); + } + } + + // Shared with IsSentTo so the private-child guard and recipient set (root/trade/openers) each live + // in one place. + private bool TryGetPrivateParent(out Container cont) + { + cont = m_Parent as Container; + return cont != null && !cont.IsPublicContainer && !cont.IsChildPublic(this); + } + + private void GetPrivateChildRecipients(Container cont, out Mobile root, out Mobile tradeFrom, out Mobile tradeTo) + { + root = cont.RootParent as Mobile; + + var trade = GetSecureTradeCont()?.Trade; + // Trade.From/To are unassigned while SecureTrade's own constructor is still adding the + // VirtualCheck to each side's container, so both must stay null-conditional. + tradeFrom = trade?.From?.Mobile; + tradeTo = trade?.To?.Mobile; + } + + /// + /// Whether 's client is expected to hold this item; gates resends keyed by a + /// client-supplied serial. + /// + public bool IsSentTo(Mobile m) + { + if (m == null || Deleted) + { + return false; + } + + if (TryGetPrivateParent(out var cont)) + { + GetPrivateChildRecipients(cont, out var root, out var tradeFrom, out var tradeTo); + + if (m == root) + { + // Mirrors ProcessDelta's own root gate: CanSee and in range. + return m.CanSee(this) && m.InRange(GetWorldLocation(), GetUpdateRange(m)); + } + + var isTradeOrOpener = m == tradeFrom || m == tradeTo || cont.Openers?.Contains(m) == true; + + // Trade parties and openers are re-validated against ProcessDelta's own gate (CanSee, + // current map, update range), so an invisible item or a stale Openers entry that hasn't + // been pruned yet never resyncs to them. + return isTradeOrOpener && m.CanSee(this) && m.Map == m_Map && + m.InRange(GetWorldLocation(), GetUpdateRange(m)); + } + + return m.CanSee(this) && m.InRange(GetWorldLocation(), GetUpdateRange(m)); + } + public virtual int GetDropSound() => -1; public Point3D GetWorldLocation() diff --git a/Projects/Server/Mobiles/Mobile.cs b/Projects/Server/Mobiles/Mobile.cs index d1db8113b..52628e74a 100644 --- a/Projects/Server/Mobiles/Mobile.cs +++ b/Projects/Server/Mobiles/Mobile.cs @@ -5203,7 +5203,8 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro { state.SendLiftReject(reject); - if (item.Deleted) + // A rejection must not resynchronize state this client was never sent (client-supplied serial). + if (item.Deleted || !item.IsSentTo(this)) { return; } diff --git a/Projects/Server/Network/Packets/OutgoingContainerPackets.cs b/Projects/Server/Network/Packets/OutgoingContainerPackets.cs index fdb71bf90..b1d846e7b 100644 --- a/Projects/Server/Network/Packets/OutgoingContainerPackets.cs +++ b/Projects/Server/Network/Packets/OutgoingContainerPackets.cs @@ -16,6 +16,7 @@ using System; using System.Buffers; using System.IO; +using Server.Buffers; using Server.Logging; namespace Server.Network; @@ -166,6 +167,10 @@ public static class OutgoingContainerPackets ns.Send(writer.Span); } + // 0x3C's length and item-count fields are both 16-bit, so a container's contents can never + // legally fill more than this many bytes regardless of how many items it actually holds. + private const int MaxContainerContentBytes = ushort.MaxValue; + public static void SendContainerContent(this NetState ns, Mobile beholder, Item beheld) { if (ns.CannotSendPackets()) @@ -175,43 +180,63 @@ public static class OutgoingContainerPackets var items = beheld.Items; var count = items.Count; + var entrySize = ns.ContainerGridLines ? 20 : 19; - var writer = new SpanWriter(stackalloc byte[5 + items.Count * (ns.ContainerGridLines ? 20 : 19)]); - writer.Write((byte)0x3C); // Packet ID - writer.Seek(4, SeekOrigin.Current); // Length & written count + var maxEntries = (MaxContainerContentBytes - 5) / entrySize; + var entriesToSend = Math.Min(count, maxEntries); + var length = 5 + entriesToSend * entrySize; - var written = 0; + // A container holding thousands of items would blow the stack; rent from the pool past a + // small, safe threshold instead. + byte[] rented = null; + var buffer = length <= 1024 ? stackalloc byte[length] : rented = STArrayPool.Shared.Rent(length); - for (var i = 0; i < count; ++i) + try { - var child = items[i]; + var writer = new SpanWriter(buffer[..length]); + writer.Write((byte)0x3C); // Packet ID + writer.Seek(4, SeekOrigin.Current); // Length & written count - if (!child.Deleted && beholder.CanSee(child)) + var written = 0; + + for (var i = 0; i < count && written < entriesToSend; ++i) { - var loc = child.Location; + var child = items[i]; - writer.Write(child.Serial); - writer.Write((ushort)child.ItemID); - writer.Write((byte)0); // signed, itemID offset - writer.Write((ushort)Math.Min(child.Amount, ushort.MaxValue)); - writer.Write((short)loc.X); - writer.Write((short)loc.Y); - if (ns.ContainerGridLines) + if (!child.Deleted && beholder.CanSee(child)) { - writer.Write((byte)0); // Grid Location? - } - writer.Write(beheld.Serial); - writer.Write((ushort)(child.QuestItem ? Item.QuestItemHue : child.Hue)); + var loc = child.Location; - ++written; + writer.Write(child.Serial); + writer.Write((ushort)child.ItemID); + writer.Write((byte)0); // signed, itemID offset + writer.Write((ushort)Math.Min(child.Amount, ushort.MaxValue)); + writer.Write((short)loc.X); + writer.Write((short)loc.Y); + if (ns.ContainerGridLines) + { + writer.Write((byte)0); // Grid Location? + } + writer.Write(beheld.Serial); + writer.Write((ushort)(child.QuestItem ? Item.QuestItemHue : child.Hue)); + + ++written; + } + } + + writer.Seek(1, SeekOrigin.Begin); + writer.Write((ushort)writer.BytesWritten); + writer.Write((ushort)written); + writer.Seek(0, SeekOrigin.End); + + ns.Send(writer.Span); + } + finally + { + if (rented != null) + { + STArrayPool.Shared.Return(rented); } } - - writer.Seek(1, SeekOrigin.Begin); - writer.Write((ushort)writer.BytesWritten); - writer.Write((ushort)written); - writer.Seek(0, SeekOrigin.End); - - ns.Send(writer.Span); } } diff --git a/Projects/UOContent.Tests/Tests/Items/Misc/Corpses/CorpseChildPublicTests.cs b/Projects/UOContent.Tests/Tests/Items/Misc/Corpses/CorpseChildPublicTests.cs new file mode 100644 index 000000000..01a58a697 --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Items/Misc/Corpses/CorpseChildPublicTests.cs @@ -0,0 +1,226 @@ +using System; +using System.Collections.Generic; +using Server; +using Server.Accounting; +using Server.Items; +using Server.Network; +using Server.Tests.Network; +using Xunit; + +namespace UOContent.Tests; + +[Collection("Sequential UOContent Tests")] +public class CorpseChildPublicTests +{ + // Weapons take their layer from tiledata, which CI doesn't load; without an explicit layer the + // equip is silently refused and the corpse's worn-gear list stays empty. + private static VikingSword EquipSword(Mobile owner) + { + var sword = new VikingSword { Layer = Layer.OneHanded }; + Assert.True(owner.EquipItem(sword)); + return sword; + } + + [Fact] + public void IsChildPublic_HumanCorpseWithWornGear_ReturnsTrue() + { + var owner = new Mobile((Serial)0x1); + owner.DefaultMobileInit(); + owner.Body = 0x190; + + var wornItem = EquipSword(owner); + + var corpse = new Corpse(owner, owner.Items); + + try + { + Assert.Equal(0x2006, corpse.ItemID); + Assert.True(((Body)corpse.Amount).IsHuman); + Assert.True(corpse.IsChildPublic(wornItem)); + } + finally + { + corpse.Delete(); + owner.Delete(); + } + } + + [Fact] + public void IsChildPublic_HumanCorpseWithLootItem_ReturnsFalse() + { + var owner = new Mobile((Serial)0x1); + owner.DefaultMobileInit(); + owner.Body = 0x190; + + var wornItem = EquipSword(owner); + + var corpse = new Corpse(owner, owner.Items); + + var lootItem = new Gold(100); + corpse.DropItem(lootItem); + + try + { + Assert.Equal(0x2006, corpse.ItemID); + Assert.True(((Body)corpse.Amount).IsHuman); + Assert.False(corpse.IsChildPublic(lootItem)); + } + finally + { + corpse.Delete(); + owner.Delete(); + } + } + + [Fact] + public void IsChildPublic_BoneCorpse_ReturnsFalse() + { + var owner = new Mobile((Serial)0x1); + owner.DefaultMobileInit(); + owner.Body = 0x190; + + var wornItem = EquipSword(owner); + + var corpse = new Corpse(owner, owner.Items); + + try + { + corpse.ItemID = 0xECA; + + Assert.True(((Body)corpse.Amount).IsHuman); + Assert.NotEqual(0x2006, corpse.ItemID); + Assert.False(corpse.IsChildPublic(wornItem)); + } + finally + { + corpse.Delete(); + owner.Delete(); + } + } + + [Fact] + public void IsChildPublic_NonHumanCorpse_ReturnsFalse() + { + var owner = new Mobile((Serial)0x1); + owner.DefaultMobileInit(); + owner.Body = 0xC9; + + var wornItem = EquipSword(owner); + + var corpse = new Corpse(owner, owner.Items); + + try + { + Assert.Equal(0x2006, corpse.ItemID); + Assert.False(((Body)corpse.Amount).IsHuman); + Assert.False(corpse.IsChildPublic(wornItem)); + } + finally + { + corpse.Delete(); + owner.Delete(); + } + } + + private static bool ReceivedRemove(NetState ns, Serial serial) + { + Span expected = stackalloc byte[OutgoingEntityPackets.RemoveEntityLength]; + OutgoingEntityPackets.CreateRemoveEntity(expected, serial); + return ns.SendBuffer.GetReadSpan().IndexOf(expected) >= 0; + } + + [Fact] + public void HumanCorpse_WornGearRemovalBroadcasts_LootRemovalDoesNot() + { + var corpseLoc = new Point3D(1500, 1500, 0); + + var bystanderNs = PacketTestUtilities.CreateTestNetState(); + bystanderNs.Account = new MockAccount(); + var bystander = new Mobile(World.NewMobile); + bystander.DefaultMobileInit(); + bystanderNs.Mobile = bystander; + bystander.NetState = bystanderNs; + bystander.MoveToWorld(new Point3D(corpseLoc.X + 1, corpseLoc.Y, corpseLoc.Z), Map.Felucca); + + var owner = new Mobile((Serial)0x1); + owner.DefaultMobileInit(); + owner.Body = 0x190; + + var sword = new VikingSword(); + var corpse = new Corpse(owner, [sword]); + corpse.AddItem(sword); + + var lootItem = new Gold(100); + corpse.DropItem(lootItem); + + corpse.MoveToWorld(corpseLoc, Map.Felucca); + + Container pouch = null; + + try + { + // A container not in the world is a valid destination; only the removal from the corpse matters here. + pouch = new Container(0xE75); + pouch.AddItem(sword); + + Assert.True(ReceivedRemove(bystanderNs, sword.Serial)); + + lootItem.Delete(); + Assert.False(ReceivedRemove(bystanderNs, lootItem.Serial)); + } + finally + { + pouch?.Delete(); + corpse.Delete(); + owner.Delete(); + bystanderNs.Mobile = null; + bystanderNs.Dispose(); + bystander.Delete(); + } + } + + [Fact] + public void HumanCorpse_LiftingWornGear_BroadcastsRemoval() + { + var corpseLoc = new Point3D(1500, 1500, 0); + + var bystanderNs = PacketTestUtilities.CreateTestNetState(); + bystanderNs.Account = new MockAccount(); + var bystander = new Mobile(World.NewMobile); + bystander.DefaultMobileInit(); + bystanderNs.Mobile = bystander; + bystander.NetState = bystanderNs; + bystander.MoveToWorld(new Point3D(corpseLoc.X + 1, corpseLoc.Y, corpseLoc.Z), Map.Felucca); + + var owner = new Mobile(World.NewMobile); + owner.DefaultMobileInit(); + owner.Body = 0x190; + owner.MoveToWorld(new Point3D(corpseLoc.X - 1, corpseLoc.Y, corpseLoc.Z), Map.Felucca); + + var sword = new VikingSword { Layer = Layer.OneHanded }; + var corpse = new Corpse(owner, [sword]); + corpse.AddItem(sword); + corpse.MoveToWorld(corpseLoc, Map.Felucca); + + try + { + // Lifting takes the piece off the worn list before the remove is sent; the remove must + // still reach everyone who was shown it worn. + owner.Lift(sword, sword.Amount, out var rejected, out _); + + Assert.False(rejected); + Assert.DoesNotContain(sword, corpse.EquipItems); + Assert.True(ReceivedRemove(bystanderNs, sword.Serial)); + } + finally + { + owner.Holding = null; + sword.Delete(); + corpse.Delete(); + owner.Delete(); + bystanderNs.Mobile = null; + bystanderNs.Dispose(); + bystander.Delete(); + } + } +} diff --git a/Projects/UOContent/Items/Misc/Corpses/Corpse.cs b/Projects/UOContent/Items/Misc/Corpses/Corpse.cs index 2cd0cea89..29a529ae5 100644 --- a/Projects/UOContent/Items/Misc/Corpses/Corpse.cs +++ b/Projects/UOContent/Items/Misc/Corpses/Corpse.cs @@ -100,6 +100,10 @@ public partial class Corpse : Container, ICarvable private Dictionary _instancedItems; + // Worn gear being lifted is off _equipItems before its remove goes out; until then it stays public so + // the remove reaches everyone who was shown it worn. + private Item _liftingWornItem; + [SerializableField(0)] private List _restoreEquip; @@ -197,6 +201,12 @@ public partial class Corpse : Container, ICarvable // Why was this public? // public override bool IsPublicContainer => true; + // Human corpses show their worn gear to everyone who sees them (SendInfoTo), not only to openers. + private bool ShowsWornGear => ((Body)Amount).IsHuman && ItemID == 0x2006; + + public override bool IsChildPublic(Item child) => + ShowsWornGear && (child == _liftingWornItem || _equipItems?.Contains(child) == true); + public Corpse(Mobile owner, List equipItems) : this(owner, 0, 0, 0, 0, equipItems) { } @@ -416,7 +426,7 @@ public partial class Corpse : Container, ICarvable { from.SendLocalizedMessage(500485); // You see nothing useful to carve from the corpse. } - else if (((Body)Amount).IsHuman && ItemID == 0x2006) + else if (ShowsWornGear) { new Blood(0x122D).MoveToWorld(Location, Map); @@ -587,6 +597,7 @@ public partial class Corpse : Container, ICarvable { _decayTimer?.Stop(); _decayTimer = null; + _liftingWornItem = null; } public static string GetCorpseName(Mobile m) => m is BaseCreature bc ? bc.CorpseNameOverride ?? bc.CorpseName : null; @@ -740,7 +751,7 @@ public partial class Corpse : Container, ICarvable { base.SendInfoTo(ns, world); - if (((Body)Amount).IsHuman && ItemID == 0x2006) + if (ShowsWornGear) { ns.SendCorpseContent(ns.Mobile, this); ns.SendCorpseEquip(ns.Mobile, this); @@ -815,7 +826,20 @@ public partial class Corpse : Container, ICarvable // Lifted gear is loot from then on. Clients only learn worn gear from 0x89 on first sight, so a // piece put back would show worn after a relog but loose to everyone already watching. - RemoveFromEquipItems(item); + if (_equipItems != null && this.Remove(_equipItems, item)) + { + _liftingWornItem = item; + } + } + + public override void OnItemRemoved(Item item) + { + base.OnItemRemoved(item); + + if (item == _liftingWornItem) + { + _liftingWornItem = null; + } } public override void GetContextMenuEntries(Mobile from, ref PooledRefList list)