fix(config): clear the item bins before rebuilding them on reload - #38
Merged
Conversation
Initialize() runs InitializeFromFile(), InitializeFromSql() and InitializeBins() and never calls Reset(), and ".reload config" goes through Initialize(). InitializeBins() only ever inserts, so on a reload: - items the reloaded configuration or an updated mod_auctionhousebot_disabled_items now excludes stay in the bins and in SellableItems, so an admin who blacklists an item and reloads sees no effect until a full restart - and SellableItems is what gates the price override path against exactly that blacklist; - the *Vec vectors, which are never cleared anywhere, gain a second copy of every item on each reload, skewing the random pick in the rarity tier fallback and growing without bound. Clear all of them at the top of the rebuild.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found by the pre-build review over
38e6a83..merkerhood, and verified against the code before writing this.Problem
AHBConfig::Initialize()isInitializeFromFile()+InitializeFromSql()+InitializeBins(). It does not callReset(), and.reload configgoes throughInitialize()(AuctionHouseBotWorldScript.cpp:127-129).InitializeBins()only ever inserts into the bins — theclear()calls live inReset()and in the copy constructor, neither of which is on the reload path.Two consequences:
mod_auctionhousebot_disabled_items, runs.reload config, and the item stays in the bins and inSellableItems— so it keeps being listed until a full server restart.SellableItemsis precisely what now gates the price-override path against the blacklist, so this blunts that fix on the one operational path an admin would reach for to fix a bad item on a running realm.*Vecvectors are never cleared anywhere at all — not even inReset(). Every reload appends a full second copy of every item. They feedgetElement(config->GreyItemsVec, urand(0, size - 1), ...)in the rarity-tier fallback, so duplicates skew the random pick, and the vectors grow without bound across reloads.Fix
Clear the 14 bin sets, the 14 vectors and
SellableItemsat the top ofInitializeBins(), before the scan.NpcItems,LootItems,DisableItemStoreandSellerWhiteListare deliberately untouched — they are filled elsewhere and are not this function's to reset.Scope note
The stale-bin half is pre-existing and shared by all the legacy bins; it is included because the new
SellableItemsset inherits the same gap and because fixing only the new set would leave the bins it is derived from inconsistent with it.Verification
Initialize()and the.reload configpath directly; confirmed noReset()call and noclear()anywhere inInitializeBins(). Confirmed by grep that the*Veccontainers have noclear()in the entire file.core-buildworkflow on this PR..reload configafter blacklisting an item, then confirming the bot stops listing it without a restart.