diff --git a/.gitignore b/.gitignore index 698bb2e..4bfaf13 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ .idea/* cmake-build-* build-export/* +build/ +.DS_Store diff --git a/CLAUDE.md b/CLAUDE.md index 25ae8dd..8e8f144 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -5,16 +5,21 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## Build Commands ```bash -# Configure and build with tests +# Build with tests (manual) mkdir build && cd build cmake -DBUILD_TESTS=ON .. make -j +# Build using CMake presets (preferred on macOS) +cmake --preset clang # Apple clang +cmake --preset clang-llvm # Homebrew LLVM (required for clang-tidy/IWYU) +cmake --build build/clang-llvm -j + # Run all tests ctest -# Run a single test by name -./lib/GameLogicTests --gtest_filter=Game.moveNorth +# Run a single test by name (lib tests) +./build/clang/lib/GameLogicTests --gtest_filter=GameTest.moveToValidRoomChangesPlayerRoom ``` ## Code Quality Tools @@ -22,48 +27,61 @@ ctest The project uses `just` for common tasks (requires [just](https://github.com/casey/just)): ```bash -just build-export # Build with compile_commands.json +just build-export # Generate compile_commands.json (uses Homebrew LLVM) just clang-tidy # Run static analysis just clang-tidy-fix # Auto-fix clang-tidy issues just iwyu # Run Include What You Use just format # Format code with clang-format ``` -Manual equivalents: -```bash -# clang-tidy (after build-export) -clang-tidy -p build-export *.cpp */*.cpp - -# IWYU -iwyu_tool.py -p build-export main*.cpp */*.cpp -- -Xiwyu --error -Xiwyu --cxx17ns - -# Format -clang-format -style=file -i main*.cpp */*.*pp -``` +> **Note**: `just build-export` uses `/opt/homebrew/opt/llvm/bin/clang++`. Apple's system clang cannot find C++ standard headers for IWYU/clang-tidy. Always use the Homebrew LLVM preset for these tools. ## CI Checks -All PRs must pass: build, clang-tidy, IWYU, clang-format. Warnings are treated as errors. +All PRs must pass: build (Ubuntu/gcc), clang-tidy, IWYU, clang-format. Warnings are treated as errors. ## Architecture -The game logic lives in `lib/` as a shared library (`GameLogic`), with `main.cpp` as a thin entry point. +The game logic lives in `lib/` as a shared library (`GameLogic`), with `main.cpp` as a thin entry point that wires up concrete implementations. + +**Interfaces** (namespace `adv_sk`) — the dependency-inversion layer: +- `IInputHandler` — input/output abstraction; `Action` enum defines all user actions +- `IMap` — map query abstraction (`next_room`, `get_room`, `get_welcome_message`) +- `IPlayer` — player state abstraction (room, inventory) -**Core classes** (namespace `adv_sk`): -- `Game` - Main game controller with game loop, owns Map, Player, and IInputHandler -- `IInputHandler` - Interface for input/output abstraction (enables testing via mock) -- `Map` - Collection of connected rooms, created via `create_map()` -- `Room` - Location with connections and inventory -- `Player` - Tracks current room and inventory -- `Direction` - Enum (North/South/East/West) with `ALL_DIRECTIONS` constant +**Concrete classes**: +- `Game` — owns `IMap`, `IPlayer`, `IInputHandler` via `unique_ptr`; drives the game loop +- `Map` / `Room` — concrete map with connected rooms; built via `create_map()` +- `Player` — tracks current room and inventory (`InventoryItem`) +- `ConsoleInputHandler` — terminal I/O implementation +- `Direction` — enum (North/South/East/West) with `ALL_DIRECTIONS` constant -**Key patterns**: -- Dependency injection: Game accepts `IInputHandler` for testability -- Tests use `TestInputHandler` mock to script game behavior without console I/O +**Test doubles** (in `lib/`, namespace `adv_sk::test`): +- `MockInputHandler`, `MockMap`, `MockPlayer` — GMock mocks for London-style unit tests +- Tests inject mocks via `Game(unique_ptr, unique_ptr, unique_ptr)` ## Naming Conventions Enforced by clang-tidy: - Classes: `CamelCase` -- Functions/variables: `lower_case` +- Functions/variables: `snake_case` - Private members: `_prefixed` +- Struct initialization: use designated initializers — `InventoryItem{.name = "sword"}` (not positional) + +## Known NOLINT Patterns + +GMock `MOCK_METHOD` macros generate public member variables, requiring suppression on the surrounding class: + +```cpp +// NOLINTBEGIN(misc-non-private-member-variables-in-classes,cppcoreguidelines-non-private-member-variables-in-classes) +class MockFoo : public IFoo { ... }; +// NOLINTEND(misc-non-private-member-variables-in-classes,cppcoreguidelines-non-private-member-variables-in-classes) +``` + +GoogleTest `ASSERT_TRUE` does not satisfy `bugprone-unchecked-optional-access` — suppress on the next line accessing the optional: + +```cpp +ASSERT_TRUE(opt.has_value()); +// NOLINTNEXTLINE(bugprone-unchecked-optional-access) +use(*opt); +``` diff --git a/docs/development-plan.md b/docs/development-plan.md new file mode 100644 index 0000000..7ec9bb2 --- /dev/null +++ b/docs/development-plan.md @@ -0,0 +1,324 @@ +# Development Plan + +A phased roadmap for extending AdventureGame. Each phase follows the established patterns: +dependency injection through pure-virtual interfaces, London-style TDD with GMock mocks, +C++20/Google style, and clang-tidy/IWYU compliance. + +All new code lives in `lib/` (namespace `adv_sk`), tests in `adv_sk::test`. +Each phase ships as its own PR so CI gates validate incrementally. + +--- + +## Phase 1 — Fix Existing Gaps + +### 1.1 Fix `ConsoleInputHandler` missing commands + +**File:** `lib/ConsoleInputHandler.cpp` + +`"drop"` → `Action::DropItem` and `"inventory"` → `Action::DisplayInventory` are absent from +`get_action()`. Add both to the existing if/else chain. No interface changes. + +### 1.2 Give `rusty sword` a `use_message` + +**File:** `lib/Map.cpp` (`create_map()`) + +The `rusty sword` `InventoryItem` has an empty `use_message`. Assign e.g.: +`"You swing the rusty sword. The blade holds, but barely."` + +### 1.3 Add a "look" action + +Re-displays the current room description and available exits without moving. + +**Files:** +- `lib/IInputHandler.hpp` — add `Look` to the `Action` enum +- `lib/ConsoleInputHandler.cpp` — add `"look"` to `get_action()` parser +- `lib/Game.hpp` / `lib/Game.cpp` — add private `handle_look()`, dispatch from `handle_user_action()` + +**Testing:** Follow the `investigate` test pattern — inject `MockMap`, `MockPlayer`, +`MockInputHandler`; set expectations that the room description and available directions are +sent via `MockInputHandler::provide_message()`. + +> **clang-tidy:** Every `switch(action)` in `Game.cpp` must gain a `case Action::Look:` branch. +> Missing cases are `-Wswitch` errors. + +--- + +## Phase 2 — Expand the World + +### 2.1 Map layout + +Extend `create_map()` in `lib/Map.cpp` from 2 rooms to 7. Bidirectional connections are +created automatically via `opposite_direction()`. + +``` + [Armoury] — East — [Barracks] + ↕ +[Library] ← [Grand Hall] → [Throne Room] + ↕ + [Kitchen] + ↕ + [Cellar] +``` + +### 2.2 Item placement + +All items start with `is_visible = false` and are revealed by `investigate`. + +| Room | Item | `use_message` | +|-------------|-----------------|-------------------------------------------------------------| +| Grand Hall | golden chalice | `"The chalice gleams. It belongs on the throne."` | +| Armoury | rusty sword | (Phase 1.2) | +| Library | old tome | `"The tome describes a hidden cellar beneath the kitchen."` | +| Kitchen | iron key | `"The key is cold and heavy in your hand."` | +| Cellar | silver pendant | `"The pendant pulses with faint warmth."` | + +**Narrative through-line:** Find the chalice → explore to gather clues → unlock the Cellar → +return the chalice to the Throne Room. + +No structural changes required — only data additions to `create_map()`. + +--- + +## Phase 3 — Win Condition + +### 3.1 `GameOutcome` enum + +Add to `lib/Game.hpp` (or a new `lib/GameOutcome.hpp`): + +```cpp +enum class GameOutcome : std::uint8_t { Ongoing, Win, Lose }; +``` + +### 3.2 Victory trigger + +**Victory:** Using the `golden chalice` while standing in the Throne Room. + +- `Game` holds `GameOutcome _outcome = GameOutcome::Ongoing` +- `Game::start()` returns `GameOutcome` instead of `void`; loop exits when `_outcome != Ongoing` +- `handle_use_item()` checks item name + current room; on match, sets `_outcome = Win` and + sends a victory message via `IInputHandler::provide_message()` + +### 3.3 Testing + +Script `MockInputHandler::get_action()` → `Action::UseItem`. Configure +`MockPlayer::get_current_room()` to return the Throne Room or another room. Verify `_outcome` +is `Win` only when both conditions are met. + +--- + +## Phase 4 — Locked Doors / Item Keys + +Lock state is map state, so lock logic belongs in `IMap`. + +### 4.1 `IMap` extension + +**File:** `lib/IMap.hpp` + +```cpp +virtual std::optional get_passage_key( + const RoomName& from, const RoomName& to) const = 0; +virtual void unlock_passage(const RoomName& from, const RoomName& to) = 0; +``` + +`get_passage_key` returns `nullopt` for open passages and the required key name for locked ones. +Returning the key name in a single call avoids a two-call round-trip. + +**File:** `lib/Map.hpp` / `lib/Map.cpp` + +`Map` holds `std::map, std::string> _locked_passages`. +`create_map()` locks the Kitchen → Cellar passage, requiring `"iron key"`. + +### 4.2 `Game::handle_move()` change + +Before calling `IMap::next_room()`: +1. Call `get_passage_key(current_room, target_room)` +2. If a key is required and the player has it → call `unlock_passage()` then proceed +3. If a key is required and the player lacks it → send `"The way is blocked.\n"` and return + +### 4.3 `MockMap` update + +Add `MOCK_METHOD` declarations for both new methods in `lib/MockMap.hpp`. +Wrap in `NOLINTBEGIN/END` per the established pattern. + +> **IWYU:** `` must be a direct include in every file that uses `std::optional`. +> Annotate any `ASSERT_TRUE` on an optional before dereferencing with +> `// NOLINTNEXTLINE(bugprone-unchecked-optional-access)`. + +--- + +## Phase 5 — NPC System + +### 5.1 `INPC` interface + +**New file:** `lib/INPC.hpp` — follows the `IPlayer` pattern. + +```cpp +namespace adv_sk { + class INPC { + public: + virtual ~INPC() = default; + virtual std::string get_name() const = 0; + virtual std::string get_next_dialogue() = 0; + }; +} +``` + +### 5.2 Concrete `NPC` + +**New files:** `lib/NPC.hpp`, `lib/NPC.cpp` + +Holds `std::string _name`, `std::vector _dialogue_lines`, and +`std::size_t _dialogue_index = 0`. `get_next_dialogue()` returns the current line and +advances the index with wrap-around. + +### 5.3 `MockNPC` + +**New file:** `lib/MockNPC.hpp` — follows `MockPlayer` / `MockInputHandler` pattern. + +```cpp +// NOLINTBEGIN(misc-non-private-member-variables-in-classes, +// cppcoreguidelines-non-private-member-variables-in-classes) +class MockNPC : public INPC { + public: + MOCK_METHOD(std::string, get_name, (), (const, override)); + MOCK_METHOD(std::string, get_next_dialogue, (), (override)); +}; +// NOLINTEND(misc-non-private-member-variables-in-classes, +// cppcoreguidelines-non-private-member-variables-in-classes) +``` + +### 5.4 NPC ownership + +`IMap` owns NPCs (keyed by room name) to avoid making `Room` non-copyable with `unique_ptr` +members: + +```cpp +virtual INPC* get_npc(const RoomName& room) const = 0; // nullptr = no NPC present +``` + +`Map` holds `std::map> _npcs`. + +### 5.5 `IInputHandler` and `Action` additions + +- Add `Talk` to `Action` enum in `lib/IInputHandler.hpp` +- Add `virtual std::string get_npc_name() = 0;` to `IInputHandler` (needed for future + multi-NPC rooms; also keeps `ConsoleInputHandler` and all mocks consistent) +- Add `"talk"` to `ConsoleInputHandler::get_action()` parser +- Add private `handle_talk()` to `Game`, dispatched from `handle_user_action()` + +### 5.6 NPC placement in `create_map()` + +| Room | NPC name | Dialogue lines | +|----------|-----------------|-----------------------------------------------------------------------------| +| Library | Old Librarian | `"The chalice belongs to the throne."`, `"I haven't left in years."` | +| Barracks | Sleeping Guard | `"Hmm... what? Move along."`, `"Don't tell anyone I was sleeping."` | + +### 5.7 Testing + +- No NPC in room → `handle_talk()` sends `"There is no one here to talk to.\n"` +- NPC present → `MockNPC::get_next_dialogue()` is called; return value sent via + `MockInputHandler::provide_message()` +- `MockMap` gains a `MOCK_METHOD` for `get_npc` + +--- + +## Phase 6 — Save / Load + +### 6.1 `GameSnapshot` + +**New file:** `lib/GameSnapshot.hpp` — plain struct, no logic. + +```cpp +struct GameSnapshot { + RoomName player_room; + std::vector player_inventory; + std::map> room_visible_items; + std::map> room_taken_items; + std::vector, std::string>> locked_passages; + std::map npc_dialogue_indices; +}; +``` + +### 6.2 `ISaveManager` interface + +**New file:** `lib/ISaveManager.hpp` + +```cpp +class ISaveManager { + public: + virtual ~ISaveManager() = default; + virtual void save(const GameSnapshot& snapshot) = 0; + virtual std::optional load() = 0; +}; +``` + +Injected into `Game` as a fourth `unique_ptr` parameter alongside `IMap`, `IPlayer`, +`IInputHandler`. + +### 6.3 Concrete `FileSaveManager` + +**New files:** `lib/FileSaveManager.hpp`, `lib/FileSaveManager.cpp` + +Writes a simple line-based text format to `savegame.txt` in the working directory. +No external dependencies (no JSON library). `load()` returns `std::nullopt` if the file +is absent or malformed. + +### 6.4 Interface additions for snapshot extraction + +Extracting and restoring state requires new query/mutator methods on `IMap` and `IPlayer`: + +**`IMap` additions:** +```cpp +virtual std::vector get_room_names() const = 0; +virtual std::vector get_room_inventory(const RoomName& room) const = 0; +virtual std::vector, std::string>> + get_locked_passages() const = 0; +virtual std::map get_npc_dialogue_state() const = 0; +// restore counterparts omitted for brevity — mirror the above as setters +``` + +All additions propagate to `MockMap`, `MockPlayer`, and `MockSaveManager`. + +### 6.5 `Action` additions + +Add `Save` and `Load` to the `Action` enum. Add `"save"` and `"load"` to +`ConsoleInputHandler::get_action()`. + +### 6.6 Testing + +- `handle_save()` → verify `MockSaveManager::save()` is called with a snapshot containing + the expected room and inventory from `MockPlayer` +- `handle_load()` with a valid snapshot → verify mutators are called on `MockMap` and + `MockPlayer` with the correct values +- `handle_load()` with `std::nullopt` → verify a "no save file found" message and no + mutators are called + +--- + +## Implementation Order + +| Phase | Depends on | +|-------|-----------| +| 1 — Fix gaps | — | +| 2 — Expand world | — | +| 3 — Win condition | Phase 2 (Throne Room + chalice) | +| 4 — Locked doors | Phase 2 (Cellar passage + iron key) | +| 5 — NPCs | — (independent) | +| 6 — Save / load | Phases 1–5 (needs final interface shapes) | + +Phases 3, 4, and 5 can be developed in parallel after Phase 2 merges. + +--- + +## Cross-Cutting Rules + +These apply to every phase and are enforced by CI: + +| Concern | Rule | +|---------|------| +| `Action` enum additions | Every `switch(action)` in `Game.cpp` must gain a new `case`; missing cases are clang-tidy errors | +| IWYU | Every new file includes only what it directly uses: ``, ``, ``, `` etc. as needed | +| GMock `NOLINT` | Wrap every mock class in `NOLINTBEGIN/END(misc-non-private-member-variables-in-classes, cppcoreguidelines-non-private-member-variables-in-classes)` | +| Optional access | Annotate the line after `ASSERT_TRUE(opt)` with `// NOLINTNEXTLINE(bugprone-unchecked-optional-access)` | +| Designated initializers | New struct construction sites use `InventoryItem{.name = "x", .use_message = "y"}` syntax | +| Naming | Classes: `CamelCase`, functions/variables: `snake_case`, private members: `_prefixed` | diff --git a/lib/ConsoleInputHandler.cpp b/lib/ConsoleInputHandler.cpp index 27f1d9f..8e0d07d 100644 --- a/lib/ConsoleInputHandler.cpp +++ b/lib/ConsoleInputHandler.cpp @@ -28,6 +28,15 @@ namespace adv_sk { if (action == "take") { return Action::TakeItem; } + if (action == "drop") { + return Action::DropItem; + } + if (action == "inventory") { + return Action::DisplayInventory; + } + if (action == "look") { + return Action::Look; + } return Action::Quit; } diff --git a/lib/ConsoleInputHandler.test.cpp b/lib/ConsoleInputHandler.test.cpp index 0f824a4..bfae3c0 100644 --- a/lib/ConsoleInputHandler.test.cpp +++ b/lib/ConsoleInputHandler.test.cpp @@ -74,6 +74,24 @@ namespace adv_sk::test { EXPECT_EQ(handler.get_action(), Action::UseItem); } + TEST(ConsoleInputHandler, getActionDrop) { + const StreamRedirector redirect("drop\n"); + ConsoleInputHandler handler; + EXPECT_EQ(handler.get_action(), Action::DropItem); + } + + TEST(ConsoleInputHandler, getActionInventory) { + const StreamRedirector redirect("inventory\n"); + ConsoleInputHandler handler; + EXPECT_EQ(handler.get_action(), Action::DisplayInventory); + } + + TEST(ConsoleInputHandler, getActionLook) { + const StreamRedirector redirect("look\n"); + ConsoleInputHandler handler; + EXPECT_EQ(handler.get_action(), Action::Look); + } + TEST(ConsoleInputHandler, getActionUnknownDefaultsToQuit) { const StreamRedirector redirect("unknown\n"); ConsoleInputHandler handler; diff --git a/lib/Game.cpp b/lib/Game.cpp index 0798828..7b5de3f 100644 --- a/lib/Game.cpp +++ b/lib/Game.cpp @@ -49,6 +49,10 @@ namespace adv_sk { display_player_inventory(); break; } + case Action::Look: { + look(); + break; + } default: { _input_handler->provide_message("Command not recognized."); }; @@ -131,6 +135,11 @@ namespace adv_sk { } } + void Game::look() { + update_message(_map->get_welcome_message(_player->get_current_room())); + _input_handler->provide_directions(get_available_directions()); + } + void Game::drop_item(const std::string& item_name) { auto& inventory = _player->get_mutable_inventory(); const auto item = std::ranges::find_if( diff --git a/lib/Game.hpp b/lib/Game.hpp index 32ffaf4..126e53f 100644 --- a/lib/Game.hpp +++ b/lib/Game.hpp @@ -45,6 +45,8 @@ namespace adv_sk { void drop_item(const std::string& item_name); + void look(); + [[nodiscard]] std::vector get_available_directions() const; [[nodiscard]] std::string get_current_message() const { diff --git a/lib/Game.test.cpp b/lib/Game.test.cpp index 236eb8a..fc57acd 100644 --- a/lib/Game.test.cpp +++ b/lib/Game.test.cpp @@ -320,6 +320,40 @@ namespace adv_sk::test { EXPECT_TRUE(game->handle_user_action()); } + TEST_F(GameTest, handleUserActionLook) { + EXPECT_CALL(*mock_input, get_action()).WillOnce(Return(Action::Look)); + EXPECT_CALL(*mock_player, get_current_room()) + .WillRepeatedly(Return("GrandHall")); + EXPECT_CALL(*mock_map, get_welcome_message("GrandHall")) + .WillOnce(Return("Grand Hall description.")); + EXPECT_CALL(*mock_map, next_room("GrandHall", _)) + .WillRepeatedly(Return(std::nullopt)); + EXPECT_CALL(*mock_input, provide_message("Grand Hall description.")); + EXPECT_CALL(*mock_input, provide_directions(_)); + + EXPECT_TRUE(game->handle_user_action()); + } + + TEST_F(GameTest, lookDisplaysRoomDescriptionAndDirections) { + EXPECT_CALL(*mock_player, get_current_room()) + .WillRepeatedly(Return("GrandHall")); + EXPECT_CALL(*mock_map, get_welcome_message("GrandHall")) + .WillOnce(Return("You are in the Grand Hall.")); + EXPECT_CALL(*mock_map, next_room("GrandHall", Direction::North)) + .WillOnce(Return(std::optional("Armoury"))); + EXPECT_CALL(*mock_map, next_room("GrandHall", Direction::South)) + .WillOnce(Return(std::nullopt)); + EXPECT_CALL(*mock_map, next_room("GrandHall", Direction::East)) + .WillOnce(Return(std::nullopt)); + EXPECT_CALL(*mock_map, next_room("GrandHall", Direction::West)) + .WillOnce(Return(std::nullopt)); + EXPECT_CALL(*mock_input, provide_message("You are in the Grand Hall.")); + EXPECT_CALL(*mock_input, + provide_directions(std::vector{Direction::North})); + + game->look(); + } + TEST_F(GameTest, handleUserActionDisplayInventory) { std::vector inv; EXPECT_CALL(*mock_input, get_action()) diff --git a/lib/IInputHandler.hpp b/lib/IInputHandler.hpp index e47ebe2..4f9177e 100644 --- a/lib/IInputHandler.hpp +++ b/lib/IInputHandler.hpp @@ -15,6 +15,7 @@ namespace adv_sk { DisplayInventory, UseItem, DropItem, + Look, Quit, }; diff --git a/lib/Map.cpp b/lib/Map.cpp index 1e52eb2..8ca089e 100644 --- a/lib/Map.cpp +++ b/lib/Map.cpp @@ -44,7 +44,9 @@ namespace adv_sk { } std::unique_ptr create_map() { - InventoryItem const sword("rusty sword"); + InventoryItem const sword{ + .name = "rusty sword", + .use_message = "You swing the rusty sword. The blade holds, but barely.\n"}; InventoryItem const chalice( "golden chalice", "You hold the golden chalice aloft. It glints in the "