diff --git a/src/NosCore.GameObject/Services/ExchangeService/ExchangeService.cs b/src/NosCore.GameObject/Services/ExchangeService/ExchangeService.cs index 9563cff4b..7aceeb252 100644 --- a/src/NosCore.GameObject/Services/ExchangeService/ExchangeService.cs +++ b/src/NosCore.GameObject/Services/ExchangeService/ExchangeService.cs @@ -11,6 +11,7 @@ using NosCore.GameObject.Ecs.Extensions; using NosCore.GameObject.Ecs.Interfaces; using NosCore.GameObject.Networking.ClientSession; +using NosCore.GameObject.Services.BroadcastService; using NosCore.GameObject.Services.InventoryService; using NosCore.GameObject.Services.ItemGenerationService; using NosCore.Packets.Enumerations; @@ -28,7 +29,8 @@ namespace NosCore.GameObject.Services.ExchangeService { public class ExchangeService(IItemGenerationService itemBuilderService, IOptions worldConfiguration, ILogger logger, IExchangeRequestRegistry exchangeRegistry, - ILogLanguageLocalizer logLanguage, IGameLanguageLocalizer gameLanguageLocalizer) + ILogLanguageLocalizer logLanguage, IGameLanguageLocalizer gameLanguageLocalizer, + ISessionRegistry sessionRegistry) : IExchangeService { public void SetGold(long visualId, long gold, long bankGold) @@ -204,12 +206,25 @@ public bool CheckExchange(long visualId, long targetId) logger.LogError(logLanguage[LogLanguageKey.TRY_REMOVE_FAILED], data.Value); } + SetInExchange(data.Key, false); + SetInExchange(data.Value, false); + return new ExcClosePacket { Type = resultType }; } + // The lifecycle owns the flag. Set at the seven call sites instead, it was simply + // never set at all, which left every InExchangeOrShop guard inert. + private void SetInExchange(long visualId, bool value) + { + if (sessionRegistry.TryGetCharacter(c => c.VisualId == visualId, out var character)) + { + character.InExchange = value; + } + } + public bool OpenExchange(long visualId, long targetVisualId) { if (CheckExchange(visualId) || CheckExchange(targetVisualId)) @@ -222,6 +237,8 @@ public bool OpenExchange(long visualId, long targetVisualId) exchangeRegistry.SetExchangeRequest(targetVisualId, visualId); exchangeRegistry.SetExchangeData(visualId, new ExchangeData()); exchangeRegistry.SetExchangeData(targetVisualId, new ExchangeData()); + SetInExchange(visualId, true); + SetInExchange(targetVisualId, true); return true; } @@ -257,6 +274,21 @@ bool IsFullTransfer } } + // The offer captured these references when the item was put in the window. Confirm + // each is still there, at that amount, before anything is created: the destination + // item is built fresh, so a source that has since gone would duplicate it. + foreach (var transfer in pendingTransfers) + { + if (!transfer.OriginInventory.TryGetValue(transfer.OriginalItem.ItemInstanceId, out var live) + || live.ItemInstance == null + || live.ItemInstance.ItemVNum != transfer.OriginalItem.ItemInstance.ItemVNum + || live.ItemInstance.Amount < transfer.Amount) + { + logger.LogError(logLanguage[LogLanguageKey.INVALID_EXCHANGE]); + return new List>(); + } + } + var addedItems = new List<(IInventoryService Inventory, InventoryItemInstance Item)>(); foreach (var transfer in pendingTransfers) diff --git a/test/NosCore.GameObject.Tests/Services/ExchangeService/ExchangeServiceTests.cs b/test/NosCore.GameObject.Tests/Services/ExchangeService/ExchangeServiceTests.cs index 745994c9b..b8ecb7d69 100644 --- a/test/NosCore.GameObject.Tests/Services/ExchangeService/ExchangeServiceTests.cs +++ b/test/NosCore.GameObject.Tests/Services/ExchangeService/ExchangeServiceTests.cs @@ -56,7 +56,8 @@ public void Setup() }; ItemProvider = new GameObject.Services.ItemGenerationService.ItemGenerationService(items, NullLoggerFactory.Instance, TestHelpers.Instance.LogLanguageLocalizer); - ExchangeProvider = new GameObject.Services.ExchangeService.ExchangeService(ItemProvider, WorldConfiguration, NullLogger.Instance, new ExchangeRequestRegistry(), TestHelpers.Instance.LogLanguageLocalizer, TestHelpers.Instance.GameLanguageLocalizer); + ExchangeProvider = new GameObject.Services.ExchangeService.ExchangeService(ItemProvider, WorldConfiguration, NullLogger.Instance, new ExchangeRequestRegistry(), TestHelpers.Instance.LogLanguageLocalizer, TestHelpers.Instance.GameLanguageLocalizer, + TestHelpers.Instance.SessionRegistry); } [TestMethod] @@ -183,7 +184,8 @@ private async Task SessionAndTargetAreSetUp() _sessionB = await TestHelpers.Instance.GenerateSessionAsync(); _realExchange = new GameObject.Services.ExchangeService.ExchangeService( ItemProvider!, WorldConfiguration!, NullLogger.Instance, new ExchangeRequestRegistry(), - TestHelpers.Instance.LogLanguageLocalizer, TestHelpers.Instance.GameLanguageLocalizer); + TestHelpers.Instance.LogLanguageLocalizer, TestHelpers.Instance.GameLanguageLocalizer, + TestHelpers.Instance.SessionRegistry); _realExchange.OpenExchange(_sessionA.Character.CharacterId, _sessionB.Character.VisualId); } @@ -332,5 +334,30 @@ private void ProcessExchangeShouldReturnItemsForBoth() var itemList = ExchangeProvider.ProcessExchange(1, 2, inventory1, inventory2); Assert.IsTrue((itemList.Count(s => s.Key == 1) == 2) && (itemList.Count(s => s.Key == 2) == 2)); } + + [TestMethod] + public void AnOfferedItemThatLeftTheInventoryDoesNotReachTheOtherSide() + { + 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, 1), 0))!.First(); + + ExchangeProvider!.OpenExchange(1, 2); + ExchangeProvider.AddItems(1, offered, 1); + + // The destination item is built fresh rather than moved, so dropping the offered + // item after putting it in the window used to hand over a second copy of it. + giver.Remove(offered.ItemInstanceId); + + var itemList = ExchangeProvider.ProcessExchange(1, 2, giver, receiver); + + Assert.AreEqual(0, itemList.Count, "a vanished offer must not transfer"); + Assert.AreEqual(0, receiver.Count, "the receiver must not gain a duplicate"); + } } }