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.
This commit is contained in:
@@ -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 <T> 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 <T> List<T> readList(String resource, Class<T> 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 <T> 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 <T> List<T> readListOptional(String resource, Class<T> 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 <T> 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> T readSingle(String resource, Class<T> 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<String> ids) {
|
||||
for (int i = 0; i < ids.size(); i++) {
|
||||
for (int j = i + 1; j < ids.size(); j++) {
|
||||
|
||||
@@ -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}.
|
||||
*
|
||||
* <p>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<String, Quest> quests;
|
||||
|
||||
/** Endings whose trigger conditions must reference known items. */
|
||||
private final List<Ending> endings;
|
||||
|
||||
/** Combination recipes, keyed by item pair. */
|
||||
private final Map<String, Combination> combinations;
|
||||
|
||||
/** Problems found so far; all are reported in one exception. */
|
||||
private final List<String> 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<String, Item> items,
|
||||
Map<String, Npc> npcs,
|
||||
Map<String, Room> rooms,
|
||||
GameDto gameDto,
|
||||
Map<String, Quest> quests,
|
||||
List<Ending> endings,
|
||||
Map<String, Combination> 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<String, Item> items,
|
||||
Map<String, Npc> npcs,
|
||||
Map<String, Room> 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<Condition> 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<Effect> 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();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<String, Item> items() {
|
||||
Map<String, Item> m = new HashMap<>();
|
||||
m.put("lamp", PlainItem.builder().id("lamp").name("Lamp").description("d").build());
|
||||
return m;
|
||||
}
|
||||
|
||||
private Map<String, Room> rooms() {
|
||||
Map<String, Room> m = new HashMap<>();
|
||||
m.put("kitchen", new Room("kitchen", "Kitchen", "d"));
|
||||
return m;
|
||||
}
|
||||
|
||||
private Map<String, Npc> npcs() {
|
||||
Map<String, Npc> 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<String, Room> rooms, Map<String, Quest> quests) {
|
||||
return new WorldValidator(items(), npcs(), rooms, game(), quests, List.of(), Map.of());
|
||||
}
|
||||
|
||||
private Map<String, Quest> questWithCompletion(List<Condition> 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<String, Room> 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<String, Room> rooms = new HashMap<>();
|
||||
rooms.put("kitchen", new Room("kitchen", "Kitchen", " "));
|
||||
|
||||
assertThatThrownBy(() -> validator(rooms, Map.of()).validate())
|
||||
.isInstanceOf(WorldLoadException.class)
|
||||
.hasMessageContaining("missing 'description'");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user