Add persona-driven bots, wealth economy, and friendship parties - #79
Merged
Conversation
📝 WalkthroughWalkthroughThe change adds persistent bot personas, persona-based party and conversation behavior, bot friendships and roster management, adaptive background-party recruitment, wealth-driven economic policies, static-buyer sales, and market telemetry. ChangesPersona and social coordination
Persona-driven economy and market telemetry
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
src/GameServer/Bot/BotManager.js (1)
213-217: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPrefer the persisted persona over re-generation in the cold panel.
BotPersona.generate(state)re-derives the persona fromstate.stats. The stored row is authoritative and is what the behavior policies consume. If the stored seed or persona version ever differs from the generator output, the panel shows a persona that no policy uses.BotPersona.snapshot(characterId)reads the cache synchronously, so the panel stays cheap.♻️ Proposed refactor
- const persona = BotPersona.generate(state); + const persona = BotPersona.snapshot(state.characterId) || BotPersona.generate(state);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/GameServer/Bot/BotManager.js` around lines 213 - 217, Update the cold-panel logic near personaTraits to use the persisted persona from BotPersona.snapshot(characterId) instead of re-generating it with BotPersona.generate(state). Keep the existing trait and goal-label formatting unchanged, and use the snapshot result consumed by behavior policies as the panel’s persona source.src/GameServer/Bot/Economy/ColdMarketListingService.js (1)
623-634: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate reduce over
liquidated.
liquidated.reduce(...)runs twice with the same accumulator logic: once forMarketTelemetry.closedand once forliquidatedCount. Compute it once and reuse the value.♻️ Proposed consolidation
.then(({ state: liquidatedState, warehouseCount, liquidated }) => { const reason = hasStock ? 'expired' : 'sold_out'; - MarketTelemetry.closed(reason, liquidated.reduce((sum, item) => sum + Number(item.count || 0), 0)); + const liquidatedCount = liquidated.reduce((sum, item) => sum + Number(item.count || 0), 0); + MarketTelemetry.closed(reason, liquidatedCount); return LifeState.upsertState(liquidatedState, hasStock ? 'cold_market_expired' : 'cold_market_sold_out') .then((saved) => ({ state: saved || liquidatedState, closed: true, reason: hasStock ? 'expired' : 'sold_out', warehouseCount, - liquidatedCount: liquidated.reduce((sum, item) => sum + Number(item.count || 0), 0) + liquidatedCount })); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/GameServer/Bot/Economy/ColdMarketListingService.js` around lines 623 - 634, In the promise handler around the liquidatedState result, compute the total liquidated count from liquidated once, store it in a local value, and reuse that value for both MarketTelemetry.closed and the returned liquidatedCount field. Preserve the existing reason and state behavior.src/GameServer/Bot/Economy/PersonaEconomicPolicy.js (1)
7-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated
personaForhelper across economy-policy modules.PersonaEconomicPolicy.jsandWealthInvestmentPolicy.jseach define an identicalpersonaFor(state)helper that resolvesstate.personaor falls back toBotPersona.generate(state). The shared root cause is the lack of a single reusable persona-resolution helper.
src/GameServer/Bot/Economy/PersonaEconomicPolicy.js#L7-L9: remove the localpersonaForand import a shared implementation instead.src/GameServer/Bot/Economy/WealthInvestmentPolicy.js#L8-L10: remove the localpersonaForand import the same shared implementation instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/GameServer/Bot/Economy/PersonaEconomicPolicy.js` around lines 7 - 9, The personaFor helper is duplicated across both economy-policy modules. Create or reuse one shared persona-resolution implementation, remove the local personaFor definitions in src/GameServer/Bot/Economy/PersonaEconomicPolicy.js lines 7-9 and src/GameServer/Bot/Economy/WealthInvestmentPolicy.js lines 8-10, and import the shared helper in both modules while preserving its existing state.persona fallback behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/GameServer/Bot/AI/BotConversation.js`:
- Around line 38-42: Update the sociability branch in restCloser so
trait(session, 'sociability') >= 0.70 returns distinct text from the default
branch, preserving the existing caution response and matching the trait-based
conversational variety used by restOpener and roleLine.
In `@src/GameServer/Bot/AI/BotFriendship.js`:
- Around line 40-44: Normalize botId once at the friendship operation boundary
to a finite, positive integer, rejecting invalid values before any SQL binding.
Reuse this validated identifier across isFriend, remove, and toggleConst, and
preserve the handler’s expected result handling so invalid IDs cannot silently
produce successful no-op operations.
In `@src/GameServer/Bot/AI/BotPersona.js`:
- Line 88: Remove the unconditional period after the style fallback in the
persona text-card template so the fallback “Speaks plainly and stays in
character.” renders with only one terminal period. Preserve the existing
punctuation for non-empty style values in the persona formatting logic.
In `@src/GameServer/Bot/BotManager.js`:
- Around line 188-193: Rename the persona-traits row in the table-building code
to a distinct label, keeping the social-memory relationship row labeled
“Social.” Update the row currently using traits.social and traits.style without
changing their values or the surrounding invite data.
In `@src/GameServer/Bot/Population/BotLifeState.js`:
- Line 1682: Update applyNpcLiquidation so the options object is spread before
the computed payout, sold, and at fields, ensuring caller-provided values cannot
overwrite the computed liquidation data.
In `@src/GameServer/Bot/Population/PopulationService.js`:
- Line 712: Update formBackgroundParties and reclaimBackgroundPartyCapacity so
refreshBackgroundPartyRequirements runs periodically whenever active parties
exist, regardless of partyWaitStates being empty or reclaimCount being zero.
Keep capacity-reclaim logic gated by backlog and reclaim eligibility, but
decouple the refresh path and preserve the configured partyRequirementRefreshMs
and partyRequirementRefreshBatchSize behavior.
- Around line 377-401: Add a rejection handler to the promise chain in
schedulePersonaBackfill so failures from BotPersona.backfillGenerated() are
caught and logged, while leaving the timer active for the next retry; preserve
the existing exhaustion cleanup and finally-based running-state reset.
In `@src/GameServer/World/Generics/NpcBypasses/BotFriends.js`:
- Line 71: Update the form-mode invite chain in BotFriendship.selected and its
bots.reduce callback to catch and absorb each World.inviteFriendByName rejection
before the next bot runs. Preserve sequential invitation processing and ensure
the final render(session) executes after all bots, even when individual invites
fail.
In `@tests/test_bot_persona.js`:
- Line 63: Remove the unused params destructuring from the Database.execute
callback in the test, retaining only the sql argument needed by the callback.
---
Nitpick comments:
In `@src/GameServer/Bot/BotManager.js`:
- Around line 213-217: Update the cold-panel logic near personaTraits to use the
persisted persona from BotPersona.snapshot(characterId) instead of re-generating
it with BotPersona.generate(state). Keep the existing trait and goal-label
formatting unchanged, and use the snapshot result consumed by behavior policies
as the panel’s persona source.
In `@src/GameServer/Bot/Economy/ColdMarketListingService.js`:
- Around line 623-634: In the promise handler around the liquidatedState result,
compute the total liquidated count from liquidated once, store it in a local
value, and reuse that value for both MarketTelemetry.closed and the returned
liquidatedCount field. Preserve the existing reason and state behavior.
In `@src/GameServer/Bot/Economy/PersonaEconomicPolicy.js`:
- Around line 7-9: The personaFor helper is duplicated across both
economy-policy modules. Create or reuse one shared persona-resolution
implementation, remove the local personaFor definitions in
src/GameServer/Bot/Economy/PersonaEconomicPolicy.js lines 7-9 and
src/GameServer/Bot/Economy/WealthInvestmentPolicy.js lines 8-10, and import the
shared helper in both modules while preserving its existing state.persona
fallback behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c430afec-ff25-420b-8c53-031df53f57e1
📒 Files selected for processing (49)
database/sql/sqlite.sqlscripts/migrate-mariadb-to-sqlite.jsscripts/run-tests.jsscripts/world-wipe.jssrc/GameServer/Bot/AI/BotAvailability.jssrc/GameServer/Bot/AI/BotBrain.jssrc/GameServer/Bot/AI/BotBrainContext.jssrc/GameServer/Bot/AI/BotConversation.jssrc/GameServer/Bot/AI/BotFriendship.jssrc/GameServer/Bot/AI/BotPersona.jssrc/GameServer/Bot/AI/BotRemoteChat.jssrc/GameServer/Bot/AI/BotStatus.jssrc/GameServer/Bot/AI/PersonaPartyDecisionPolicy.jssrc/GameServer/Bot/BotManager.jssrc/GameServer/Bot/Economy/ColdMarketListingService.jssrc/GameServer/Bot/Economy/ColdMarketService.jssrc/GameServer/Bot/Economy/MarketTelemetry.jssrc/GameServer/Bot/Economy/MarketTownPolicy.jssrc/GameServer/Bot/Economy/PersonaEconomicPolicy.jssrc/GameServer/Bot/Economy/StaticBuyerService.jssrc/GameServer/Bot/Economy/WealthInvestmentPolicy.jssrc/GameServer/Bot/Goals/NeedsEvaluator.jssrc/GameServer/Bot/Population/BackgroundPartyComposition.jssrc/GameServer/Bot/Population/BotLifeState.jssrc/GameServer/Bot/Population/GeneratedColdSeeder.jssrc/GameServer/Bot/Population/PersonaPartyPolicy.jssrc/GameServer/Bot/Population/PopulationConfig.jssrc/GameServer/Bot/Population/PopulationService.jssrc/GameServer/Bot/Population/PopulationStatus.jssrc/GameServer/Network/Request/Speak.jssrc/GameServer/World/Generics/NpcBypasses/BotFriends.jssrc/GameServer/World/World.jstests/test_bot_availability.jstests/test_bot_background_party_affinity.jstests/test_bot_background_party_recruitment.jstests/test_bot_cold_market_listing.jstests/test_bot_conversation.jstests/test_bot_friendship.jstests/test_bot_goal_planner.jstests/test_bot_persona.jstests/test_bot_persona_background_intent.jstests/test_bot_persona_economic_policy.jstests/test_bot_persona_party_decision.jstests/test_bot_population_state.jstests/test_bot_remote_chat_persona.jstests/test_bot_spot_risk_baseline.jstests/test_bot_static_buyer_sale.jstests/test_party_companion_rest_follow.jstests/test_wealth_investment_policy.js
Comment on lines
+38
to
42
| function restCloser(session) { | ||
| if (trait(session, 'caution') >= 0.70) return 'Sounds good. We can keep it steady and avoid rushing back in.'; | ||
| if (trait(session, 'sociability') >= 0.70) return 'Sounds good. Better than rushing back in alone.'; | ||
| return 'Sounds good. Better than rushing back in alone.'; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the dead sociability branch in restCloser.
The trait(session, 'sociability') >= 0.70 branch on Line 40 returns the same string as the default branch on Line 41. This branch can never produce a different result than omitting it, unlike restOpener and roleLine, which each vary text per trait or drive.
Give the sociability branch distinct text, consistent with the drive/trait-based variety used elsewhere in this file.
🐛 Proposed fix
function restCloser(session) {
if (trait(session, 'caution') >= 0.70) return 'Sounds good. We can keep it steady and avoid rushing back in.';
- if (trait(session, 'sociability') >= 0.70) return 'Sounds good. Better than rushing back in alone.';
+ if (trait(session, 'sociability') >= 0.70) return 'Sounds good. It is better with company than rushing back in alone.';
return 'Sounds good. Better than rushing back in alone.';
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function restCloser(session) { | |
| if (trait(session, 'caution') >= 0.70) return 'Sounds good. We can keep it steady and avoid rushing back in.'; | |
| if (trait(session, 'sociability') >= 0.70) return 'Sounds good. Better than rushing back in alone.'; | |
| return 'Sounds good. Better than rushing back in alone.'; | |
| } | |
| function restCloser(session) { | |
| if (trait(session, 'caution') >= 0.70) return 'Sounds good. We can keep it steady and avoid rushing back in.'; | |
| if (trait(session, 'sociability') >= 0.70) return 'Sounds good. It is better with company than rushing back in alone.'; | |
| return 'Sounds good. Better than rushing back in alone.'; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/GameServer/Bot/AI/BotConversation.js` around lines 38 - 42, Update the
sociability branch in restCloser so trait(session, 'sociability') >= 0.70
returns distinct text from the default branch, preserving the existing caution
response and matching the trait-based conversational variety used by restOpener
and roleLine.
Comment on lines
+40
to
+44
| isFriend(player, botId) { | ||
| const playerId = id(player); | ||
| if (!playerId || !botId) return Promise.resolve(false); | ||
| return Database.execute(["SELECT 1 FROM bot_friendships WHERE playerId = ? AND botId = ? AND status = 'accepted'", [playerId, Number(botId)]]).then((rows) => !!rows[0]); | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
echo '--- candidate files ---'
fd -i 'BotFriendship\.js|BotFriends\.js' src
echo '--- BotFriendship outline ---'
ast-grep outline src/GameServer/Bot/AI/BotFriendship.js --view expanded
echo '--- BotFriends handler references ---'
rg -n -C 8 'handler|parts\[2\]|isFriend|remove|toggleConst' src/GameServer/World/Generics/NpcBypasses/BotFriends.js
echo '--- BotFriendship relevant implementation ---'
cat -n src/GameServer/Bot/AI/BotFriendship.js | sed -n '1,125p'
echo '--- Database.execute implementation and callers ---'
rg -n -C 6 'execute\s*\(' src | head -n 240Repository: pmbstyle/L2Solo
Length of output: 29498
🏁 Script executed:
#!/bin/bash
set -eu
echo '--- Database binding path ---'
cat -n src/Database.js | sed -n '1,95p'
cat -n src/Database.js | sed -n '220,275p'
echo '--- dependency and SQLite references ---'
rg -n -i 'sqlite|better-sqlite|bind|prepare|run\(' package.json package-lock.json yarn.lock pnpm-lock.yaml src/Database.js 2>/dev/null | head -n 180
echo '--- standalone Number coercion probe ---'
node - <<'JS'
const values = ['abc', '12', '', ' ', '0', '-1', '1.5', 'Infinity', Infinity, NaN, null, undefined, 12];
for (const value of values) {
const normalized = Number(value) || 0;
console.log(JSON.stringify(value), 'Number=', Number(value), 'Number||0=', normalized,
'isFinite=', Number.isFinite(Number(value)), 'integer=', Number.isInteger(Number(value)));
}
JSRepository: pmbstyle/L2Solo
Length of output: 10908
🏁 Script executed:
#!/bin/bash
set -eu
echo '--- exact node:sqlite NaN binding probe ---'
node - <<'JS'
const { DatabaseSync } = require('node:sqlite');
const db = new DatabaseSync(':memory:');
const statement = db.prepare('SELECT ? AS value');
for (const value of [NaN, Infinity, 1.5, -1, 0, 12, 'abc']) {
try {
const row = statement.get(value);
console.log(JSON.stringify(value), 'OK', row);
} catch (error) {
console.log(JSON.stringify(value), 'ERROR', error.name + ': ' + error.message);
}
}
JS
echo '--- Database.run error and queue behavior ---'
cat -n src/Database.js | sed -n '87,145p'
echo '--- all BotFriendship method call sites ---'
rg -n -C 5 '\.(isFriend|remove|toggleConst)\s*\(' srcRepository: pmbstyle/L2Solo
Length of output: 14885
🏁 Script executed:
#!/bin/bash
set -eu
echo '--- bot identifier schema definitions ---'
rg -n -C 5 'bot_friendships|bot_friend_roster|bot_life_state|characterId.*INTEGER|botId.*INTEGER' database src | head -n 260
echo '--- relevant handler rendering behavior ---'
cat -n src/GameServer/World/Generics/NpcBypasses/BotFriends.js | sed -n '50,75p'Repository: pmbstyle/L2Solo
Length of output: 22092
Reject invalid botId values before SQL binding.
parts[2] can be a non-numeric string. Number("abc") becomes NaN, which node:sqlite binds as NULL. remove then returns { ok: true } without deleting a row. toggleConst returns not_friend, but the handler ignores the result. Normalize once to a finite, positive integer and reuse it in isFriend, remove, and toggleConst.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/GameServer/Bot/AI/BotFriendship.js` around lines 40 - 44, Normalize botId
once at the friendship operation boundary to a finite, positive integer,
rejecting invalid values before any SQL binding. Reuse this validated identifier
across isFriend, remove, and toggleConst, and preserve the handler’s expected
result handling so invalid IDs cannot silently produce successful no-op
operations.
| const risk = traits.caution >= 0.62 | ||
| ? 'avoids needless danger' | ||
| : traits.caution <= 0.38 ? 'will take a calculated chance' : 'weighs danger against the reward'; | ||
| return `${persona.archetype.replace(/_/g, ' ')} focused on ${driveLabel(persona.primaryDrive)}. ${group}; ${risk}. ${style || 'Speaks plainly and stays in character.'}.`; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the double period in the text-card fallback.
The fallback string already ends with a period. The template appends another one, so an empty style yields Speaks plainly and stays in character... This text is shown in the bot status panel and sent to the LLM context.
🐛 Proposed fix
- return `${persona.archetype.replace(/_/g, ' ')} focused on ${driveLabel(persona.primaryDrive)}. ${group}; ${risk}. ${style || 'Speaks plainly and stays in character.'}.`;
+ return `${persona.archetype.replace(/_/g, ' ')} focused on ${driveLabel(persona.primaryDrive)}. ${group}; ${risk}. ${style ? `${style}.` : 'Speaks plainly and stays in character.'}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return `${persona.archetype.replace(/_/g, ' ')} focused on ${driveLabel(persona.primaryDrive)}. ${group}; ${risk}. ${style || 'Speaks plainly and stays in character.'}.`; | |
| return `${persona.archetype.replace(/_/g, ' ')} focused on ${driveLabel(persona.primaryDrive)}. ${group}; ${risk}. ${style ? `${style}.` : 'Speaks plainly and stays in character.'}`; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/GameServer/Bot/AI/BotPersona.js` at line 88, Remove the unconditional
period after the style fallback in the persona text-card template so the
fallback “Speaks plainly and stays in character.” renders with only one terminal
period. Preserve the existing punctuation for non-empty style values in the
persona formatting logic.
Comment on lines
+188
to
193
| ['Type', safe(personaArchetype(persona))], | ||
| ['Drive', safe(persona?.primaryDrive || 'unavailable')], | ||
| ['Social', safe(traits.social)], | ||
| ['Style', safe(traits.style)], | ||
| ['Social', safe(social)], | ||
| ['Invite', safe(invite)] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Two rows now share the Social label.
Line 190 shows persona traits. Line 192 shows social-memory relationship data. The table renders both labels as Social, so the operator cannot tell the rows apart. Rename the persona row.
🐛 Proposed fix
['Type', safe(personaArchetype(persona))],
['Drive', safe(persona?.primaryDrive || 'unavailable')],
- ['Social', safe(traits.social)],
+ ['Traits', safe(traits.social)],
['Style', safe(traits.style)],
- ['Social', safe(social)],
+ ['Relationship', safe(social)],
['Invite', safe(invite)]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ['Type', safe(personaArchetype(persona))], | |
| ['Drive', safe(persona?.primaryDrive || 'unavailable')], | |
| ['Social', safe(traits.social)], | |
| ['Style', safe(traits.style)], | |
| ['Social', safe(social)], | |
| ['Invite', safe(invite)] | |
| ['Type', safe(personaArchetype(persona))], | |
| ['Drive', safe(persona?.primaryDrive || 'unavailable')], | |
| ['Traits', safe(traits.social)], | |
| ['Style', safe(traits.style)], | |
| ['Relationship', safe(social)], | |
| ['Invite', safe(invite)] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/GameServer/Bot/BotManager.js` around lines 188 - 193, Rename the
persona-traits row in the table-building code to a distinct label, keeping the
social-memory relationship row labeled “Social.” Update the row currently using
traits.social and traits.style without changing their values or the surrounding
invite data.
| }, | ||
|
|
||
| applyNpcLiquidation(state, candidates = []) { | ||
| applyNpcLiquidation(state, candidates = [], options = {}) { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect every caller of applyNpcLiquidation to check what keys are passed as `options`.
rg -n -B3 -A8 '\bapplyNpcLiquidation\s*\(' --type=jsRepository: pmbstyle/L2Solo
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- candidate files ---'
fd -i 'BotLifeState\.js$|.*Population.*' . | head -80
printf '%s\n' '--- method definition and references ---'
rg -n -F 'applyNpcLiquidation' . --glob '!node_modules' --glob '!dist' --glob '!build' || true
printf '%s\n' '--- relevant implementation ---'
file=$(fd -i 'BotLifeState\.js$' . | head -1)
if [ -n "${file}" ]; then
nl -ba "$file" | sed -n '1665,1725p'
fi
printf '%s\n' '--- repository status and tracked path ---'
git ls-files | rg '(^|/)BotLifeState\.js$|Population|population' | head -120Repository: pmbstyle/L2Solo
Length of output: 2949
🏁 Script executed:
#!/bin/bash
set -u
file=src/GameServer/Bot/Population/BotLifeState.js
printf '%s\n' '--- implementation ---'
sed -n '1668,1725p' "$file" | awk '{printf "%5d %s\n", NR+1667, $0}'
printf '%s\n' '--- StaticBuyerService caller ---'
sed -n '35,78p' src/GameServer/Bot/Economy/StaticBuyerService.js | awk '{printf "%5d %s\n", NR+34, $0}'
printf '%s\n' '--- ColdMarketListingService caller ---'
sed -n '600,630p' src/GameServer/Bot/Economy/ColdMarketListingService.js | awk '{printf "%5d %s\n", NR+599, $0}'
printf '%s\n' '--- all call-site context ---'
rg -n -B8 -A14 -F 'applyNpcLiquidation' src testsRepository: pmbstyle/L2Solo
Length of output: 14088
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- exports around applyNpcLiquidation ---'
rg -n -B4 -A12 'module\.exports|exports\.' src/GameServer/Bot/Population/BotLifeState.js | tail -80
printf '%s\n' '--- every literal field passed in options at call sites ---'
python3 - <<'PY'
from pathlib import Path
for p in Path("src").rglob("*.js"):
text = p.read_text(errors="replace")
needle = "applyNpcLiquidation"
if needle not in text:
continue
lines = text.splitlines()
for i, line in enumerate(lines):
if needle in line:
print(f"{p}:{i+1}")
for j in range(i, min(i+8, len(lines))):
print(f" {j+1}: {lines[j]}")
PY
printf '%s\n' '--- isolated object-spread behavior ---'
node - <<'JS'
const payout = 12;
const sold = [{ selfId: 1, amount: 2, price: 6 }];
const at = 100;
const safeOptions = { source: 'static_buyer', town: 'Town', buyers: ['NPC'] };
const collidingOptions = { payout: 999, sold: [], at: 200 };
console.log(JSON.stringify({
currentWithKnownOptions: { payout, sold, at, ...safeOptions },
currentWithCollisions: { payout, sold, at, ...collidingOptions },
proposedWithCollisions: { ...collidingOptions, payout, sold, at }
}));
JSRepository: pmbstyle/L2Solo
Length of output: 2208
Prevent options from overwriting computed liquidation data.
Current callers use safe keys or no options. The exported method still allows options to overwrite payout, sold, or at. Spread options before these computed fields.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/GameServer/Bot/Population/BotLifeState.js` at line 1682, Update
applyNpcLiquidation so the options object is spread before the computed payout,
sold, and at fields, ensuring caller-provided values cannot overwrite the
computed liquidation data.
Comment on lines
+377
to
+401
| schedulePersonaBackfill() { | ||
| if (this.personaBackfillTimer) return; | ||
|
|
||
| const run = () => { | ||
| if (this.personaBackfillRunning) return; | ||
| this.personaBackfillRunning = true; | ||
| BotPersona.backfillGenerated().then((result) => { | ||
| // Only a successful short read closes this one-time migration. | ||
| // A failed write stays scheduled for a later retry. | ||
| if (result.exhausted && this.personaBackfillTimer) { | ||
| clearInterval(this.personaBackfillTimer); | ||
| this.personaBackfillTimer = null; | ||
| } | ||
| }).finally(() => { | ||
| this.personaBackfillRunning = false; | ||
| }); | ||
| }; | ||
|
|
||
| run(); | ||
| this.personaBackfillTimer = setInterval(run, 2000); | ||
| if (typeof this.personaBackfillTimer.unref === 'function') { | ||
| this.personaBackfillTimer.unref(); | ||
| } | ||
| }, | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Add error handling to the persona-backfill run loop.
BotPersona.backfillGenerated().then(...).finally(...) has no .catch(). If the returned promise rejects, .finally() does not swallow the rejection, so the chain remains unhandled. This function re-runs every 2 seconds until result.exhausted, so any transient failure (for example a database error) becomes a recurring unhandled promise rejection. In current Node.js versions, an unhandled promise rejection terminates the process by default.
The comment "A failed write stays scheduled for a later retry" states the intended behavior, but nothing here catches the failure to make that retry safe.
🛡️ Proposed fix to catch and log backfill failures
BotPersona.backfillGenerated().then((result) => {
// Only a successful short read closes this one-time migration.
// A failed write stays scheduled for a later retry.
if (result.exhausted && this.personaBackfillTimer) {
clearInterval(this.personaBackfillTimer);
this.personaBackfillTimer = null;
}
- }).finally(() => {
+ }).catch((err) => {
+ utils.infoWarn('BotPopulation', 'persona backfill failed: %s', err.message);
+ }).finally(() => {
this.personaBackfillRunning = false;
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| schedulePersonaBackfill() { | |
| if (this.personaBackfillTimer) return; | |
| const run = () => { | |
| if (this.personaBackfillRunning) return; | |
| this.personaBackfillRunning = true; | |
| BotPersona.backfillGenerated().then((result) => { | |
| // Only a successful short read closes this one-time migration. | |
| // A failed write stays scheduled for a later retry. | |
| if (result.exhausted && this.personaBackfillTimer) { | |
| clearInterval(this.personaBackfillTimer); | |
| this.personaBackfillTimer = null; | |
| } | |
| }).finally(() => { | |
| this.personaBackfillRunning = false; | |
| }); | |
| }; | |
| run(); | |
| this.personaBackfillTimer = setInterval(run, 2000); | |
| if (typeof this.personaBackfillTimer.unref === 'function') { | |
| this.personaBackfillTimer.unref(); | |
| } | |
| }, | |
| schedulePersonaBackfill() { | |
| if (this.personaBackfillTimer) return; | |
| const run = () => { | |
| if (this.personaBackfillRunning) return; | |
| this.personaBackfillRunning = true; | |
| BotPersona.backfillGenerated().then((result) => { | |
| // Only a successful short read closes this one-time migration. | |
| // A failed write stays scheduled for a later retry. | |
| if (result.exhausted && this.personaBackfillTimer) { | |
| clearInterval(this.personaBackfillTimer); | |
| this.personaBackfillTimer = null; | |
| } | |
| }).catch((err) => { | |
| utils.infoWarn('BotPopulation', 'persona backfill failed: %s', err.message); | |
| }).finally(() => { | |
| this.personaBackfillRunning = false; | |
| }); | |
| }; | |
| run(); | |
| this.personaBackfillTimer = setInterval(run, 2000); | |
| if (typeof this.personaBackfillTimer.unref === 'function') { | |
| this.personaBackfillTimer.unref(); | |
| } | |
| }, |
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 395-395: Avoid using the initial state variable in setState
Context: setInterval(run, 2000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/GameServer/Bot/Population/PopulationService.js` around lines 377 - 401,
Add a rejection handler to the promise chain in schedulePersonaBackfill so
failures from BotPersona.backfillGenerated() are caught and logged, while
leaving the timer active for the next retry; preserve the existing exhaustion
cleanup and finally-based running-state reset.
| }; | ||
| }); | ||
| })) | ||
| .then(({ states, partyWaitBacklog, partyWaitCount }) => { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Party-requirement refresh is not actually periodic.
refreshBackgroundPartyRequirements is reachable only through reclaimBackgroundPartyCapacity, and that function returns early whenever partyWaitStates is empty (line 866) or reclaimCount is 0 (line 873-874). formBackgroundParties passes [] for partyWaitStates whenever there is no party-wait backlog (line 714). As a result, active parties can go well beyond Config.partyRequirementRefreshMs without a requirement refresh, or never refresh at all if capacity reclaim rarely triggers, contradicting the periodic refresh intent behind partyRequirementRefreshMs/partyRequirementRefreshBatchSize.
Decouple the refresh from the reclaim-count gate so it runs whenever there are active parties, independent of backlog size.
♻️ Proposed fix to run refresh independent of reclaim eligibility
reclaimBackgroundPartyCapacity(partyWaitStates = [], partyWaitCount = partyWaitStates.length) {
- if (!partyWaitStates.length) return Promise.resolve([]);
const activeParties = BackgroundPartyState.active();
- const availableSlots = Math.max(0, maxBackgroundPartiesForBacklog(partyWaitCount) - activeParties.length);
- const wantedSlots = Math.min(
- Config.partyFormationBatchSize,
- Math.floor(partyWaitStates.length / Math.max(1, Config.partyMinSize))
- );
- const reclaimCount = Math.max(0, wantedSlots - availableSlots);
- if (!reclaimCount || !activeParties.length) return Promise.resolve([]);
-
- return this.refreshBackgroundPartyRequirements(activeParties)
- .then(() => LifeState.partyRequirementCounts(activeParties.map((party) => party.partyId)))
+ if (!activeParties.length) return Promise.resolve([]);
+ return this.refreshBackgroundPartyRequirements(activeParties).then(() => {
+ if (!partyWaitStates.length) return [];
+ const availableSlots = Math.max(0, maxBackgroundPartiesForBacklog(partyWaitCount) - activeParties.length);
+ const wantedSlots = Math.min(
+ Config.partyFormationBatchSize,
+ Math.floor(partyWaitStates.length / Math.max(1, Config.partyMinSize))
+ );
+ const reclaimCount = Math.max(0, wantedSlots - availableSlots);
+ if (!reclaimCount) return [];
+ return LifeState.partyRequirementCounts(activeParties.map((party) => party.partyId))
.then((counts) => {
const countByPartyId = new Map(counts.map((count) => [count.partyId, count]));
return activeParties
.filter((party) => Number(countByPartyId.get(party.partyId)?.requiredMembers || 0) === 0)
.sort((a, b) => Number(a.startedAt || 0) - Number(b.startedAt || 0))
.slice(0, reclaimCount);
})
.then((parties) => parties.reduce((chain, party) => (
chain.then((reclaimed) => dissolveBackgroundParty(party, 'party_capacity_reclaimed', party.memberIds?.length || 0)
.then(() => [...reclaimed, party]))
- ), Promise.resolve([])));
+ ), Promise.resolve([])));
+ });
},Also applies to: 714-714, 865-877
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/GameServer/Bot/Population/PopulationService.js` at line 712, Update
formBackgroundParties and reclaimBackgroundPartyCapacity so
refreshBackgroundPartyRequirements runs periodically whenever active parties
exist, regardless of partyWaitStates being empty or reclaimCount being zero.
Keep capacity-reclaim logic gated by backlog and reclaim eligibility, but
decouple the refresh path and preserve the configured partyRequirementRefreshMs
and partyRequirementRefreshBatchSize behavior.
| })); | ||
| } | ||
| if (mode === 'const' && parts[2]) return BotFriendship.toggleConst(session, parts[2]).then(() => render(session, 'friends', parts[3])); | ||
| if (mode === 'form') return BotFriendship.selected(session).then((bots) => bots.reduce((chain, bot) => chain.then(() => World.inviteFriendByName(session, session.actor, bot.characterName || bot.name, undefined, 'friend_const')), Promise.resolve()).then(() => render(session))); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard each invite in the form reduce chain against rejection.
bots.reduce((chain, bot) => chain.then(() => World.inviteFriendByName(...)), Promise.resolve()) has no .catch() per iteration. If World.inviteFriendByName rejects for one bot (for example, a DB error from LifeState.findByName or BotFriendship.isFriend), the whole chain rejects. Remaining friends in the roster are never invited, and the trailing .then(() => render(session)) never runs, so the player sees no response after clicking "Form my party."
Catch per-iteration failures so one bot's failure does not block the rest of the roster or suppress the final render.
🛠️ Proposed fix
- if (mode === 'form') return BotFriendship.selected(session).then((bots) => bots.reduce((chain, bot) => chain.then(() => World.inviteFriendByName(session, session.actor, bot.characterName || bot.name, undefined, 'friend_const')), Promise.resolve()).then(() => render(session)));
+ if (mode === 'form') return BotFriendship.selected(session).then((bots) => bots.reduce((chain, bot) => chain.then(() => World.inviteFriendByName(session, session.actor, bot.characterName || bot.name, undefined, 'friend_const').catch(() => false)), Promise.resolve()).then(() => render(session)));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (mode === 'form') return BotFriendship.selected(session).then((bots) => bots.reduce((chain, bot) => chain.then(() => World.inviteFriendByName(session, session.actor, bot.characterName || bot.name, undefined, 'friend_const')), Promise.resolve()).then(() => render(session))); | |
| if (mode === 'form') return BotFriendship.selected(session).then((bots) => bots.reduce((chain, bot) => chain.then(() => World.inviteFriendByName(session, session.actor, bot.characterName || bot.name, undefined, 'friend_const').catch(() => false)), Promise.resolve()).then(() => render(session))); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/GameServer/World/Generics/NpcBypasses/BotFriends.js` at line 71, Update
the form-mode invite chain in BotFriendship.selected and its bots.reduce
callback to catch and absorb each World.inviteFriendByName rejection before the
next bot runs. Preserve sequential invitation processing and ensure the final
render(session) executes after all bots, even when individual invites fail.
| assert.strictEqual(insert.params[4], persona.archetype, 'archetype must be queryable without parsing traits'); | ||
| const originalEnsure = BotPersona.ensure; | ||
| BotPersona.reset(); | ||
| Database.execute = ([sql, params]) => { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the unused params destructure.
ESLint flags params as unused in this callback (Line 63). Unlike the earlier mock (Line 45), this callback does not read params.
🧹 Proposed fix
- Database.execute = ([sql, params]) => {
+ Database.execute = ([sql]) => {
if (String(sql).startsWith('SELECT 1')) return Promise.resolve([]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Database.execute = ([sql, params]) => { | |
| Database.execute = ([sql]) => { |
🧰 Tools
🪛 ESLint
[error] 63-63: 'params' is defined but never used.
(no-unused-vars)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_bot_persona.js` at line 63, Remove the unused params destructuring
from the Database.execute callback in the test, retaining only the sql argument
needed by the callback.
Source: Linters/SAST tools
This was referenced Aug 1, 2026
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
1 participant
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.
Summary
This PR adds the first complete persona-driven layer for simulated player bots and connects it to party behavior, dialogue, friendship, and the in-game economy.
Bot personas and behavior
Wealth-driven economy
wealthprimary drive.Bot friendship and const parties
Persistence, lifecycle, and coverage
Validation
node scripts/run-tests.js(full suite)git diff --checkThe full suite passes.