From 0966cbb8d78f85ea8abb1b5f4205205bfd7972c6 Mon Sep 17 00:00:00 2001 From: Marsh Macy Date: Fri, 30 Oct 2026 21:36:09 -0700 Subject: [PATCH 1/3] Add the quest-item guard contract for RemoveMember and its acceptance tests In a playtest of a game built on osrlib, the player removed a member who carried the music box the main quest needed, without give_to. The box left the game with the member, and the quest could no longer be finished. osrlib knows which items an adventure depends on, so the session will refuse that removal, and every game gets the check. This commit adds the contract and the tests. The handler doesn't emit the code yet, so the chunk's tests are marked xfail for chunk: quest-item-guard. The contract: - RemoveMember without give_to is refused with party.remove.carries_quest_item when the leaving member carries an item the adventure depends on. With give_to, the item moves to that member, so the command isn't refused. - The adventure depends on an item when its catalog id is named by a HasItemCondition or an ItemAcquiredPattern anywhere in the adventure. The sites are the gate of a door or a transition, a trigger's pattern and conditions, and a quest's activation, each objective's when, and each objective's reveal_when. - Ids match the way has_item matches them. Mundane items, magic items, and the adventure's own items count, equipped or in the pack. A spent stack doesn't count. The member can be alive, dead, turned to stone, or asleep. Another member who carries the same item doesn't lift the refusal. - A completed quest protects nothing, and neither does a trigger that has fired and isn't repeatable. A quest that is still open protects the items in all its clauses, even an objective that is already complete. A gate always protects its item. - The check is the last of the RemoveMember refusals. Its params are the member's id as character and, as item, the catalog id of the first such item in Inventory.all_instances order. - A refusal costs nothing: no draws, no time, no log entry, and no change. The RemoveMember docstring lists the code last in its Rejections section and says when to use give_to. The rejection-code docs gate is marked xfail until the handler emits the code. The tests of removals the check must still allow pass today and carry no marker. Claude-Session: https://claude.ai/code/session_01V7ErtnsmhaP7aLwcfmzohD --- src/osrlib/crawl/commands.py | 26 +- tests/test_docs_gates.py | 2 + tests/test_remove_member_quest_items.py | 372 ++++++++++++++++++++++++ 3 files changed, 399 insertions(+), 1 deletion(-) create mode 100644 tests/test_remove_member_quest_items.py diff --git a/src/osrlib/crawl/commands.py b/src/osrlib/crawl/commands.py index 00b928d..009cf5a 100644 --- a/src/osrlib/crawl/commands.py +++ b/src/osrlib/crawl/commands.py @@ -3284,6 +3284,24 @@ class RemoveMember(Command): new owner can drop it or give it away. If they put it on, the curse is revealed again. Without `give_to`, everything the member carries leaves the game with them. + Without `give_to`, the session refuses to remove a member who carries an item the adventure + depends on, because the party might then never open a door or finish a quest. The adventure + depends on an item when its catalog id appears in a + [`HasItemCondition`][osrlib.crawl.gates.HasItemCondition] or an + [`ItemAcquiredPattern`][osrlib.crawl.triggers.ItemAcquiredPattern] anywhere in the adventure. + That means the gate of a door, a stair, or a passage, a trigger, or any clause of a quest. The + session matches ids the way a `HasItemCondition` does. Mundane items, magic items, and the + adventure's own items all count, equipped or not. An item counts even when another member + carries the same item too. When the session refuses, send the command again and name a + member in `give_to` to take everything, the item included. + + Some parts of an adventure can't matter again, and they stop protecting their items. A quest + that [`GameSession.quests`][osrlib.crawl.session.GameSession.quests] shows as `completed` + protects nothing. A trigger protects nothing once it has fired, which means its id is in + `GameSession.fired_triggers`, unless it's `repeatable`. A quest that is still open protects + the items in all its clauses, even an objective that is already complete. A gate always + protects its item, because the party can come back to it. + The session also drops what it tracks about the member. The effects on them end without events, and their death record, their food and water counts, and the record of when they last prepared spells are deleted. An effect the member cast on another member lasts its full @@ -3321,6 +3339,11 @@ class RemoveMember(Command): they're dead, asleep, paralysed, or turned to stone. - `party.remove.last_standing` - no member would be left standing. That includes removing the only member. + - `party.remove.carries_quest_item` - `give_to` is `None`, and the member carries an + item the adventure depends on. The rejection includes the member's id as `character` + and the item's catalog id as `item`. When the member carries more than one such item, + it names the first in the order that + [`Inventory.all_instances`][osrlib.core.items.Inventory.all_instances] lists them. The session checks in this order and reports the first that fails. @@ -3341,7 +3364,8 @@ class RemoveMember(Command): give_to: str | None = None """The member who takes everything the leaving member carries, by [`MemberView.id`][osrlib.crawl.views.MemberView.id]. Leave it `None` to let it all leave the - game with the member. The member named here has to be able to act.""" + game with the member. The session refuses `None` when the member carries an item the + adventure depends on. The member named here has to be able to act.""" class AwardXP(Command): diff --git a/tests/test_docs_gates.py b/tests/test_docs_gates.py index 78f5650..fca25fc 100644 --- a/tests/test_docs_gates.py +++ b/tests/test_docs_gates.py @@ -158,6 +158,8 @@ def test_documented_modes_equal_allowed_modes(self): wrong.append(f"{cls.__name__}: documents {sorted(documented)}, allows {sorted(actual)}") assert not wrong, "\n".join(wrong) + # `RemoveMember` documents `party.remove.carries_quest_item`, which its handler doesn't emit yet. + @pytest.mark.xfail(reason="chunk: quest-item-guard", strict=True) def test_documented_rejection_codes_exist_in_the_source(self, vocabulary): wrong = [] for cls in self._commands(): diff --git a/tests/test_remove_member_quest_items.py b/tests/test_remove_member_quest_items.py new file mode 100644 index 0000000..d08f6c3 --- /dev/null +++ b/tests/test_remove_member_quest_items.py @@ -0,0 +1,372 @@ +"""Acceptance tests for `party.remove.carries_quest_item`, the `RemoveMember` refusal that keeps quest items in play. + +In a playtest of a game built on osrlib, the player removed a member who carried the music +box the main quest needed, without `give_to`. The box left the game with the member, and the +quest could no longer be finished. osrlib knows which items an adventure depends on, so the +session refuses that removal, and every game gets the check. + +The contract these tests state: + +- `RemoveMember` without `give_to` is refused with `party.remove.carries_quest_item` when the + leaving member carries an item the loaded adventure depends on. With `give_to`, the item + moves to that member, so the command isn't refused. +- The adventure depends on an item when its catalog id is named by a `HasItemCondition` or an + `ItemAcquiredPattern` anywhere in it: the gate of a door or a transition, a trigger's + pattern or conditions, and a quest's activation, an objective's `when`, or an objective's + `reveal_when`, pattern or conditions alike. +- Ids match the way `has_item` matches them: mundane, magic, and the adventure's own items, + equipped or in the pack, and never a spent stack. The member can be alive, dead, turned to + stone, or asleep. Another member carrying the same item doesn't lift the refusal. +- A quest whose status is `completed` stops protecting its items, and so does a trigger that + has fired and isn't `repeatable`. A quest that is still open protects the items of every + clause, including an objective already complete. A gate always protects its item. +- The check comes last, after every other `RemoveMember` refusal. Its params are the member's + id as `character` and the catalog id of the first such item in `Inventory.all_instances` + order as `item`. +- A refusal costs nothing: no draws, no time, no log entry, and no change. + +The tests in `TestRefused` fail until the session emits the code, so they carry the chunk's +marker. The tests in `TestAllowed` pass already and have to keep passing, because they state +the removals the check must not refuse. +""" + +from typing import Any + +import pytest + +from crawl_fixtures import GATE_KEY, build_party +from osrlib.core.items import MagicItemInstance +from osrlib.crawl.adventure import Adventure +from osrlib.crawl.commands import ( + ActivateQuest, + CompleteObjective, + CompleteQuest, + EquipItem, + GrantItem, + MarkTriggerFired, + RemoveMember, + SessionMode, +) +from osrlib.crawl.dungeon import Direction, edge_key +from osrlib.crawl.gates import GateSpec, HasItemCondition +from osrlib.crawl.quests import ObjectiveSpec, QuestSpec, TriggerClause +from osrlib.crawl.session import GameSession +from osrlib.crawl.triggers import ItemAcquiredPattern, TownEnteredPattern, TriggerSpec +from test_add_member import SEED, frozen_state, member_ids, quiet_adventure, stream_states +from test_remove_member import LEAD, THIEF, granted_magic, put_in_state, remove + +CHUNK = pytest.mark.xfail(reason="chunk: quest-item-guard", strict=True) + +CODE = "party.remove.carries_quest_item" + +QUEST = "errand" +"""The id of the quest each quest site builds.""" + +TRIGGER = "watch" +"""The id of the trigger each trigger site builds.""" + +SITES = ( + "door_gate", + "stair_gate", + "toll_gate", + "trigger_pattern", + "trigger_condition", + "activation_pattern", + "activation_condition", + "objective_pattern", + "objective_condition", + "reveal_pattern", + "reveal_condition", +) +"""Every place in an adventure where a `HasItemCondition` or an `ItemAcquiredPattern` can name an item. + +A trap's transition can carry no gate, so it isn't a site. +""" + + +# ---------------------------------------------------------------------- fixtures + + +def _clause(item_id: str, kind: str) -> TriggerClause: + """A clause that names `item_id` in its pattern or in its conditions.""" + if kind == "pattern": + return TriggerClause(pattern=ItemAcquiredPattern(item_id=item_id)) + return TriggerClause(pattern=TownEnteredPattern(), conditions=(HasItemCondition(item_id=item_id),)) + + +def _quest(site: str, item_id: str) -> QuestSpec: + """A quest that names `item_id` at one of its clause sites.""" + home = ObjectiveSpec(id="home", name="Come home", when=TriggerClause(pattern=TownEnteredPattern())) + place, kind = site.split("_") + if place == "activation": + return QuestSpec(id=QUEST, name="The errand", activation=_clause(item_id, kind), objectives=(home,)) + if place == "objective": + find = ObjectiveSpec(id="find", name="Find it", when=_clause(item_id, kind)) + return QuestSpec(id=QUEST, name="The errand", objectives=(find, home)) + hidden = ObjectiveSpec( + id="find", + name="Find it", + when=TriggerClause(pattern=TownEnteredPattern()), + hidden=True, + reveal_when=_clause(item_id, kind), + ) + return QuestSpec(id=QUEST, name="The errand", objectives=(hidden, home)) + + +def adventure_naming(*named: tuple[str, str], repeatable: bool = False) -> Adventure: + """The quiet delve, with each `(site, item_id)` pair naming its item at that site. + + The adventure bundles the brass key, so an authored item can be named too. A trigger site + builds one trigger, `repeatable` or not, and a quest site builds one quest. Each site can + appear once. + + Args: + named: The sites and the item each names. + repeatable: Whether the trigger a trigger site builds is repeatable. + """ + doc: dict[str, Any] = quiet_adventure().model_dump(mode="json") + doc["items"] = [GATE_KEY.model_dump(mode="json")] + level = doc["dungeons"][0]["levels"][0] + triggers = [] + quests = [] + for site, item_id in named: + if site == "door_gate": + # The locked closet door south of (4, 1). + gate = GateSpec(condition=HasItemCondition(item_id=item_id)) + level["edges"][edge_key((4, 1), Direction.SOUTH)]["door"]["requires"] = gate.model_dump(mode="json") + elif site in ("stair_gate", "toll_gate"): + # The stair down at (4, 1). + condition = HasItemCondition(item_id=item_id, consumes=site == "toll_gate") + level["transitions"][0]["requires"] = GateSpec(condition=condition).model_dump(mode="json") + elif site.startswith("trigger_"): + clause = _clause(item_id, site.removeprefix("trigger_")) + trigger = TriggerSpec(id=TRIGGER, when=clause.pattern, conditions=clause.conditions, repeatable=repeatable) + triggers.append(trigger.model_dump(mode="json")) + else: + quests.append(_quest(site, item_id).model_dump(mode="json")) + doc["triggers"] = triggers + doc["quests"] = quests + return Adventure.model_validate(doc) + + +def session_naming(*named: tuple[str, str], repeatable: bool = False) -> GameSession: + """A session in town with the stock party, playing `adventure_naming(*named)` with no listener.""" + return GameSession.new(build_party(), adventure_naming(*named, repeatable=repeatable), seed=SEED) + + +def grant(session: GameSession, character_id: str, item_id: str, *, equip: bool = False) -> None: + """Give a member one of an item, and equip it when `equip` is set.""" + result = session.execute(GrantItem(character_id=character_id, item_id=item_id)) + assert result.accepted, result.rejections + if equip: + held = session.member(character_id).inventory.carried_item(item_id) + assert held is not None + target = held.instance_id if isinstance(held, MagicItemInstance) else item_id + assert session.execute(EquipItem(character_id=character_id, item_id=target)).accepted + assert held in session.member(character_id).inventory.equipped_instances() + + +def assert_guarded(session: GameSession, character_id: str, item_id: str) -> None: + """Removing the member without `give_to` is refused for `item_id`, at no cost.""" + state = frozen_state(session) + streams = stream_states(session) + rounds = session.clock.rounds + logged = len(session.command_log) + members = member_ids(session) + result = session.execute(RemoveMember(character_id=character_id)) + assert not result.accepted + assert [(rejection.code, rejection.params) for rejection in result.rejections] == [ + (CODE, {"character": character_id, "item": item_id}) + ] + assert list(result.events) == [] + assert stream_states(session) == streams, "a refusal draws nothing" + assert session.clock.rounds == rounds, "a refusal spends no time" + assert len(session.command_log) == logged, "a refusal is not logged" + assert member_ids(session) == members, "a refusal removes nobody" + assert frozen_state(session) == state, "a refusal changes nothing" + + +def assert_refused_with(session: GameSession, command: RemoveMember, code: str) -> None: + """The command is refused with `code` alone.""" + result = session.execute(command) + assert not result.accepted + assert [rejection.code for rejection in result.rejections] == [code] + + +# ---------------------------------------------------------------------- the contract + + +class TestTheContract: + def test_the_docstring_lists_the_code_last(self): + doc = RemoveMember.__doc__ or "" + rejections = doc[doc.index("Rejections:") : doc.index("Events:")] + listed = [line.strip().split("`")[1] for line in rejections.splitlines() if line.strip().startswith("- `")] + assert listed[-1] == CODE + assert listed[:-1] == [ + "session.command.wrong_mode", + "session.command.unknown_member", + "party.remove.same_member", + "session.command.member_incapacitated", + "party.remove.last_standing", + ] + + +# ---------------------------------------------------------------------- refused + + +@CHUNK +class TestRefused: + @pytest.mark.parametrize("site", SITES) + def test_an_item_named_at_any_site_is_protected(self, site): + session = session_naming((site, "holy_water")) + grant(session, THIEF, "holy_water") + assert_guarded(session, THIEF, "holy_water") + + @pytest.mark.parametrize( + ("item_id", "equip"), + [ + ("holy_water", False), + ("sword", True), + ("brass_key", False), + ("potion_of_healing", False), + ("ring_of_regeneration", True), + ], + ids=["mundane", "mundane-equipped", "authored", "magic", "magic-equipped"], + ) + def test_every_kind_of_item_counts_equipped_or_not(self, item_id, equip): + session = session_naming(("trigger_condition", item_id)) + grant(session, THIEF, item_id, equip=equip) + assert_guarded(session, THIEF, item_id) + + @pytest.mark.parametrize("state", ["alive", "dead", "petrified", "asleep"]) + def test_a_member_in_any_state_is_protected(self, state): + session = session_naming(("objective_condition", "holy_water")) + grant(session, THIEF, "holy_water") + put_in_state(session, THIEF, state) + assert_guarded(session, THIEF, "holy_water") + + def test_another_member_s_copy_doesn_t_lift_the_refusal(self): + session = session_naming(("door_gate", "holy_water")) + grant(session, LEAD, "holy_water") + grant(session, THIEF, "holy_water") + assert_guarded(session, THIEF, "holy_water") + + def test_an_inactive_quest_protects_its_items(self): + session = session_naming(("activation_pattern", "holy_water")) + assert session.quests[QUEST].status == "inactive" + grant(session, THIEF, "holy_water") + assert_guarded(session, THIEF, "holy_water") + + def test_an_open_quest_protects_the_item_of_a_completed_objective(self): + session = session_naming(("objective_pattern", "holy_water")) + grant(session, THIEF, "holy_water") + assert session.execute(CompleteObjective(quest_id=QUEST, objective_id="find")).accepted + assert session.quests[QUEST].status == "active" + assert_guarded(session, THIEF, "holy_water") + + def test_a_repeatable_trigger_that_has_fired_still_protects(self): + session = session_naming(("trigger_condition", "holy_water"), repeatable=True) + grant(session, THIEF, "holy_water") + assert session.execute(MarkTriggerFired(trigger_id=TRIGGER)).accepted + assert_guarded(session, THIEF, "holy_water") + + def test_a_gate_protects_after_a_quest_naming_the_same_item_is_completed(self): + session = session_naming(("door_gate", "holy_water"), ("objective_condition", "holy_water")) + grant(session, THIEF, "holy_water") + assert session.execute(CompleteQuest(quest_id=QUEST)).accepted + assert_guarded(session, THIEF, "holy_water") + + def test_an_open_quest_protects_after_a_fired_trigger_naming_the_same_item(self): + session = session_naming(("trigger_pattern", "holy_water"), ("reveal_condition", "holy_water")) + grant(session, THIEF, "holy_water") + assert session.execute(MarkTriggerFired(trigger_id=TRIGGER)).accepted + assert_guarded(session, THIEF, "holy_water") + + @pytest.mark.parametrize( + ("first", "second"), [("holy_water", "potion_of_healing"), ("potion_of_healing", "holy_water")] + ) + def test_the_rejection_names_the_first_item_in_the_pack(self, first, second): + # The adventure names the potion first and the holy water second, so the order the + # rejection follows is the inventory's, not the adventure's. + session = session_naming(("trigger_condition", "potion_of_healing"), ("door_gate", "holy_water")) + grant(session, THIEF, first) + grant(session, THIEF, second) + assert_guarded(session, THIEF, first) + + def test_the_pack_comes_before_what_is_equipped(self): + session = session_naming(("trigger_condition", "sword"), ("door_gate", "holy_water")) + grant(session, THIEF, "sword", equip=True) + grant(session, THIEF, "holy_water") + assert_guarded(session, THIEF, "holy_water") + + +# ---------------------------------------------------------------------- allowed + + +class TestAllowed: + """Removals the check must not refuse. These pass before the check exists.""" + + @pytest.mark.parametrize("state", ["alive", "dead"]) + def test_give_to_takes_the_item_and_the_member_leaves(self, state): + session = session_naming(("door_gate", "holy_water"), ("objective_pattern", "brass_key")) + grant(session, THIEF, "holy_water") + grant(session, THIEF, "brass_key") + put_in_state(session, THIEF, state) + remove(session, THIEF, give_to=LEAD) + assert THIEF not in member_ids(session) + assert session.member(LEAD).inventory.carried_item("holy_water") is not None + assert session.member(LEAD).inventory.carried_item("brass_key") is not None + + def test_a_member_carrying_nothing_the_adventure_names_leaves(self): + session = session_naming(("door_gate", "holy_water")) + grant(session, LEAD, "holy_water") + grant(session, THIEF, "torch") + remove(session, THIEF) + assert THIEF not in member_ids(session) + + @pytest.mark.parametrize("site", [site for site in SITES if not site.endswith("_gate")]) + def test_a_completed_quest_or_a_spent_trigger_protects_nothing(self, site): + session = session_naming((site, "holy_water")) + grant(session, THIEF, "holy_water") + if site.startswith("trigger_"): + assert session.execute(MarkTriggerFired(trigger_id=TRIGGER)).accepted + else: + if session.quests[QUEST].status == "inactive": + assert session.execute(ActivateQuest(quest_id=QUEST)).accepted + assert session.execute(CompleteQuest(quest_id=QUEST)).accepted + assert session.quests[QUEST].status == "completed" + remove(session, THIEF) + assert THIEF not in member_ids(session) + + def test_a_spent_stack_is_not_carried(self): + session = session_naming(("trigger_condition", "potion_of_healing")) + instance_id = granted_magic(session, THIEF, "potion_of_healing") + spent = next(item for item in session.member(THIEF).inventory.items if item.instance_id == instance_id) + assert isinstance(spent, MagicItemInstance) + spent.quantity = 0 + remove(session, THIEF) + assert THIEF not in member_ids(session) + + def test_the_mode_is_checked_first(self): + session = session_naming(("door_gate", "holy_water")) + grant(session, THIEF, "holy_water") + session.mode = SessionMode.VICTORY + assert_refused_with(session, RemoveMember(character_id=THIEF), "session.command.wrong_mode") + + def test_an_unknown_member_is_reported_first(self): + session = session_naming(("door_gate", "holy_water")) + grant(session, THIEF, "holy_water") + assert_refused_with(session, RemoveMember(character_id="character-0099"), "session.command.unknown_member") + + def test_the_last_standing_member_is_reported_first(self): + party = build_party((("Brakk", "fighter"),)) + session = GameSession.new(party, adventure_naming(("door_gate", "holy_water")), seed=SEED) + grant(session, LEAD, "holy_water") + assert_refused_with(session, RemoveMember(character_id=LEAD), "party.remove.last_standing") + + def test_an_incapacitated_give_to_is_reported_and_not_the_item(self): + session = session_naming(("door_gate", "holy_water")) + grant(session, THIEF, "holy_water") + put_in_state(session, LEAD, "asleep") + command = RemoveMember(character_id=THIEF, give_to=LEAD) + assert_refused_with(session, command, "session.command.member_incapacitated") From 6d33c406131453191a0f9e8475bc065bb00a04ce Mon Sep 17 00:00:00 2001 From: Marsh Macy Date: Fri, 30 Oct 2026 21:40:21 -0700 Subject: [PATCH 2/3] Refuse RemoveMember when the member carries a quest item RemoveMember without give_to now refuses with party.remove.carries_quest_item when the leaving member carries an item the adventure depends on. The handler runs the check last, after every other refusal, before it changes anything or draws anything. The adventure depends on an item when a HasItemCondition or an ItemAcquiredPattern names it in a door or transition gate, in a trigger that can still fire, or in any clause of a quest that isn't completed. The rejection names the member as character and the first matching item in Inventory.all_instances order as item. A spent stack doesn't count. The rejection code has a description, the changelog has a bullet, and the LLM referee guide mentions the refusal. The chunk's xfail markers stay for the lead to remove. Claude-Session: https://claude.ai/code/session_01V7ErtnsmhaP7aLwcfmzohD --- CHANGELOG.md | 4 +++ docs/front-ends/llm-referees.md | 2 +- src/osrlib/crawl/session.py | 57 +++++++++++++++++++++++++++++++++ tools/docs/rejection_codes.json | 1 + 4 files changed, 63 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 628a962..932faf7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ## [Unreleased] +### Changed + +- [`RemoveMember`][osrlib.crawl.commands.RemoveMember] without `give_to` now refuses to remove a member who carries an item the adventure depends on, so the party can't lose a quest item by dropping the member who held it. The adventure depends on an item when a gate on a door or transition, a trigger that can still fire, or any clause of a quest that isn't completed names its catalog id. The session refuses with `party.remove.carries_quest_item`, and the rejection's `character` and `item` params give the member and the item. Name a member in `give_to` and the item moves to them, so the removal goes through. + ## [2.2.0] - 2026-10-30 ### Added diff --git a/docs/front-ends/llm-referees.md b/docs/front-ends/llm-referees.md index 3925b78..8b661c7 100644 --- a/docs/front-ends/llm-referees.md +++ b/docs/front-ends/llm-referees.md @@ -64,7 +64,7 @@ Player commands let the model drive the party's turn. Referee commands let it *r - [`GrantItem`][osrlib.crawl.commands.GrantItem], [`GrantCoins`][osrlib.crawl.commands.GrantCoins], [`AwardXP`][osrlib.crawl.commands.AwardXP] - place rewards directly, magic items included - [`RemoveItem`][osrlib.crawl.commands.RemoveItem], [`RemoveCoins`][osrlib.crawl.commands.RemoveCoins] - take items or coins away: a price, a toll, armour a shop buys back - [`AddMember`][osrlib.crawl.commands.AddMember] - bring a new character into the party in town or while exploring, like a replacement for a fallen member. The new member joins at the back of the marching order, and the event gives their id -- [`RemoveMember`][osrlib.crawl.commands.RemoveMember] - take a character out of the party for good in town or while exploring, like one who died too long ago for *raise dead*. Name another member in `give_to` to keep what the character carries in the party. The removed id is never used again +- [`RemoveMember`][osrlib.crawl.commands.RemoveMember] - take a character out of the party for good in town or while exploring, like one who died too long ago for *raise dead*. Name another member in `give_to` to keep what the character carries in the party. The removed id is never used again. The session refuses with `party.remove.carries_quest_item` when you leave `give_to` out and the character carries an item the adventure depends on, so name a member to take it. - [`SetDoorState`][osrlib.crawl.commands.SetDoorState] - rewrite any door's state anywhere: lock it, wedge it, reveal it - [`PlaceParty`][osrlib.crawl.commands.PlaceParty] and [`AdvanceTime`][osrlib.crawl.commands.AdvanceTime] - teleport the party, advance the clock - [`AddJournalEntry`][osrlib.crawl.commands.AddJournalEntry] and [`RecordNote`][osrlib.crawl.commands.RecordNote] - what the agent records in the game world: a journal entry is written for the players and appears in their view, and a note is for the referee only and stays out of the player view diff --git a/src/osrlib/crawl/session.py b/src/osrlib/crawl/session.py index 2104277..e8eeed5 100644 --- a/src/osrlib/crawl/session.py +++ b/src/osrlib/crawl/session.py @@ -184,8 +184,10 @@ TriggerFiredEvent, XpAwardedEvent, ) +from osrlib.crawl.gates import ConditionSpec, HasItemCondition from osrlib.crawl.party import Party, _anyone_standing, _stands from osrlib.crawl.quests import ObjectiveSpec, QuestSpec +from osrlib.crawl.triggers import ItemAcquiredPattern, TriggerPattern from osrlib.data import load_equipment, load_magic_items, load_monsters from osrlib.errors import ContentValidationError from osrlib.versioning import SCHEMA_VERSION, engine_version @@ -2312,6 +2314,19 @@ def _handle_remove_member(session: GameSession, command: RemoveMember) -> tuple[ return [Rejection(code="session.command.member_incapacitated", params={"character": command.give_to})], [] if not any(_stands(other) for other in session.party.members if other is not member): return [Rejection(code="party.remove.last_standing", params={"character": command.character_id})], [] + if recipient is None: + needed = _quest_item_ids(session) + for instance in member.inventory.all_instances(): + if instance.quantity < 1: + continue + item_id = instance.template_id if isinstance(instance, MagicItemInstance) else instance.template.id + if item_id in needed: + return [ + Rejection( + code="party.remove.carries_quest_item", + params={"character": command.character_id, "item": item_id}, + ) + ], [] member_id = _member_id(member) events: list[Event] = [] if recipient is not None: @@ -2334,6 +2349,48 @@ def _handle_remove_member(session: GameSession, command: RemoveMember) -> tuple[ return [], events +def _quest_item_ids(session: GameSession) -> set[str]: + """Return the catalog id of every item the adventure still depends on. + + An item counts when a `HasItemCondition` or an `ItemAcquiredPattern` names it in a door or + transition gate, in a trigger that can still fire, or in any clause of a quest that isn't + completed. A trigger that has fired and isn't repeatable can't matter again, and neither can + a completed quest. A gate always counts, because the party can come back to it. + """ + adventure = session.adventure + clauses: list[tuple[TriggerPattern, tuple[ConditionSpec, ...]]] = [] + conditions: list[ConditionSpec] = [] + for dungeon in adventure.dungeons: + for level in dungeon.levels: + conditions.extend( + edge.door.requires.condition + for edge in level.edges.values() + if edge.door is not None and edge.door.requires is not None + ) + conditions.extend( + transition.requires.condition for transition in level.transitions if transition.requires is not None + ) + for trigger in adventure.triggers: + if trigger.repeatable or trigger.id not in session.fired_triggers: + clauses.append((trigger.when, trigger.conditions)) + for quest in adventure.quests: + if session.quests[quest.id].status == "completed": + continue + if quest.activation is not None: + clauses.append((quest.activation.pattern, quest.activation.conditions)) + for objective in quest.objectives: + clauses.append((objective.when.pattern, objective.when.conditions)) + if objective.reveal_when is not None: + clauses.append((objective.reveal_when.pattern, objective.reveal_when.conditions)) + item_ids: set[str] = set() + for pattern, clause_conditions in clauses: + if isinstance(pattern, ItemAcquiredPattern): + item_ids.add(pattern.item_id) + conditions.extend(clause_conditions) + item_ids.update(condition.item_id for condition in conditions if isinstance(condition, HasItemCondition)) + return item_ids + + def _hand_over_everything(session: GameSession, giver: Character, recipient: Character) -> list[Event]: """Move everything a leaving member carries to `recipient`, as a `GiveItems` of all of it would. diff --git a/tools/docs/rejection_codes.json b/tools/docs/rejection_codes.json index 53e9a8d..aa22ae5 100644 --- a/tools/docs/rejection_codes.json +++ b/tools/docs/rejection_codes.json @@ -132,6 +132,7 @@ "party.add.carries_instances": "A referee tried to add a party member who carries a magic item or a valuable, equipped or not. Those have instance ids from the game that created them, which could match an id this session hands out. Add the character without them and give magic items afterwards with a grant.", "party.add.id_assigned": "A referee tried to add a party member whose id is already set: a character from another game, or one who is already a member. The session assigns the new member's id, so the character must arrive with no id.", "party.add.under_effects": "A referee tried to add a party member who has a condition, such as dead or asleep, or a stat modifier. Each of those belongs to an effect in the game that applied it, and this session can't end an effect it doesn't have.", + "party.remove.carries_quest_item": "A referee tried to remove a party member without naming a member in `give_to`, and the member carries an item the adventure depends on. A door, stair, trigger, or open quest names the item, so removing the member would take it out of the game and could leave the party stuck. Send the command again with `give_to` set to a member who can take everything, the item included.", "party.remove.last_standing": "A referee tried to remove the last party member who is neither dead nor turned to stone, or the only member. A party with nobody standing has lost the game. Add the replacement first, and then remove the old member.", "party.remove.same_member": "A referee tried to remove a party member and give their belongings to that same member. Name a different member to take them, or leave the recipient out so the belongings leave with the member.", "session.command.encounter_in_progress": "The referee tried to spawn an NPC party or place the party while an encounter or battle was already in progress.", From 7c2c906188e6e6893a6b0b62eb8d1c17a8ca2f59 Mon Sep 17 00:00:00 2001 From: Marsh Macy Date: Fri, 30 Oct 2026 21:41:50 -0700 Subject: [PATCH 3/3] Remove the quest-item-guard markers The tests for party.remove.carries_quest_item pass, so their expected-to-fail markers and the one on the rejection-code docs gate come off. Claude-Session: https://claude.ai/code/session_01V7ErtnsmhaP7aLwcfmzohD --- tests/test_docs_gates.py | 2 -- tests/test_remove_member_quest_items.py | 7 +------ 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/test_docs_gates.py b/tests/test_docs_gates.py index fca25fc..78f5650 100644 --- a/tests/test_docs_gates.py +++ b/tests/test_docs_gates.py @@ -158,8 +158,6 @@ def test_documented_modes_equal_allowed_modes(self): wrong.append(f"{cls.__name__}: documents {sorted(documented)}, allows {sorted(actual)}") assert not wrong, "\n".join(wrong) - # `RemoveMember` documents `party.remove.carries_quest_item`, which its handler doesn't emit yet. - @pytest.mark.xfail(reason="chunk: quest-item-guard", strict=True) def test_documented_rejection_codes_exist_in_the_source(self, vocabulary): wrong = [] for cls in self._commands(): diff --git a/tests/test_remove_member_quest_items.py b/tests/test_remove_member_quest_items.py index d08f6c3..47e5353 100644 --- a/tests/test_remove_member_quest_items.py +++ b/tests/test_remove_member_quest_items.py @@ -25,9 +25,7 @@ order as `item`. - A refusal costs nothing: no draws, no time, no log entry, and no change. -The tests in `TestRefused` fail until the session emits the code, so they carry the chunk's -marker. The tests in `TestAllowed` pass already and have to keep passing, because they state -the removals the check must not refuse. +The tests in `TestAllowed` state the removals the check must not refuse. """ from typing import Any @@ -55,8 +53,6 @@ from test_add_member import SEED, frozen_state, member_ids, quiet_adventure, stream_states from test_remove_member import LEAD, THIEF, granted_magic, put_in_state, remove -CHUNK = pytest.mark.xfail(reason="chunk: quest-item-guard", strict=True) - CODE = "party.remove.carries_quest_item" QUEST = "errand" @@ -214,7 +210,6 @@ def test_the_docstring_lists_the_code_last(self): # ---------------------------------------------------------------------- refused -@CHUNK class TestRefused: @pytest.mark.parametrize("site", SITES) def test_an_item_named_at_any_site_is_protected(self, site):