Bot pickup must pre-check inventory space (full pack → failed pickup, not item loss) #52

New issue

Closed

opened 2026-06-28 15:14:10 +00:00 by sickday · 0 comments

sickday commented

2026-06-28 15:14:10 +00:00

Owner

Problem

BotHandlers.pickup_at_tile/_ removes the ground item via GroundItems.pickup/3 and then calls Container.add/3. If the bot's backpack is full, the add fails after the item has already left the ground — the item is silently lost. The player pickup path (Actions.do_pickup/4) pre-checks with Container.has_space?/2, so it never reaches this state.

This is item-loss, not a dupe, and only affects bots, so stakes are low — but it quietly drains items out of the economy pool.

Task

Pre-check space before removing from the ground (mirror do_pickup/4's Container.has_space?/2 guard) so a full pack results in a failed pickup with the item left on the ground, not a pickup-then-loss. Do not add a pickup-then-redrop path — the correct outcome is a clean failed pickup.

Acceptance

A bot attempting to pick up with a full inventory leaves the ground item untouched.
Regression test: full-pack bot pickup is a no-op on both the backpack and the ground tile.

Refs: lib/black_lobster/game/entity/player/bot_handlers.ex (pickup_at_tile), cf. lib/black_lobster/game/entity/player/actions.ex do_pickup.

Part of the Security & Data-Integrity Hardening epic.

## Problem `BotHandlers.pickup_at_tile/_` removes the ground item via `GroundItems.pickup/3` and **then** calls `Container.add/3`. If the bot's backpack is full, the add fails *after* the item has already left the ground — the item is silently lost. The player pickup path (`Actions.do_pickup/4`) pre-checks with `Container.has_space?/2`, so it never reaches this state. This is item-loss, not a dupe, and only affects bots, so stakes are low — but it quietly drains items out of the economy pool. ## Task Pre-check space **before** removing from the ground (mirror `do_pickup/4`'s `Container.has_space?/2` guard) so a full pack results in a **failed pickup with the item left on the ground**, not a pickup-then-loss. Do **not** add a pickup-then-redrop path — the correct outcome is a clean failed pickup. ## Acceptance - A bot attempting to pick up with a full inventory leaves the ground item untouched. - Regression test: full-pack bot pickup is a no-op on both the backpack and the ground tile. Refs: `lib/black_lobster/game/entity/player/bot_handlers.ex` (`pickup_at_tile`), cf. `lib/black_lobster/game/entity/player/actions.ex` `do_pickup`. Part of the Security & Data-Integrity Hardening epic.

sickday added the

bug

epic:security-hardening

priority:low

server

labels

2026-06-28 15:14:25 +00:00

sickday self-assigned this

2026-06-28 15:14:46 +00:00

sickday added this to the v1 project

2026-06-28 15:14:48 +00:00

sickday added this to the v1 milestone