From 65a8935e32efeff1ef77ac6923524c003fa450f8 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Tue, 14 Jul 2026 12:38:00 +0200 Subject: [PATCH] fix(loader): resolve every id reference at load time WorldValidator only checked id syntax, startRoom and NPC greetings. Quests, endings and combinations were never even passed to it, so a broken world could load "successfully" and then fail during play: - A room without a 'name' loaded cleanly and threw an NPE in ConsoleIO.frameTop the moment the player walked in. - A typo'd item id in a quest condition silently evaluated to false forever, leaving the stage uncompletable with no diagnostic. The validator now receives quests, endings and combinations, and resolves every item and quest id used in conditions, effects and recipes. Rooms and items must carry a non-blank name and description. All problems are collected and reported together instead of one per run. --- .../jeanluc/adventure/loader/WorldLoader.java | 43 +++- .../adventure/loader/WorldValidator.java | 205 +++++++++++++++++- .../loader/WorldValidatorReferencesTest.java | 107 +++++++++ 3 files changed, 344 insertions(+), 11 deletions(-) create mode 100644 Semesterprojekt/src/test/java/thb/jeanluc/adventure/loader/WorldValidatorReferencesTest.java diff --git a/Semesterprojekt/src/main/java/thb/jeanluc/adventure/loader/WorldLoader.java b/Semesterprojekt/src/main/java/thb/jeanluc/adventure/loader/WorldLoader.java index c2ee7ef..8e1ed32 100644 --- a/Semesterprojekt/src/main/java/thb/jeanluc/adventure/loader/WorldLoader.java +++ b/Semesterprojekt/src/main/java/thb/jeanluc/adventure/loader/WorldLoader.java @@ -122,7 +122,7 @@ public class WorldLoader { resolver.resolveRooms(roomDtos); resolver.resolveNpcs(npcDtos); - new WorldValidator(items, npcs, rooms, gameDto).validate(); + new WorldValidator(items, npcs, rooms, gameDto, quests, endings, combinations).validate(); Room start = rooms.get(gameDto.startRoom()); int gold = gameDto.startGold() == null ? 0 : gameDto.startGold(); @@ -135,6 +135,15 @@ public class WorldLoader { return new LoadResult(world, player); } + /** + * Reads a required YAML list resource. + * + * @param element type + * @param resource classpath path of the YAML file + * @param elementType class of the list elements + * @return the parsed elements, empty if the document is blank + * @throws WorldLoadException if the resource is missing or unparsable + */ private List readList(String resource, Class elementType) { try (InputStream in = openResource(resource)) { CollectionType type = yaml.getTypeFactory().constructCollectionType(List.class, elementType); @@ -145,6 +154,15 @@ public class WorldLoader { } } + /** + * Reads an optional YAML list resource. + * + * @param element type + * @param resource classpath path of the YAML file + * @param elementType class of the list elements + * @return the parsed elements, empty if the resource is absent or blank + * @throws WorldLoadException if the resource exists but is unparsable + */ private List readListOptional(String resource, Class elementType) { try (InputStream in = getClass().getResourceAsStream(resource)) { if (in == null) { @@ -158,6 +176,15 @@ public class WorldLoader { } } + /** + * Reads a required YAML resource holding a single object. + * + * @param target type + * @param resource classpath path of the YAML file + * @param type class to deserialize into + * @return the parsed object + * @throws WorldLoadException if the resource is missing or unparsable + */ private T readSingle(String resource, Class type) { try (InputStream in = openResource(resource)) { return yaml.readValue(in, type); @@ -166,6 +193,13 @@ public class WorldLoader { } } + /** + * Opens a classpath resource. + * + * @param resource classpath path of the resource + * @return an open stream; the caller closes it + * @throws WorldLoadException if the resource is not on the classpath + */ private InputStream openResource(String resource) { InputStream in = getClass().getResourceAsStream(resource); if (in == null) { @@ -174,6 +208,13 @@ public class WorldLoader { return in; } + /** + * Asserts that no id occurs twice. + * + * @param kind label of the entity type, used in the error message + * @param ids the ids to check + * @throws WorldLoadException on the first duplicate id + */ private void requireUniqueIds(String kind, List ids) { for (int i = 0; i < ids.size(); i++) { for (int j = i + 1; j < ids.size(); j++) { diff --git a/Semesterprojekt/src/main/java/thb/jeanluc/adventure/loader/WorldValidator.java b/Semesterprojekt/src/main/java/thb/jeanluc/adventure/loader/WorldValidator.java index 5408924..b42f97d 100644 --- a/Semesterprojekt/src/main/java/thb/jeanluc/adventure/loader/WorldValidator.java +++ b/Semesterprojekt/src/main/java/thb/jeanluc/adventure/loader/WorldValidator.java @@ -1,19 +1,32 @@ package thb.jeanluc.adventure.loader; -import lombok.RequiredArgsConstructor; import thb.jeanluc.adventure.loader.dto.GameDto; +import thb.jeanluc.adventure.model.Combination; +import thb.jeanluc.adventure.model.Condition; +import thb.jeanluc.adventure.model.Effect; +import thb.jeanluc.adventure.model.Ending; import thb.jeanluc.adventure.model.Npc; +import thb.jeanluc.adventure.model.Quest; +import thb.jeanluc.adventure.model.QuestStage; import thb.jeanluc.adventure.model.Room; import thb.jeanluc.adventure.model.item.Item; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; import java.util.Map; import java.util.regex.Pattern; /** * Phase 4 of the loading pipeline. Walks the fully wired world and * checks the invariants documented in {@code yaml-schemas.md}. + * + *

Every reference an id can point at is resolved here — item ids in + * conditions, effects and recipes, quest ids in {@code START_QUEST} — so a + * typo in the world files fails at load time with a precise message rather + * than silently soft-locking a quest or throwing an NPE mid-game. All + * problems found are reported together, not one per run. */ -@RequiredArgsConstructor public class WorldValidator { /** Regex every entity id must match. */ @@ -31,44 +44,216 @@ public class WorldValidator { /** Loaded game.yaml whose {@code startRoom} must exist. */ private final GameDto gameDto; + /** Quest registry; quest ids are the targets of {@code START_QUEST} effects. */ + private final Map quests; + + /** Endings whose trigger conditions must reference known items. */ + private final List endings; + + /** Combination recipes, keyed by item pair. */ + private final Map combinations; + + /** Problems found so far; all are reported in one exception. */ + private final List problems = new ArrayList<>(); + /** - * Runs every validation rule. + * Full constructor covering every entity kind in the world. * - * @throws WorldLoadException on the first failure + * @param items item registry + * @param npcs NPC registry + * @param rooms room registry + * @param gameDto loaded {@code game.yaml} + * @param quests quest registry + * @param endings loaded endings + * @param combinations recipe registry, keyed by {@link Combination#key()} + */ + public WorldValidator(Map items, + Map npcs, + Map rooms, + GameDto gameDto, + Map quests, + List endings, + Map combinations) { + this.items = items; + this.npcs = npcs; + this.rooms = rooms; + this.gameDto = gameDto; + this.quests = quests; + this.endings = endings; + this.combinations = combinations; + } + + /** + * Convenience constructor for a world without quests, endings or recipes. + * + * @param items item registry + * @param npcs NPC registry + * @param rooms room registry + * @param gameDto loaded {@code game.yaml} + */ + public WorldValidator(Map items, + Map npcs, + Map rooms, + GameDto gameDto) { + this(items, npcs, rooms, gameDto, Map.of(), List.of(), Map.of()); + } + + /** + * Runs every validation rule and reports all problems at once. + * + * @throws WorldLoadException if any rule fails */ public void validate() { validateIds(); validateStartRoom(); validateGreetings(); + validateRequiredText(); + validateQuests(); + validateEndings(); + validateCombinations(); + + if (!problems.isEmpty()) { + throw new WorldLoadException("Invalid world:\n - " + String.join("\n - ", problems)); + } } private void validateIds() { items.keySet().forEach(this::requireValidId); npcs.keySet().forEach(this::requireValidId); rooms.keySet().forEach(this::requireValidId); + quests.keySet().forEach(this::requireValidId); } private void requireValidId(String id) { if (id == null || !ID_PATTERN.matcher(id).matches()) { - throw new WorldLoadException("Invalid id '" + id + "': must match " + ID_PATTERN); + problems.add("Invalid id '" + id + "': must match " + ID_PATTERN); } } private void validateStartRoom() { if (gameDto == null || gameDto.startRoom() == null) { - throw new WorldLoadException("game.yaml is missing 'startRoom'"); + problems.add("game.yaml is missing 'startRoom'"); + return; } if (!rooms.containsKey(gameDto.startRoom())) { - throw new WorldLoadException( - "game.yaml.startRoom '" + gameDto.startRoom() + "' does not match any room"); + problems.add("game.yaml.startRoom '" + gameDto.startRoom() + + "' does not match any room"); } } private void validateGreetings() { for (Npc npc : npcs.values()) { - if (npc.getGreeting() == null || npc.getGreeting().isBlank()) { - throw new WorldLoadException("NPC '" + npc.getId() + "' has an empty greeting"); + if (isBlank(npc.getGreeting())) { + problems.add("NPC '" + npc.getId() + "' has an empty greeting"); } } } + + /** + * Names and descriptions are rendered unconditionally by the IO layer, so a + * missing one is an NPE waiting to happen the moment the player walks in. + */ + private void validateRequiredText() { + for (Room room : rooms.values()) { + if (isBlank(room.getName())) { + problems.add("Room '" + room.getId() + "' is missing 'name'"); + } + if (isBlank(room.getDescription())) { + problems.add("Room '" + room.getId() + "' is missing 'description'"); + } + } + for (Item item : items.values()) { + if (isBlank(item.getName())) { + problems.add("Item '" + item.getId() + "' is missing 'name'"); + } + if (isBlank(item.getDescription())) { + problems.add("Item '" + item.getId() + "' is missing 'description'"); + } + } + } + + private void validateQuests() { + for (Quest quest : quests.values()) { + String where = "quest '" + quest.id() + "'"; + if (isBlank(quest.title())) { + problems.add(where + " is missing 'title'"); + } + if (quest.stages().isEmpty()) { + problems.add(where + " has no stages"); + } + checkEffects(quest.onComplete(), where + " onComplete"); + for (int i = 0; i < quest.stages().size(); i++) { + QuestStage stage = quest.stages().get(i); + String stageWhere = where + " stage " + i; + if (isBlank(stage.objective())) { + problems.add(stageWhere + " is missing 'objective'"); + } + checkConditions(stage.completion(), stageWhere + " completion"); + checkEffects(stage.onComplete(), stageWhere + " onComplete"); + } + } + } + + private void validateEndings() { + for (Ending ending : endings) { + String where = "ending '" + ending.id() + "'"; + requireValidId(ending.id()); + if (isBlank(ending.text())) { + problems.add(where + " is missing 'text'"); + } + checkConditions(ending.when(), where + " when"); + } + } + + private void validateCombinations() { + for (Combination c : combinations.values()) { + String where = "combination '" + c.key() + "'"; + requireKnownItem(c.a(), where + " operand 'a'"); + requireKnownItem(c.b(), where + " operand 'b'"); + if (c.produce() != null) { + requireKnownItem(c.produce(), where + " 'produce'"); + } + for (String id : c.consume()) { + requireKnownItem(id, where + " 'consume'"); + } + checkConditions(c.requires(), where + " requires"); + checkEffects(c.effects(), where + " effects"); + } + } + + /** Resolves the item ids referenced by {@code HAS_ITEM} conditions. */ + private void checkConditions(Collection conditions, String where) { + for (Condition c : conditions) { + if (c.type() == Condition.Type.HAS_ITEM) { + requireKnownItem(c.arg(), where); + } + } + } + + /** Resolves the item and quest ids referenced by effects. */ + private void checkEffects(Collection effects, String where) { + for (Effect e : effects) { + switch (e.type()) { + case GIVE_ITEM, REMOVE_ITEM -> requireKnownItem(e.arg(), where); + case START_QUEST -> { + if (!quests.containsKey(e.arg())) { + problems.add(where + " starts unknown quest '" + e.arg() + "'"); + } + } + default -> { + // SET_FLAG / CLEAR_FLAG / SAY carry free-form text. + } + } + } + } + + private void requireKnownItem(String id, String where) { + if (!items.containsKey(id)) { + problems.add(where + " references unknown item '" + id + "'"); + } + } + + private static boolean isBlank(String s) { + return s == null || s.isBlank(); + } } diff --git a/Semesterprojekt/src/test/java/thb/jeanluc/adventure/loader/WorldValidatorReferencesTest.java b/Semesterprojekt/src/test/java/thb/jeanluc/adventure/loader/WorldValidatorReferencesTest.java new file mode 100644 index 0000000..7290d20 --- /dev/null +++ b/Semesterprojekt/src/test/java/thb/jeanluc/adventure/loader/WorldValidatorReferencesTest.java @@ -0,0 +1,107 @@ +package thb.jeanluc.adventure.loader; + +import org.junit.jupiter.api.Test; +import thb.jeanluc.adventure.loader.dto.GameDto; +import thb.jeanluc.adventure.model.Condition; +import thb.jeanluc.adventure.model.Effect; +import thb.jeanluc.adventure.model.Npc; +import thb.jeanluc.adventure.model.Quest; +import thb.jeanluc.adventure.model.QuestStage; +import thb.jeanluc.adventure.model.Room; +import thb.jeanluc.adventure.model.item.Item; +import thb.jeanluc.adventure.model.item.PlainItem; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Dangling ids and missing text used to load cleanly and only blow up (or + * silently soft-lock) at play time. The validator must reject them up front. + */ +class WorldValidatorReferencesTest { + + private Map items() { + Map m = new HashMap<>(); + m.put("lamp", PlainItem.builder().id("lamp").name("Lamp").description("d").build()); + return m; + } + + private Map rooms() { + Map m = new HashMap<>(); + m.put("kitchen", new Room("kitchen", "Kitchen", "d")); + return m; + } + + private Map npcs() { + Map m = new HashMap<>(); + m.put("man", Npc.shell("man", "Man", "d", "hi")); + return m; + } + + private GameDto game() { + return new GameDto("t", "1", "kitchen", 0, "w"); + } + + private WorldValidator validator(Map rooms, Map quests) { + return new WorldValidator(items(), npcs(), rooms, game(), quests, List.of(), Map.of()); + } + + private Map questWithCompletion(List completion) { + Quest q = new Quest("main", "Main", true, + List.of(new QuestStage("do it", completion, List.of())), List.of()); + return Map.of("main", q); + } + + @Test + void validate_withTypoedItemIdInQuestCondition_throws() { + var quests = questWithCompletion( + List.of(new Condition(Condition.Type.HAS_ITEM, "lampp"))); + + assertThatThrownBy(() -> validator(rooms(), quests).validate()) + .isInstanceOf(WorldLoadException.class) + .hasMessageContaining("unknown item 'lampp'"); + } + + @Test + void validate_withKnownItemIdInQuestCondition_doesNotThrow() { + var quests = questWithCompletion( + List.of(new Condition(Condition.Type.HAS_ITEM, "lamp"))); + + assertThatCode(() -> validator(rooms(), quests).validate()).doesNotThrowAnyException(); + } + + @Test + void validate_withUnknownQuestInStartQuestEffect_throws() { + Quest q = new Quest("main", "Main", true, + List.of(new QuestStage("do it", List.of(), List.of())), + List.of(new Effect(Effect.Type.START_QUEST, "ghost_quest"))); + + assertThatThrownBy(() -> validator(rooms(), Map.of("main", q)).validate()) + .isInstanceOf(WorldLoadException.class) + .hasMessageContaining("ghost_quest"); + } + + @Test + void validate_withRoomMissingName_throws() { + Map rooms = new HashMap<>(); + rooms.put("kitchen", new Room("kitchen", null, "d")); + + assertThatThrownBy(() -> validator(rooms, Map.of()).validate()) + .isInstanceOf(WorldLoadException.class) + .hasMessageContaining("missing 'name'"); + } + + @Test + void validate_withRoomMissingDescription_throws() { + Map rooms = new HashMap<>(); + rooms.put("kitchen", new Room("kitchen", "Kitchen", " ")); + + assertThatThrownBy(() -> validator(rooms, Map.of()).validate()) + .isInstanceOf(WorldLoadException.class) + .hasMessageContaining("missing 'description'"); + } +}