Repository navigation
Conversation
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 1 medium |
🟢 Metrics 274 complexity · 14 duplication
Metric Results Complexity 274 Duplication 14
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
|
The remaining Codacy finding (SonarC# S1172, searchRange in BotMarketPlanner.GetReachableSafezone) is a false positive: the parameter is used inside the local function IsUsable, which limits the search to the stall range around the arrival point. Sonar doesn't see the use through the local function. Removing it would make every search cover the whole safezone, which the city stalls must not do. |
sven-n
left a comment
sven-n
left a comment
There was a problem hiding this comment.
Private code review of PR #1455 (commit 7b2ad3e). I recommend addressing or disproving these two lifecycle issues before merging:
[Medium] Failed store openings bypass the market-shift cooldown. In src/GameLogic/Bots/BotNavigator.cs around lines 1799–1835, a failed OpenStoreAsync marks the shift as ending and queues cleanup, but does not set NextMarketShiftUtc as the normal shift-ending path does. After cleanup, the bot can be selected again immediately, causing repeated trips/open attempts when a store consistently fails to open (for example, because its configured currency is unavailable). Please use the common end-shift path or apply a retry cooldown, and add a regression test for repeated failures.
[Medium; lifecycle needs verification] Sale-income cleanup can be skipped on immediate disconnect. src/GameLogic/Bots/BotMerchantPlugIn.cs around lines 76–86 enqueues BotMerchandise.RemoveIncomeAsync for the bot's next tick after a sale. The disconnect path in src/GameLogic/Offline/OfflinePlayer.cs around lines 447–482 does not visibly drain pending bot actions. If the bot logs out, rotates, or the server shuts down before the next tick, the sale proceeds may survive instead of being removed, potentially affecting economy integrity. Please guarantee settlement before disconnect/persistence, or demonstrate that the lifecycle already drains these actions; test purchase followed immediately by forced logout/reconnect, checking Zen and jewel proceeds.
The new classification/pricing and planner tests are useful, but neither of these failure paths appears covered. The feature is disabled by default, which limits deployment exposure. This PR is stacked on #1438; please validate it after that dependency is merged or rebased. No confirmed directly exploitable security vulnerability was found.
|
Thanks for the review - both points were right, fixed in two new commits: Failed store openings bypass the cooldown - b74c982: a store which the game doesn't open now ends the shift through the common path ( Sale income on immediate disconnect - af09883: confirmed, the pending bot actions aren't drained on disconnect, so the queued removal was lost when the bot logged out before its next tick. I'll rebase onto master and validate again once PR 1438 is merged. |
fc0862e to
9f2f394
Compare
sven-n
left a comment
sven-n
left a comment
There was a problem hiding this comment.
Thanks! The bot merchants fit the existing bot and bot-chat design well: they're off by default, use the game's own store actions, and remove the earnings after each sale so no money enters the economy.
It conflicts with master now, though: #1504 regenerated GameLogic/Properties/PlugInResources.Designer.cs (and #1463 changed BotNavigator.cs). Please rebase: keep the entries of both sides in PlugInResources.resx and regenerate the designer. I'll run the tests and review it in full once it's clean.
Generated by Claude Code
A bot with enough priced goods in its personal store goes to the market map (Loren Market) for a shift of random length, opens its store at a stall apart from the other merchants and warps home to hunt afterward. It leaves early when it sold everything, after a long time without a sale, or when the game closed its store. The new Bot merchants plugin (off by default) limits the merchants per game server. The bot reaches the market through its spawn gate, not the warp list, whose level requirement and costs are for the players. While on a shift, a bot declines party invitations and isn't put into bot parties, and a store left open at logout stays closed at the next login.
Instead of selling it to an NPC, a bot puts the loot worth a stall on the market into its personal store, with a price: ancient items, excellent items with two or more options or with a wanted option (damage decrease, defense success rate, reflection, excellent damage, damage increase, attack speed), excellent items with a cheap option and luck, and normal items of the higher sets from +7 with luck. The price follows from three price classes, each with its currency (zen or a jewel) and surcharges per level above +7 and per additional option. Ancient items and the best excellent items of the higher sets cost more. Goods whose currency isn't allowed in player shops are sold to the NPC as before. A zen price is drawn within 10 % up or down when the good goes into the store and rounded the way a player would ask for it, so the same good costs a bit differently at two merchants; prices in jewels stay exact.
The jewels a bot doesn't keep for its own gear go into its store too, at most a configured number per kind. For each jewel the admin sets the currency - zen or another jewel - and the price; by default the jewels are sold for their value in zen. A jewel is never sold for itself.
A bot buys nothing, so the zen and the jewels paid for its goods are removed right after each sale - the jewels would otherwise fill its inventory until it can't take any payment anymore. It keeps what it had when it opened the store. There are no discounts, because the players would learn to wait for them. A good which found no buyer for a configured number of shifts leaves the store and is sold to an NPC, making room for fresh goods. When the plugin is switched off, the bots take their goods out of the store and sell them to the NPC as well.
The merchant adverts of the bot chat were meant for merchant bots which
didn't exist yet, so they were off. Now only a bot behind its open store
on the market advertises - with new lines which point to its store
({store} is its name) - and the chance is 80 % by default: while the
Bot merchants plugin is off, there's nobody to advertise.
A player who whispers to such a bot what it sells ("what are you
selling?", "za ile?"), or asks it by name in the normal chat next to its
stall, is pointed to its store by the new merchant whisper replies; any
other message gets a usual answer. Their keywords cover the trade
keywords of the usual replies, so a merchant never says it has nothing
to sell.
Each shift drew one of the 8 default store names regardless of the stores already open, so two stores next to each other on the market often had the same name. A shift now draws one which no other open store has; when all are taken, the store gets the name of the bot. The default list holds 50 names, enough for the maximum number of merchants.
Players don't only sell on Loren Market - some open their store in town. 20 % of the shifts (City merchants) now take place in one of the city maps (City map numbers: Lorencia, Devias, Noria and Elvenland by default), in the safezone near the spawn gate, with the stalls kept apart per map. A city which can't take the bot leaves it to the market. The maximum of merchants is now 50 by default and at most 50: it's a ceiling for the bots which collected enough goods, which go back hunting after their shift, not a number of dedicated merchants.
Some default store names promised certain goods ("Weapons here", "Jewels
here", "Exc items"), while the store of a bot holds whatever it looted,
e.g. only Jewels of Chaos. The default list now holds 50 names which fit
any goods.
Right after a server start, and every hour after, most bots are grouped into parties of bots, and only a bot without a party could start a market shift. On a lab server with 350 bots, 89 had enough goods, but only 2 went to the market. A bot in a party of bots now leaves it for its shift, like a player who goes selling; a party with a player is never left. Bots without a party go first. At most 3 bots (New merchants per 30 seconds) start a shift every check, so after a server start the market fills up within minutes and the parties don't fall apart at once.
The stalls were only drawn within 18 tiles of the spawn gate, so on Loren Market the stores crowded one side while most of its safezone stayed empty, and a store could stand right against a wall or in the narrow entrance. On the market, a stall is now drawn from the whole safezone which the bot can walk to from the arrival point; in a city, the stores stay within the range around the spawn gate, in the middle of the town. Every stall keeps two tiles of walkable safezone around it, so no store stands against a wall and corridors narrower than five tiles stay free.
A bot drawn for a city without a free stall skipped its shift until the next check; like for a city which can't be used, it now goes to the market. The documentation names the settings of the merchants.
Each jewel counts as one of the goods a bot needs to start a market shift, and Jewels of Chaos are the most common loot. With up to five per kind, most stores on a live server held nothing but four or five Jewels of Chaos. Two per kind still sells the surplus, but a bot needs some other goods to open a store.
A bot whose store the game didn't open (e.g. a price in a currency which isn't allowed) ended its shift without setting the time of its next one, so it could be sent to the market again right away and fail again. It ends the shift like any other reason now.
The income of a sale is removed in the next tick of the bot. A bot which logged out, rotated or got stopped by a shutdown before that tick kept the income, because the logout saved it. The bot now closes its store and removes the income under its persistence lock before the logout saves it.
The plugin counted the shifts each good spent in a store without a buyer and forgot a count only when a player bought the good. A retired good which went to an NPC, or the good of a removed bot, kept its count until the server restarted. Each count now remembers when it was counted last; every 10 minutes, the counts which weren't counted for a day - or for several shifts and breaks, if they're configured longer - are forgotten.
The plugin picked the bots for a market shift, looked for their stalls and only then took them out of their parties. Meanwhile, a player could join the party, so the bot left a party with a player, or the bot could start an errand or an event meeting. The bot is checked again right before it leaves its party, and a party is left only while it has no player.
A store stays open until the next tick of the bot after its shift ended. A sale in between queued a removal of its income, which ran after the store was closed - when the goods without a buyer for too long were already back in the inventory. They looked like income and were destroyed instead of being sold to an NPC. Closing the store settles the shift now, and a later removal of the income does nothing.
…d off The opening of a store is queued for the next tick of the bot. When the plugin was switched off in between, the store still opened, only to be closed again in the tick after. It stays closed now; the next tick ends the shift as before.
|
omg not again... conflict |
|
Rebased onto current master (64c534a, with PR 1487, 1471, 1464 and 1461). PlugInResources.resx keeps the entries of both sides, and PlugInResources.Designer.cs is regenerated in every commit, so it matches its resx throughout the history. BotNavigator.cs merged cleanly with the changes of PR 1463. Full test suite on the new head d8c7410: MUnique.OpenMU.Tests 3146 passed / 2 skipped, Web.Tests 709, the rest green, except ItemStatProfilesDataTest.Season6ItemCountsPerProfile (expected default: 244, was default: 246). It fails the same way on master 64c534a itself, probably because PR 1471, 1464 and 1461 each added items and adjusted the count separately, so it isn't caused by this PR. |
Summary
sell their best loot to the players in personal stores – on Loren Market, the map made for it,
and with a share of the shifts in the towns, the way players do.
trips to town (instead of selling them to the NPC) and now and then goes on a market shift of
30–90 minutes. It uses the game's own store actions, so players buy exactly as from a player.
plus surcharges; jewels above the bot's own stock are sold too. All of it is configurable in the panel.
don't pump money into the economy, they only move loot to the players.
How it looks in the game
📖 Full description: docs › Server-side AI bots › Selling on the market
(part of this PR).
Enable Bot merchants under Feature plugins. For prices in jewels, also enable Player shop
currencies with these jewels (the original client can only pay zen). Then:
how much?/wts?(also Polish:ile za,sklep, …)Commits
feat(bots): let bots sell their goods on market shifts– plugin, configuration, the shift (warp, stall, store open/close, limit)feat(bots): stock the bots' stores with their best loot– goods classes and prices, put into the store before the NPC is visitedfeat(bots): let the bots sell their surplus jewels– currency and price per jewel kindfeat(bots): remove the merchants' income and retire unsold goods– after 3 shifts without a buyer, the good goes to the NPCfeat(bots): let only the merchants on the market advertise– Bot chat: adverts and whisper replies only from bots with an open storedocs(bots): describe selling on the marketfix(bots): give every open store of the merchants its own namefeat(bots): let a share of the merchants sell in the citiesfix(bots): use store names which fit any goodsfix(bots): let bots leave their party of bots for a market shift– otherwise ~80 % of the bots (in bot parties) could never gofix(bots): spread the market stalls and keep them away from the wallsfix(bots): send the merchant to the market when its city is fullfix(bots): put at most two jewels of a kind into a merchant's store– with five, most stores on a live server held only Jewels of Chaosfix(bots): wait for the next market shift after a store failed to open– reviewfix(bots): remove the market income of a bot which logs out– reviewfix(bots): forget the unsold shifts of goods which left the market– the counts of retired goods piled up until a restart; now forgotten after a day without a shiftfix(bots): check a merchant again before it leaves its party– a player could join the party between the pick and the leavefix(bots): keep the retired goods when a late sale removes the income– a sale right before the store closed made its queued income removal destroy the goods just taken out of the storefix(bots): don't open a merchant's store after the plugin was switched offDesign notes
BotMerchantPlugIn(IDisabledByDefault) owns the configuration and decideswho starts a shift (
BotMarketPlanner);BotNavigatoronly drives the trip, like an event meeting.OpenStoreAction,CloseStoreActionand the normal price items, so allchecks (allowed currencies, prices) apply as for a player. Opening and closing run in the MU Helper
tick, like the trades (
OfflinePlayer.PendingBotActions).players, which is not meant for an NPC-like seller.
(
BotMerchandise.RemoveIncomeAsync). A bot's full inventory would otherwise block its own shopping.a party with a human player is never left.
the original client pays player stores in zen only. A server for the original client can set all
price classes to zen.
the plugin is enabled.
only reach a fresh configuration. There, set Bot chat → Merchant adverts: chance (%) (0 there, 80 by
default here) and fill Merchant whisper replies (empty there) in the panel - or reset the Bot chat
configuration to its defaults, which drops the own changes.
Testing
BotMarketPlannerTest,BotMerchandiseTest, additions toBotChatTest(stall placement,distinct store names, city share, party leaving, goods classes, prices and spread, income removal, forgetting old unsold counts).
00f737374: all green –MUnique.OpenMU.Tests3019 passed / 2 skipped,Persistence.Initialization.Tests838/12 skipped,Web.Tests661, the rest green; with commit 13 the bot tests again (466 passed)3738d38e9): full suite green –MUnique.OpenMU.Tests3050 passed / 2 skipped,Persistence.Initialization.Tests851/12 skipped,Web.Tests666, the rest greend2504ab7e(with PR 1438) and with commits 16–19: full suite green –MUnique.OpenMU.Tests3060 passed / 2 skipped,Persistence.Initialization.Tests851/12 skipped,Web.Tests666, the rest green. The new tests of commits 17–19 fail without their fix.ring for 2 jewels; income removed; an emptied store closed by the game and the bot warped home.
Stores spread over the whole market (36 shifts in 6 minutes: 31 market, 5 towns), none at a wall,
no repeated store names; whisper replies and adverts from merchants work.