diff --git a/backend/app/routers/adventures.py b/backend/app/routers/adventures.py index 89a71c4..8a6d795 100644 --- a/backend/app/routers/adventures.py +++ b/backend/app/routers/adventures.py @@ -1489,15 +1489,18 @@ def add_take( db: Session = Depends(get_db), user: models.User = CurrentUser, ): - """Play one of the player's own turns again, differently. + """Play a turn again, differently — whoever wrote it. - The gap SP7 left: a retry gives an AI turn another take, and nothing gave - one to the player's own message. So the only way to change something you - had written was to overwrite it and lose the story it led to. + One operation for what used to be two and a half. `retry` gave an AI turn + another take, but only the newest one; the player's own message had none at + all, so changing something you had typed meant overwriting it and losing the + story it led to. Here an AI turn regenerates and a player turn takes the + text supplied, and neither cares where in the story it sits. - This is the same move a retry makes, on the other kind of node. The turn is - played again with new text; whatever the story did with the old one stays - exactly where it is, on the line it was written on. + The tip is the only case needing no branch, and only for an AI turn: nothing + was played after it, so its takes are still leaves. A player turn is never + at the tip — the reply to it is — so it takes a branch every time it has + been answered. A branch is needed here for the same reason `fork` needs one — the turn being retaken already has a story after it, and that story was written as a @@ -1512,38 +1515,49 @@ def add_take( action = db.get(models.Action, action_id) if action is None or action.adventure_id != adventure_id: raise HTTPException(404, "Action not found") - if action.type == "ai": - raise HTTPException( - 400, "Retry gives an AI turn another take; this is for your own." - ) - if action.type not in ("do", "say", "story", "continue"): + if action.type not in ("do", "say", "story", "continue", "ai"): # The opening is not a turn anybody played, so there is no second way # to have played it. Editing the scenario is what changes it. raise HTTPException(400, "The opening of a story has no other take.") if action.depth is None or not lineage.path_of(db, adventure).contains(action): raise HTTPException(400, "That turn is not on the story you are reading.") acquire_turn_lock(adventure_id) + retry_of = None try: - # Only when something was played after it. At the tip there is no story - # to protect and no branch is owed — the same rule `stand_on` follows. - if adventure.head_depth > action.depth: + newest = last_action(adventure, db) + at_the_tip = newest is not None and newest.id == action.id + if at_the_tip and action.type == "ai": + # Nothing was played after it, so its takes are still leaves and a + # branch would be for nothing. This is exactly `retry`. + retry_of = action + attempts.roll_back_before(db, adventure, action) + else: + # The turn has a story after it, written as a continuation of what + # is there now. The new take leaves the path just before the turn, + # so that story keeps the take it was written for. tree.branch_at(db, adventure, action.depth - 1) - adventure.updated_at = models.utcnow() - db.commit() - db.refresh(adventure) + attempts.roll_back_before(db, adventure, action) + adventure.updated_at = models.utcnow() + db.commit() + db.refresh(adventure) except BaseException: _active_turns.discard(adventure_id) raise + if action.type == "ai": + # No player action to write: the one this turn answers is already on + # the path, borrowed from the line being left. + stream = generate_turn( + adventure, db, ScriptPipeline(adventure, db), user, retry_of=retry_of + ) + else: + stream = run_player_turn( + adventure, + db, + schemas.ActionCreate(type=action.type, text=payload.text), + user, + ) return StreamingResponse( - with_turn_lock( - adventure_id, - run_player_turn( - adventure, - db, - schemas.ActionCreate(type=action.type, text=payload.text), - user, - ), - ), + with_turn_lock(adventure_id, stream), media_type="text/event-stream", headers=SSE_HEADERS, ) diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 6f5d547..7a47785 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -266,9 +266,15 @@ class ActionCreate(BaseModel): class TakeCreate(BaseModel): - """Another take of a turn the player wrote themselves (SP9).""" + """Another take of a turn (SP9). - text: ActionText + `text` is what the player is saying instead, and is theirs to write only + when the turn was theirs. An AI turn's other take is generated, so the field + is ignored there rather than refused — the client asks the same way for both + and the node type decides what happens. + """ + + text: ActionText = "" class AdventureOut(ORMModel): diff --git a/backend/tests/test_take_parentage.py b/backend/tests/test_take_parentage.py index 2f926bf..2cc9685 100644 --- a/backend/tests/test_take_parentage.py +++ b/backend/tests/test_take_parentage.py @@ -358,12 +358,47 @@ def test_both_takes_of_a_players_turn_are_one_group(client): assert _group_size(first.id) == 2, "and reads the same from the other take" -def test_an_ai_turn_is_refused_by_the_take_endpoint(client): +def test_an_ai_turn_at_the_tip_takes_no_branch(client): + """Its takes are still leaves. This is `retry`, reached the other way.""" _play(client) + before = _branch_count(client.adv_id) + ai = _ai_rows(client.adv_id)[0] - r = _take(client, ai.id, "nope") - assert r.status_code == 400 - assert "Retry" in r.json()["detail"] + r = _take(client, ai.id, "") + assert r.status_code == 200, r.text + + assert _branch_count(client.adv_id) == before + assert len(_ai_rows(client.adv_id)) == 2, "a second take, beside the first" + + +def test_an_ai_turn_the_story_moved_past_takes_a_branch(client): + """Retry could never reach here at all — it only ever saw the newest turn.""" + _play(client) + _play(client, "press on") + before = _branch_count(client.adv_id) + first_ai = _ai_rows(client.adv_id)[0] + + r = _take(client, first_ai.id, "") + assert r.status_code == 200, r.text + + assert _branch_count(client.adv_id) == before + 1 + # The turn the story moved past now has two takes, and the line it was on + # keeps the one it was written for. + assert _group_size(first_ai.id) == 2 + assert first_ai.id in {a.id for a in _ai_rows(client.adv_id)} + + +def test_the_old_line_still_has_its_continuation(client): + _play(client, "open the door") + _play(client, "press on") + first_ai = _ai_rows(client.adv_id)[0] + _take(client, first_ai.id, "") + + # The new take is what this branch tells; "press on" belonged to the other. + blob = "\n".join(_path_texts(client)) + assert "press on" not in blob + kept = "\n".join(a.text for a in _user_rows(client.adv_id)) + assert "press on" in kept, "still there, on the line it was played on" def test_the_opening_has_no_other_take(client):