From 3b7c8dbdb57bb665053cf26d13b655eb9fed831c Mon Sep 17 00:00:00 2001 From: Chomp Date: Wed, 26 Feb 2025 16:45:04 +0000 Subject: [PATCH] Fixed ragfair offers not expiring + some perf improvements todo: fix "offer not found" Replace _offersByTraderLock with hashset + manual locks --- Libraries/Core/Servers/RagfairServer.cs | 17 +- .../Core/Services/RagfairOfferService.cs | 86 +++------- Libraries/Core/Utils/RagfairOfferHolder.cs | 148 ++++++++++++++---- 3 files changed, 151 insertions(+), 100 deletions(-) diff --git a/Libraries/Core/Servers/RagfairServer.cs b/Libraries/Core/Servers/RagfairServer.cs index 2269951d..c15f1904 100644 --- a/Libraries/Core/Servers/RagfairServer.cs +++ b/Libraries/Core/Servers/RagfairServer.cs @@ -4,6 +4,7 @@ using Core.Models.Enums; using Core.Models.Spt.Config; using Core.Models.Utils; using Core.Services; +using Core.Utils; using SptCommon.Annotations; namespace Core.Servers; @@ -11,11 +12,13 @@ namespace Core.Servers; [Injectable] public class RagfairServer( ISptLogger _logger, + TimeUtil timeUtil, RagfairOfferService _ragfairOfferService, RagfairCategoriesService _ragfairCategoriesService, RagfairRequiredItemsService _ragfairRequiredItemsService, LocalisationService _localisationService, RagfairOfferGenerator _ragfairOfferGenerator, + RagfairOfferHolder _ragfairOfferHolder, ConfigServer _configServer ) { @@ -47,16 +50,16 @@ public class RagfairServer( } // Regenerate expired offers when over threshold limit - if (_ragfairOfferService.GetExpiredOfferCount() >= _ragfairConfig.Dynamic.ExpiredOfferThreshold) + _ragfairOfferHolder.FlagExpiredOffersAfterDate(timeUtil.GetTimeStamp()); + if (_ragfairOfferService.EnoughExpiredOffersExistToProcess()) { - // Must occur BEFORE "ExpireStaleOffers" - var expiredAssortsWithChildren = _ragfairOfferService.GetExpiredOfferAssorts(); + // Must occur BEFORE "RemoveExpiredOffers" + var expiredAssortsWithChildren = _ragfairOfferHolder.GetExpiredOfferItems(); + + // Replace the expired offers with new ones _ragfairOfferGenerator.GenerateDynamicOffers(expiredAssortsWithChildren); - _ragfairOfferService.ExpireStaleOffers(); - - // Clear out expired offers now we've regenerated them - _ragfairOfferService.ResetExpiredOfferIds(); + _ragfairOfferService.RemoveExpiredOffers(); } _ragfairRequiredItemsService.BuildRequiredItemTable(); diff --git a/Libraries/Core/Services/RagfairOfferService.cs b/Libraries/Core/Services/RagfairOfferService.cs index f8f64cf5..dcf61482 100644 --- a/Libraries/Core/Services/RagfairOfferService.cs +++ b/Libraries/Core/Services/RagfairOfferService.cs @@ -1,7 +1,7 @@ -using System.Collections.Concurrent; using Core.Helpers; using Core.Models.Eft.Common.Tables; using Core.Models.Eft.Ragfair; +using Core.Models.Spt.Config; using Core.Models.Utils; using Core.Servers; using Core.Utils; @@ -23,11 +23,12 @@ public class RagfairOfferService( ProfileHelper profileHelper, LocalisationService localisationService, ICloner cloner, - RagfairOfferHolder ragfairOfferHolder + RagfairOfferHolder ragfairOfferHolder, + ConfigServer configServer ) { - protected ConcurrentBag _expiredOfferIds = new(); protected bool _playerOffersLoaded; + protected RagfairConfig _ragfairConfig = configServer.GetConfig(); /** * Get all offers @@ -53,57 +54,6 @@ public class RagfairOfferService( ragfairOfferHolder.AddOffer(offer); } - /// - /// Add a stale offers id to collection for later use - /// - /// Id of offer to add to stale collection - public void AddOfferIdToExpired(string staleOfferId) - { - _expiredOfferIds.Add(staleOfferId); - } - - /** - * Get total count of current expired offers - * @returns Number of expired offers - */ - public int GetExpiredOfferCount() - { - return _expiredOfferIds.Count; - } - - /** - * Get an array of arrays of expired offer items + children - * @returns Expired offer assorts - */ - public List> GetExpiredOfferAssorts() - { - // list of lists of item+children - var expiredItems = new List>(); - - foreach (var expiredOfferId in _expiredOfferIds) - { - var offer = ragfairOfferHolder.GetOfferById(expiredOfferId); - if (offer?.Items?.Count == 0) - { - logger.Error($"Unable to process expired offer: {expiredOfferId}, it has no items"); - - continue; - } - - expiredItems.Add(offer.Items); - } - - return expiredItems; - } - - /** - * Clear out internal expiredOffers dictionary of all items - */ - public void ResetExpiredOfferIds() - { - _expiredOfferIds.Clear(); - } - /** * Does the offer exist on the ragfair * @param offerId offer id to check for @@ -139,8 +89,8 @@ public class RagfairOfferService( offer.Quantity -= amount; if (offer.Quantity <= 0) { - // Reducing Quantity has made it 0 or below, offer is now 'stale' - ProcessStaleOffer(offer); + // Reducing Quantity has made it 0 or below, offer is now 'stale' and needs to be flagged as expired so it can be removed/regenerated on the next ragfair update() + ragfairOfferHolder.FlagOfferAsExpired(offer.Id); } } @@ -192,15 +142,18 @@ public class RagfairOfferService( _playerOffersLoaded = true; } - public void ExpireStaleOffers() + /// + /// Process the expired ids and remove offers + /// + public void RemoveExpiredOffers() { - var time = timeUtil.GetTimeStamp(); - foreach (var staleOffer in ragfairOfferHolder.GetStaleOffers(time)) - { - ProcessStaleOffer(staleOffer); - } + ragfairOfferHolder.RemoveExpiredOffers(); + + // Clear out expired offer ids now we've regenerated them + ragfairOfferHolder.ResetExpiredOfferIds(); } + /** * Remove stale offer from flea * @param staleOffer Stale offer to process @@ -221,9 +174,9 @@ public class RagfairOfferService( // Handle dynamic offer if (!(isTrader || isPlayer)) - // Dynamic offer { - AddOfferIdToExpired(staleOfferId); + // Not trader/player offer + ragfairOfferHolder.FlagOfferAsExpired(staleOfferId); } // Handle player offer - items need returning/XP adjusting. Checking if offer has actually expired or not. @@ -348,4 +301,9 @@ public class RagfairOfferService( return result; } + + public bool EnoughExpiredOffersExistToProcess() + { + return ragfairOfferHolder.GetExpiredOfferCount() >= _ragfairConfig.Dynamic.ExpiredOfferThreshold; + } } diff --git a/Libraries/Core/Utils/RagfairOfferHolder.cs b/Libraries/Core/Utils/RagfairOfferHolder.cs index 107198d6..9f2bbd46 100644 --- a/Libraries/Core/Utils/RagfairOfferHolder.cs +++ b/Libraries/Core/Utils/RagfairOfferHolder.cs @@ -1,4 +1,6 @@ +using System.Collections.Concurrent; using Core.Helpers; +using Core.Models.Eft.Common.Tables; using Core.Models.Eft.Ragfair; using Core.Models.Spt.Config; using Core.Models.Utils; @@ -10,14 +12,14 @@ namespace Core.Utils; [Injectable(InjectionType.Singleton)] public class RagfairOfferHolder( - ISptLogger _logger, + ISptLogger logger, RagfairServerHelper ragfairServerHelper, ProfileHelper profileHelper, HashUtil hashUtil, - LocalisationService _localisationService, + LocalisationService localisationService, ConfigServer configServer) { - protected int _maxOffersPerTemplate = (int) configServer.GetConfig().Dynamic.OfferItemCount.Max; + protected int _maxOffersPerTemplate = configServer.GetConfig().Dynamic.OfferItemCount.Max; protected Dictionary _offersById = new(); protected object _offersByIdLock = new(); protected Dictionary> _offersByTemplate = new(); // key = tplId, value = list of offerIds @@ -25,6 +27,9 @@ public class RagfairOfferHolder( protected Dictionary> _offersByTrader = new(); // key = traderId, value = list of offerIds protected object _offersByTraderLock = new(); + // Use dict for the fast key lookup, value is not used + protected ConcurrentDictionary _expiredOfferIds = []; + public RagfairOffer? GetOfferById(string id) { lock (_offersByIdLock) @@ -103,8 +108,9 @@ public class RagfairOfferHolder( var itemTpl = offer.Items?.FirstOrDefault()?.Template; // If it is an NPC PMC offer AND we have already reached the maximum amount of possible offers // for this template, just don't add in more + var sellerIsTrader = ragfairServerHelper.IsTrader(sellerId); if (itemTpl != null && - !(ragfairServerHelper.IsTrader(sellerId) || profileHelper.IsPlayer(sellerId)) && + !(sellerIsTrader || profileHelper.IsPlayer(sellerId)) && _offersByTemplate.TryGetValue(itemTpl, out var offers) && offers?.Count >= _maxOffersPerTemplate ) @@ -114,8 +120,12 @@ public class RagfairOfferHolder( _offersById.Add(offerId, offer); - AddOfferByTrader(sellerId, offer); - AddOfferByTemplates(itemTpl, offer); + if (sellerIsTrader) + { + AddOfferByTrader(sellerId, offer); + } + + AddOfferByTemplates(sellerId, offer); } } @@ -123,29 +133,33 @@ public class RagfairOfferHolder( * Purge offer from offer cache * @param offer Offer to remove */ - public void RemoveOffer(string offerId) + public void RemoveOffer(string offerId, bool checkTraderOffers = true) { lock (_offersByIdLock) { if (!_offersById.TryGetValue(offerId, out var offer)) { - _logger.Warning(_localisationService.GetText("ragfair-unable_to_remove_offer_doesnt_exist", offerId)); + logger.Warning(localisationService.GetText("ragfair-unable_to_remove_offer_doesnt_exist", offerId)); return; } _offersById.Remove(offer.Id); - lock (_offersByTraderLock) + + if (checkTraderOffers) { - if (_offersByTrader.ContainsKey(offer.User.Id)) + lock (_offersByTraderLock) { - _offersByTrader[offer.User.Id].Remove(offer.Id); - // This was causing a memory leak, we need to make sure that we remove - // the user ID from the cached offers after they dont have anything else - // on the flea placed. We regenerate the ID for the NPC users, making it - // continuously grow otherwise - if (_offersByTrader[offer.User.Id].Count == 0) + if (_offersByTrader.ContainsKey(offer.User.Id)) { - _offersByTrader.Remove(offer.User.Id); + _offersByTrader[offer.User.Id].Remove(offer.Id); + // This was causing a memory leak, we need to make sure that we remove + // the user ID from the cached offers after they dont have anything else + // on the flea placed. We regenerate the ID for the NPC users, making it + // continuously grow otherwise + if (_offersByTrader[offer.User.Id].Count == 0) + { + _offersByTrader.Remove(offer.User.Id); + } } } } @@ -165,9 +179,8 @@ public class RagfairOfferHolder( { lock (_offersByTraderLock) { - if (_offersByTrader.ContainsKey(traderId)) + if (_offersByTrader.TryGetValue(traderId, out var offerIdsToRemove)) { - var offerIdsToRemove = _offersByTrader[traderId]; foreach (var offerId in offerIdsToRemove) { _offersById.Remove(offerId); @@ -179,15 +192,6 @@ public class RagfairOfferHolder( } } - /** - * Get an array of stale offers that are still shown to player - * @returns RagfairOffer array - */ - public IEnumerable GetStaleOffers(long time) - { - return GetOffers().Where(o => IsStale(o, time)); - } - protected void AddOfferByTemplates(string template, RagfairOffer offer) { lock (_offersByTemplateLock) @@ -225,6 +229,92 @@ public class RagfairOfferHolder( return false; } - return offer.EndTime < time || (offer.Items.FirstOrDefault().Upd?.StackObjectsCount ?? 0) < 1; + return offer.EndTime < time || (offer.Quantity ?? 0) < 1; + } + + /// + /// Add a stale offers id to collection for later use + /// + /// Id of offer to add to stale collection + public void FlagOfferAsExpired(string staleOfferId) + { + _expiredOfferIds.TryAdd(staleOfferId, null); + } + + /// + /// Get total count of current expired offers + /// + /// Number of expired offers + public int GetExpiredOfferCount() + { + return _expiredOfferIds.Count; + } + + /// + /// Get an array of arrays of expired offer items + children + /// + /// Expired offer assorts + public List> GetExpiredOfferItems() + { + // list of lists of item+children + var expiredItems = new List>(); + + foreach (var (expiredOfferId, _) in _expiredOfferIds) + { + var offer = GetOfferById(expiredOfferId); + if (offer is null) + { + logger.Warning($"offerId: {expiredOfferId} was not found !!"); + continue; + } + if (offer?.Items?.Count == 0) + { + logger.Error($"Unable to process expired offer: {expiredOfferId}, it has no items"); + + continue; + } + + expiredItems.Add(offer.Items); + } + + return expiredItems; + } + + /** + * Clear out internal expiredOffers dictionary of all items + */ + public void ResetExpiredOfferIds() + { + _expiredOfferIds.Clear(); + } + + /// + /// Flag offers with an expiry before the passed in timestamp + /// + /// + public void FlagExpiredOffersAfterDate(long timestamp) + { + foreach (var offer in GetOffers()) + { + if (_expiredOfferIds.ContainsKey(offer.Id) || ragfairServerHelper.IsTrader(offer.User.Id)) + { + // Already flagged or trader offer (handled separately), skip + continue; + } + + if (IsStale(offer, timestamp)) + { + _expiredOfferIds.TryAdd(offer.Id, null); + } + } + } + + public void RemoveExpiredOffers() + { + logger.Warning($"removing {_expiredOfferIds.Count} expired offers"); + foreach (var (expiredOfferId, _) in _expiredOfferIds) + { + RemoveOffer(expiredOfferId, false); + } } }