diff --git a/Directory.Packages.props b/Directory.Packages.props index f718476a5..2e3b14f1e 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -43,7 +43,7 @@ - + diff --git a/src/NosCore.GameObject/Services/ExchangeService/ExchangeService.cs b/src/NosCore.GameObject/Services/ExchangeService/ExchangeService.cs index 7aceeb252..ff2a08691 100644 --- a/src/NosCore.GameObject/Services/ExchangeService/ExchangeService.cs +++ b/src/NosCore.GameObject/Services/ExchangeService/ExchangeService.cs @@ -333,7 +333,7 @@ bool IsFullTransfer items.Add(new KeyValuePair(transfer.SessionId, (sourceItem ?? transfer.OriginalItem).GeneratePocketChange((PocketType)transfer.OriginalItem.Type, transfer.OriginalItem.Slot))); items.Add(new KeyValuePair(transfer.TargetId, - transfer.OriginalItem.GeneratePocketChange((PocketType)addedItem.Item.Type, addedItem.Item.Slot))); + addedItem.Item.GeneratePocketChange((PocketType)addedItem.Item.Type, addedItem.Item.Slot))); } return items; diff --git a/src/NosCore.PacketHandlers/Exchange/ExcListPacketHandler.cs b/src/NosCore.PacketHandlers/Exchange/ExcListPacketHandler.cs index 881bb64e6..abbd0b57b 100644 --- a/src/NosCore.PacketHandlers/Exchange/ExcListPacketHandler.cs +++ b/src/NosCore.PacketHandlers/Exchange/ExcListPacketHandler.cs @@ -39,7 +39,7 @@ public override async Task ExecuteAsync(ExcListPacket packet, ClientSession clie (s.VisualId == exchangeService.GetTargetId(clientSession.Character.VisualId)) && (s.MapInstanceId == clientSession.Character.MapInstanceId), out var target); - if ((packet.SubPackets!.Count > 0) && hasTarget) + if ((packet.SubPackets?.Count > 0) && hasTarget) { byte i = 0; foreach (var value in packet.SubPackets) diff --git a/src/NosCore.PacketHandlers/Exchange/ExchangeRequestPacketHandler.cs b/src/NosCore.PacketHandlers/Exchange/ExchangeRequestPacketHandler.cs index d57d0198f..7225188c5 100644 --- a/src/NosCore.PacketHandlers/Exchange/ExchangeRequestPacketHandler.cs +++ b/src/NosCore.PacketHandlers/Exchange/ExchangeRequestPacketHandler.cs @@ -67,7 +67,7 @@ await clientSession.SendPacketAsync(new Infoi2Packet return; } - if (!hasTarget || target.ExchangeBlocked) + if (!hasTarget || target.ExchangeBlocked || (target.VisualId == clientSession.Character.VisualId)) { await clientSession.SendPacketAsync(new Infoi2Packet { @@ -184,7 +184,7 @@ await clientSession.SendPacketAsync(new InfoiPacket var success = exchangeService.ValidateExchange(clientSession, exchangeTarget); - if (success.Item1 == ExchangeResultType.Success) + if (success.Item1 == ExchangeResultType.Failure) { foreach (var infoPacket in success.Item2!) { diff --git a/test/NosCore.GameObject.Tests/Services/ExchangeService/ExchangeServiceTests.cs b/test/NosCore.GameObject.Tests/Services/ExchangeService/ExchangeServiceTests.cs index b8ecb7d69..676648738 100644 --- a/test/NosCore.GameObject.Tests/Services/ExchangeService/ExchangeServiceTests.cs +++ b/test/NosCore.GameObject.Tests/Services/ExchangeService/ExchangeServiceTests.cs @@ -19,6 +19,7 @@ using NosCore.Packets.ClientPackets.Inventory; using NosCore.Packets.Enumerations; using NosCore.Packets.Interfaces; +using NosCore.Packets.ServerPackets.Inventory; using NosCore.Packets.ServerPackets.UI; using NosCore.Tests.Shared; using Microsoft.Extensions.Logging.Abstractions; @@ -359,5 +360,31 @@ public void AnOfferedItemThatLeftTheInventoryDoesNotReachTheOtherSide() Assert.AreEqual(0, itemList.Count, "a vanished offer must not transfer"); Assert.AreEqual(0, receiver.Count, "the receiver must not gain a duplicate"); } + + [TestMethod] + public void ProcessExchangeNotifiesReceiverWithTheirActualMergedAmount() + { + IInventoryService giver = + new GameObject.Services.InventoryService.InventoryService(new List { new Item { VNum = 1012, Type = NoscorePocketType.Main } }, + WorldConfiguration!, NullLogger.Instance); + IInventoryService receiver = + new GameObject.Services.InventoryService.InventoryService(new List { new Item { VNum = 1012, Type = NoscorePocketType.Main } }, + WorldConfiguration!, NullLogger.Instance); + + var offered = giver.AddItemToPocket(InventoryItemInstance.Create(ItemProvider!.Create(1012, 5), 0))!.First(); + receiver.AddItemToPocket(InventoryItemInstance.Create(ItemProvider.Create(1012, 4), 0)); + + ExchangeProvider!.OpenExchange(1, 2); + // Offering less than the full stack (2 of 5) forces the partial-transfer branch, + // which mutates the giver's own item object in place - this used to leak into the + // receiver's notification packet. + ExchangeProvider.AddItems(1, offered, 2); + + var itemList = ExchangeProvider.ProcessExchange(1, 2, giver, receiver); + + var receiverPacket = itemList.First(s => s.Key == 2).Value; + var receiverAmount = receiverPacket.IvnSubPackets!.First()!.RareAmount; + Assert.AreEqual((short)6, receiverAmount, "the receiver must be notified of their true merged total (4 + 2), not the giver's remaining balance"); + } } } diff --git a/test/NosCore.PacketHandlers.Tests/Exchange/ExcListPacketHandlerTests.cs b/test/NosCore.PacketHandlers.Tests/Exchange/ExcListPacketHandlerTests.cs index bf436eae1..b4582f9ae 100644 --- a/test/NosCore.PacketHandlers.Tests/Exchange/ExcListPacketHandlerTests.cs +++ b/test/NosCore.PacketHandlers.Tests/Exchange/ExcListPacketHandlerTests.cs @@ -104,6 +104,28 @@ public async Task SettingValidGoldShouldUpdateExchange() .ExecuteAsync(); } + [TestMethod] + public async Task GoldOnlyOfferShouldNotCloseExchange() + { + await new Spec("Gold only offer should not close exchange") + .Given(CharacterHasGold) + .And(ExchangeIsOpenWithTarget) + .WhenAsync(OfferingOnlyGold) + .Then(ExchangeShouldNotBeClosed) + .ExecuteAsync(); + } + + [TestMethod] + public async Task GoldOnlyOfferShouldSetGold() + { + await new Spec("Gold only offer should set gold") + .Given(CharacterHasGold) + .And(ExchangeIsOpenWithTarget) + .WhenAsync(OfferingOnlyGold) + .Then(ExchangeGoldShouldBeSet) + .ExecuteAsync(); + } + private void CharacterHasNoGold() { Session.Character.Gold = 0; @@ -179,11 +201,26 @@ await Handler.ExecuteAsync(new ExcListPacket }, Session); } + private async Task OfferingOnlyGold() + { + await Handler.ExecuteAsync(new ExcListPacket + { + Gold = 5000, + BankGold = 0, + SubPackets = null + }, Session); + } + private void ExchangeGoldShouldNotBeSet() { ExchangeService.Verify(x => x.SetGold(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); } + private void ExchangeShouldNotBeClosed() + { + ExchangeService.Verify(x => x.CloseExchange(It.IsAny(), It.IsAny()), Times.Never); + } + private void ExchangeGoldShouldBeSet() { var characterId = Session.Character.CharacterId; diff --git a/test/NosCore.PacketHandlers.Tests/Exchange/ExchangeRequestPacketHandlerTests.cs b/test/NosCore.PacketHandlers.Tests/Exchange/ExchangeRequestPacketHandlerTests.cs index b749483af..8f80a00c1 100644 --- a/test/NosCore.PacketHandlers.Tests/Exchange/ExchangeRequestPacketHandlerTests.cs +++ b/test/NosCore.PacketHandlers.Tests/Exchange/ExchangeRequestPacketHandlerTests.cs @@ -86,6 +86,26 @@ public async Task RequestingExchangeWhenTargetBlocksExchangeShouldFail() .ExecuteAsync(); } + [TestMethod] + public async Task RequestingExchangeWithSelfShouldFail() + { + await new Spec("Requesting exchange with self should fail") + .Given(TargetIsSelf) + .WhenAsync(RequestingExchangeWithSelf) + .Then(ShouldReceiveBlockingTradesMessage) + .ExecuteAsync(); + } + + [TestMethod] + public async Task RequestingExchangeWithSelfShouldNotOpenExchange() + { + await new Spec("Requesting exchange with self should not open an exchange") + .Given(TargetIsSelf) + .WhenAsync(RequestingExchangeWithSelf) + .Then(NoExchangeShouldBeOpened) + .ExecuteAsync(); + } + [TestMethod] public async Task RequestingExchangeWithBlacklistedPlayerShouldFail() { @@ -128,6 +148,12 @@ private void TargetBlocksExchange() ExchangeService.Setup(x => x.CheckExchange(It.IsAny())).Returns(false); } + private void TargetIsSelf() + { + Session.Character.MapInstance = TestHelpers.Instance.MapInstanceAccessorService.GetBaseMapById(1)!; + ExchangeService.Setup(x => x.CheckExchange(It.IsAny())).Returns(false); + } + private void TargetIsBlacklisted() { Session.Character.MapInstance = TestHelpers.Instance.MapInstanceAccessorService.GetBaseMapById(1)!; @@ -161,6 +187,15 @@ await Handler.ExecuteAsync(new ExchangeRequestPacket }, Session); } + private async Task RequestingExchangeWithSelf() + { + await Handler.ExecuteAsync(new ExchangeRequestPacket + { + RequestType = RequestExchangeType.Requested, + VisualId = Session.Character.VisualId + }, Session); + } + private async Task CancellingExchange() { await Handler.ExecuteAsync(new ExchangeRequestPacket