Add PvP bots for Clan Wars Free-For-All#967
Open
HarleyGilpin wants to merge 69 commits intoGregHib:mainfrom
Open
Add PvP bots for Clan Wars Free-For-All#967HarleyGilpin wants to merge 69 commits intoGregHib:mainfrom
HarleyGilpin wants to merge 69 commits intoGregHib:mainfrom
Conversation
…ing and ice blitz tactics
Add KDoc on Bot.pinned and Bot.refresh listing the three BotManager call sites and why the normal available/previous pick/resolve path can't replace them for single-role bots like PvP clan-war tiers.
BotRole is too generic; the detection heuristic is clan-war tier-specific (pure/tank/hybrid/healer thresholds), so scope the name accordingly to leave room for other-activity roles later. Also rename the TOML condition keyword "role" to "clan_war_role"; no existing TOML uses it.
Each template only listed its primary training skill in produces. Since produces keys reset the frame timeout on matching XP/events, hybrids and ranged/magic builds could time out mid-fight when the only XP gained was in an unlisted skill (e.g. dharoker earning attack XP, tribrid earning defence/constitution). List every realistic XP source per tier so the timeout only fires when the bot is truly idle.
Reviewer flagged the uniform { restart = { success = { area = … } } }
block as potentially unbounded. Document the three signals that actually
bound the loop — area exit, death, activity timeout — at the top of the
file so the next reader doesn't re-raise the same concern.
Arena definitions (spawn area + tier list) and per-tier spawn levels + combat style were hardcoded in BotCommands.kt. Move them to data/bot/clan_wars_arenas.tables.toml and clan_wars_tiers.tables.toml following the project's existing tables pattern. Adding a new arena or tuning spawn levels no longer requires a code change.
BotCombatContextBuilder was rescanning a 31x31 tile square per bot per tick for any activity with a reactive entry, even though most reactive checks only read the cheap fields (incoming attacker style, own HP). Build the cheap shell eagerly and defer the spiral scan behind lazy getters on nearbyEnemies / nearbyAllies / enemiesByTile; consumers (BotOutmatched, BotFightPlayer, BotCastSpell) trigger it only when actually needed.
list<skill> is stored as List<Int> (skill ordinals) because the skill column type resolves to ReaderEntity. stringList() then cast-lies due to generic erasure and blew up at runtime in loadPvpArenas on the String destructure. Use list<string> so the names are read back as names.
Tier-0 instrumentation so we can measure BotManager cost before doing any optimization. BotMetrics is off by default (one volatile read per tick when idle); the /bot_stress admin command flips it on for N ticks (default 500), with optional warmup delay, then prints a multi-line report with avg/p50/p95/p99/max for managerRun (ms) and per-bot tick (us) plus spiral-scan / target-pick counters. Report goes to the bot logger and back to the invoking player as console chat lines.
Drops always land on the deceased's exact tile (PlayerDeath.kt drop() calls FloorItems.add(tile, ...) with no spreading or offsets), so walking a 21x21 (or 25x25) Spiral every loot tick was wasted work. Capture the deceased's tile.id alongside the loot_pending clock when a bot kills a player. takeLoot now reads that pointer and inspects only FloorItems.at(dropTile). Pathing to the drop is unchanged — the existing InteractFloorItem instruction handles movement. Both the clock and the tile pointer are cleared together when the lookup returns nothing eligible. Most-recent-kill wins on a streak; older drops linger under their own disappearTicks and may be missed. Acceptable trade-off vs queueing multiple kill tiles per bot.
The post-death bot handler unconditionally set dropItems=false to keep applyTier from duplicating the tier kit on the floor. That meant a killer in the dangerous FFA arena got nothing for the kill. Skip the override when the bot died in a dangerous arena (tier id starts with clan_wars_ffa_dangerous_ or death tile lies in clan_wars_ffa_dangerous_arena). The engine then drops their inventory and equipment for the killer; applyTier still re-equips the bot fresh on respawn. Safe-arena bots and any other PvP-bot configurations still keep their items as before.
The dangerous FFA arena is meant to play like wilderness PvP — kills should hand the loot directly to the killer, not stash it under a gravestone. Today the arena is not flagged as wilderness, so deaths fell through to Gravestone.spawn and items sat hidden under a grave the killer couldn't loot. Treat clan_wars_ffa_dangerous_arena as a PvP drop zone for the gravestone branch and the per-item drop routing: when killed by a Player there, time = 0 (no grave) and items drop with revealTicks / disappearTicks tuned for the killer-owned wilderness path. Affects both human and bot deaths in the dangerous arena. Safe arena, corporeal beast lair, and the standard PvE death path are unchanged.
Adds Character.inFullPvp — wilderness OR clan_wars_ffa_dangerous_arena — and routes the three places that gate wilderness-style death consequences through it: - Skull.kt combatStart: attacking another Player inside the dangerous arena now applies the 10-minute skull, just like wilderness. - ItemsKeptOnDeath.kt: Wilderness-tagged items (ItemKept.Wilderness) drop in the dangerous arena instead of being kept. - PlayerDeath.kt: dropItems uses inFullPvp directly (replaces the inline tile-in-areas check added in d8031b5a1) and drops the now- unused inWilderness parameter from the dropItems signature. Net effect for a typical fight in the dangerous arena: attacker gets skulled on first engagement (save = 0), dies later, drops everything to the killer. Protect-item prayer keeps one item. Wilderness, FFA safe arena, corp lair, generic PvE death are unchanged.
Revert the auto-skull trigger in the dangerous arena (Skull.kt combatStart re-gates on inWilderness only) so players don't get the visible 10-minute skull icon for fighting there. Apply skull-equivalent drop rules instead inside ItemsKeptOnDeath: when the deceased's tile is in clan_wars_ffa_dangerous_arena, save count starts at 0 just like a real skull would force. Protect-item prayer still adds 1, so the kept-item table matches the user's spec: Protect_item off: drop everything. Protect_item on: keep top 1 item. Wilderness behaviour is unchanged — skulled there still drops all, unskulled keeps 3, and the existing skull mechanic still grants the overhead icon when wilderness combat starts.
The old retreat action walked the bot to the lobby tile via pathfind, which never worked in the FFA arenas — they're sealed by walls and the only exit is the south portal. Replace the in-template retreat with a per-instance portal-leave action on dangerous-arena bots only. - New BotPvpRetreatNeeded condition: true only when player is in a single-combat tile and has no items with the Eat option. Multi-combat half is excluded (you'd die before reaching the portal). - BotInteractObject gains an optional `if` gate (parser + start/update short-circuit to Success when the condition is false), so any object action can self-skip without a wrapper. - pvp_combat.templates.toml: drop the broken retreat block from all 8 PvP templates. - clan_wars.bots.toml: prepend a portal-leave object action to each of the 8 dangerous instances. Fragment.resolveActions runs prepended instance actions before template actions, so retreat is evaluated first each restart cycle. Safe-arena bots have no retreat at all. After exit the bot teleports to clan_wars_teleport; the pinned respawn cycle clears blocked, refresh re-applies the tier (full kit + food) and the existing enter_clan_wars_ffa_dangerous resolver walks them back through the entry portal.
5 new BotPvpRetreatNeededTest cases (food present/absent/empty inv, multi-combat short-circuit) and 2 new BotInteractObjectTest cases for the new optional `if` parameter (skips when false, runs when true).
Replace the broken portal-walk retreat with an instant games-necklace
teleport. Each PvP tier carries a games_necklace_6 in the kit
inventory; dangerous-arena bots equip it and click the worn-slot
"Clan Wars" option when out of food in single combat.
- BotInterfaceOption gains an optional `if` gate (parser + first-line
short-circuit to Success), mirroring the BotInteractObject change.
- pvp_combat.templates.toml: add games_necklace_6 (equippable) to all
8 tier inventories. applyTier re-fills it on each respawn cycle.
- clan_wars.bots.toml: dangerous instances now run two prepended
interface actions gated by pvp_retreat_needed:
1. Wear games_necklace_6 from inventory.
2. Click "Clan Wars" on worn_equipment:amulet_slot:games_necklace_6;
success when in clan_wars_teleport area.
The amulet swap costs the bot their fury / berserker necklace mid-
retreat, but they're escaping anyway and the kit is replaced on the
next respawn / refresh cycle.
Hook the entered("clan_wars_teleport") event in BotCommands and emit
an info-level pvpLogger line when a tracked PvP bot whose tier id
starts with clan_wars_ffa_dangerous_ enters that area. Fires both on
initial spawn (bots spawn at clan_wars_teleport) and on a games-
necklace retreat, so a successful retreat shows up as a second log
entry per bot per cycle.
Death sequence in PlayerDeath.kt sets dead=true, then teles the
deceased to onDeath.teleport (clan_wars_teleport for the dangerous
arena), then clears dead. The entered("clan_wars_teleport") hook fired
during that tele, producing a false "PvP bot retreat" log on every
death. Skip the log when player.dead is still true; only genuine
games-necklace retreats now print.
…ing interface interactions
…me public, so bots don't become loot piñatas when bots kill each other.
Drops always land on the deceased's exact tile (PlayerDeath.kt drop() calls FloorItems.add(tile, ...) with no spreading or offsets), so walking a 21x21 (or 25x25) Spiral every loot tick was wasted work. Capture the deceased's tile.id alongside the loot_pending clock when a bot kills a player. takeLoot now reads that pointer and inspects only FloorItems.at(dropTile). Pathing to the drop is unchanged — the existing InteractFloorItem instruction handles movement. Both the clock and the tile pointer are cleared together when the lookup returns nothing eligible. Most-recent-kill wins on a streak; older drops linger under their own disappearTicks and may be missed. Acceptable trade-off vs queueing multiple kill tiles per bot.
GregHib
reviewed
May 1, 2026
Owner
GregHib
left a comment
GregHib
left a comment
There was a problem hiding this comment.
First pass.
I wanted to flag upfront that I have mixed feelings about the approach and would value your thoughts.
The longer-term goal is to extend this to open wilderness bots and to populate other minigames like PC. Right now, the behavior for each combat bot is driven by configs in data/bot/pvp.combat.templates.toml, which has gotten a bit complex. I suspect there's a cleaner way to architect this, so I'm genuinely open to any criticism or suggestions you have.
I think minigame_combat.templates.toml just needs splitting up, maybe into pvp styles?
I think for now it's okay and these larger complexity issues don't need addressing in this PR as some of them are quite fundamental to the bot system as a whole.
There's a few things the bot system could do with which this work highlights:
- Reusable action templates could tidy it up i.e. not having to write
{ pray = { id = "deflect_melee", if = { attacker_style = { equals = "melee" } } } },in full everytime. - A proper target finding system (there's probably some cool things we could do with influence mapping)
- A gear/loadout best-fit kind of system or extension on the current (like what you've started with any but for setup)
| [clan_wars_tiers] | ||
| row_id = "string" | ||
| combat_style = "string" | ||
| skills = "list<string>" |
Owner
There was a problem hiding this comment.
Suggested change
| skills = "list<string>" | |
| skills = "list<skill>" |
Comment on lines
+96
to
+103
| val skillNames = stringList("skills") | ||
| val values = intList("levels") | ||
| require(skillNames.size == values.size) { "clan_wars_tiers.$rowId: skills/levels size mismatch." } | ||
| val levels = LinkedHashMap<Skill, Int>(skillNames.size) | ||
| for ((index, name) in skillNames.withIndex()) { | ||
| val skillId = Skill.map[name] ?: error("clan_wars_tiers.$rowId: unknown skill '$name'.") | ||
| levels[Skill.all[skillId]] = values[index] | ||
| } |
Owner
There was a problem hiding this comment.
Suggested change
| val skillNames = stringList("skills") | |
| val values = intList("levels") | |
| require(skillNames.size == values.size) { "clan_wars_tiers.$rowId: skills/levels size mismatch." } | |
| val levels = LinkedHashMap<Skill, Int>(skillNames.size) | |
| for ((index, name) in skillNames.withIndex()) { | |
| val skillId = Skill.map[name] ?: error("clan_wars_tiers.$rowId: unknown skill '$name'.") | |
| levels[Skill.all[skillId]] = values[index] | |
| } | |
| val skills = skillList("skills") | |
| val values = intList("levels") | |
| require(skills.size == values.size) { "clan_wars_tiers.$rowId: skills/levels size mismatch." } | |
| val levels = LinkedHashMap<Skill, Int>(skills.size) | |
| for ((index, skill) in skills.withIndex()) { | |
| levels[skill] = values[index] | |
| } |
| ] | ||
|
|
||
| [enter_clan_wars_ffa_safe] | ||
| weight = -10 |
Owner
There was a problem hiding this comment.
Don't think the weight needs to be negative, 0 would be fine?
| * Dramatically cheaper than calling [at] per-tile in a spiral: one zone lookup per overlapping | ||
| * zone (≤ 25 for radius 15) versus 961 tile lookups each scanning the full zone. | ||
| */ | ||
| inline fun forEachInRadius(center: Tile, radius: Int, action: (Player) -> Unit) { |
Owner
There was a problem hiding this comment.
This doesn't need to be inline? and indexArray/map can stay private
Comment on lines
+83
to
+99
| val minZx = (center.x - radius) shr 3 | ||
| val maxZx = (center.x + radius) shr 3 | ||
| val minZy = (center.y - radius) shr 3 | ||
| val maxZy = (center.y + radius) shr 3 | ||
| val level = center.level | ||
| for (zx in minZx..maxZx) { | ||
| for (zy in minZy..maxZy) { | ||
| map.onEach(Zone.id(zx, zy, level)) { index -> | ||
| val player = indexArray[index] ?: return@onEach | ||
| val dx = player.tile.x - center.x | ||
| if (dx < -radius || dx > radius) return@onEach | ||
| val dy = player.tile.y - center.y | ||
| if (dy < -radius || dy > radius) return@onEach | ||
| action(player) | ||
| } | ||
| } | ||
| } |
Owner
There was a problem hiding this comment.
(Double check this behaviour)
Suggested change
| val minZx = (center.x - radius) shr 3 | |
| val maxZx = (center.x + radius) shr 3 | |
| val minZy = (center.y - radius) shr 3 | |
| val maxZy = (center.y + radius) shr 3 | |
| val level = center.level | |
| for (zx in minZx..maxZx) { | |
| for (zy in minZy..maxZy) { | |
| map.onEach(Zone.id(zx, zy, level)) { index -> | |
| val player = indexArray[index] ?: return@onEach | |
| val dx = player.tile.x - center.x | |
| if (dx < -radius || dx > radius) return@onEach | |
| val dy = player.tile.y - center.y | |
| if (dy < -radius || dy > radius) return@onEach | |
| action(player) | |
| } | |
| } | |
| } | |
| val level = center.level | |
| val area = center.toCuboid(radius) | |
| for (zone in center.toCuboid(radius).toZones(level)) { | |
| map.onEach(zone) { index -> | |
| val player = indexArray[index] ?: return@onEach | |
| if (player.tile !in area) { | |
| return@onEach | |
| } | |
| action(player) | |
| } | |
| } |
Comment on lines
+114
to
+119
| val dx = (bot.tile.x - target.tile.x).coerceIn(-1, 1) | ||
| val dy = (bot.tile.y - target.tile.y).coerceIn(-1, 1) | ||
| if (dx == 0 && dy == 0) return | ||
| val kx = bot.tile.x + dx * 2 | ||
| val ky = bot.tile.y + dy * 2 | ||
| val dest = Tile(kx, ky, bot.tile.level) |
Owner
There was a problem hiding this comment.
(Double check the logic)
Suggested change
| val dx = (bot.tile.x - target.tile.x).coerceIn(-1, 1) | |
| val dy = (bot.tile.y - target.tile.y).coerceIn(-1, 1) | |
| if (dx == 0 && dy == 0) return | |
| val kx = bot.tile.x + dx * 2 | |
| val ky = bot.tile.y + dy * 2 | |
| val dest = Tile(kx, ky, bot.tile.level) | |
| val dir = bot.tile.delta(target.tile).toDirection() | |
| if (dir == Direction.None) { | |
| return | |
| } | |
| val dest = bot.tile.add(dir).add(dir) |
Comment on lines
+9
to
+29
| enum class BotLootStrategy { | ||
| /** Loot any owned item above the value threshold, in spiral order. */ | ||
| DEFAULT, | ||
|
|
||
| /** Only loot consumables (food, potions, prayer restores). */ | ||
| SURVIVAL, | ||
|
|
||
| /** Loot any owned item above the value threshold, picking the highest total value first. */ | ||
| WEALTH; | ||
|
|
||
| /** Whether [item] is eligible to be looted under this strategy. */ | ||
| fun accepts(item: FloorItem): Boolean = when (this) { | ||
| SURVIVAL -> isConsumable(item) | ||
| DEFAULT, WEALTH -> true | ||
| } | ||
|
|
||
| /** Whether candidates should be gathered and ranked instead of taking the first match. */ | ||
| fun ranks(): Boolean = this == WEALTH | ||
|
|
||
| /** Total coin value to compare candidates by; higher is better. */ | ||
| fun score(item: FloorItem): Long = item.value |
Owner
There was a problem hiding this comment.
I wonder if it's worth making looting it's own activity. On one hand it would keep the code duplication down and remove it from the combat actions, on the other hand having it switch from fighting to looting after the target is killed would prevent survival looting of nearby (though this could also be a separate reactive action?) wdyt?
| import world.gregs.voidps.engine.entity.character.player.equip.equipped | ||
| import world.gregs.voidps.network.login.protocol.visual.update.player.EquipSlot | ||
|
|
||
| data class BotTargetArmorType(val equals: Set<String>) : Condition(1) { |
Owner
There was a problem hiding this comment.
For consistency 😜
Suggested change
| data class BotTargetArmorType(val equals: Set<String>) : Condition(1) { | |
| data class BotTargetArmourType(val equals: Set<String>) : Condition(1) { |
| val id = item.def.stringId | ||
| if (id == "shark" || id.startsWith("saradomin_brew_")) { | ||
| count += item.amount | ||
| } |
Owner
There was a problem hiding this comment.
Maybe a data table with different options? Might be a good idea to reuse the logic when finding the index of a healing item to consume as well (and support drinking to heal not just "Eat")
GregHib
reviewed
May 1, 2026
GregHib
reviewed
May 1, 2026
…bled clan wars FFA bot spawns by default.
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
2 participants
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
Adds bots that fight each other in the Clan Wars Free-For-All arenas
(Safe and Dangerous). Bots use prayers, switch styles, drink potions,
eat food, cast specs, and retreat when low. Dangerous arena follows
wilderness PvP rules.
What's included
potion drinking, food eating, anti-clump movement
kits drop, no gravestone
become loot piñatas
recorded-tile loot pickup, and a
/bot_stressadmin command forbenchmarking
Configuration (
game/src/main/resources/game.properties)bots.pvp.clan_wars_ffa_safe.count, bots in Safe arenabots.pvp.clan_wars_ffa_dangerous.count, bots in Dangerous arenabots.pvp.clan_wars_ffa_*.spawnSeconds, spawn cadenceSet either count to
0to disable that arena's bots.Test plan
./gradlew :game:test :engine:testpasses./gradlew spotlessCheckpassesfighting. Die and confirm no items drop on death.
rules apply, kit drops on death, no gravestone.
nearby; loot from the loser should not visible.
vengeance off cooldown, switch to magic when kited.
/bot_stresswith 1,000 bots and confirmed BotMetrics performance is appropriate.Notes for reviewer
I wanted to flag upfront that I have mixed feelings about the approach and would value your thoughts.
The longer-term goal is to extend this to open wilderness bots and to populate other minigames like PC. Right now, the behavior for each combat bot is driven by configs in data/bot/pvp.combat.templates.toml, which has gotten a bit complex. I suspect there's a cleaner way to architect this, so I'm genuinely open to any criticism or suggestions you have.
No rush on the review. I'd rather take the time to get it right than ship something we'll want to redo later.