From 0f1c119c76b78e894ca7c21dc14ed14a3ca0e5a5 Mon Sep 17 00:00:00 2001
From: Sam Merritt <spam@andcheese.org>
Date: Mon, 13 Jan 2025 00:52:21 -0800
Subject: [PATCH] Factorio: improve error message for config validation (#4421)

---
 worlds/factorio/Options.py              | 14 +++++++--
 worlds/factorio/test_file_validation.py | 39 +++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 2 deletions(-)
 create mode 100644 worlds/factorio/test_file_validation.py

diff --git a/worlds/factorio/Options.py b/worlds/factorio/Options.py
index 72f43877..0fa75e1b 100644
--- a/worlds/factorio/Options.py
+++ b/worlds/factorio/Options.py
@@ -3,13 +3,23 @@ from __future__ import annotations
 from dataclasses import dataclass
 import typing
 
-from schema import Schema, Optional, And, Or
+from schema import Schema, Optional, And, Or, SchemaError
 
 from Options import Choice, OptionDict, OptionSet, DefaultOnToggle, Range, DeathLink, Toggle, \
     StartInventoryPool, PerGameCommonOptions, OptionGroup
 
 # schema helpers
-FloatRange = lambda low, high: And(Or(int, float), lambda f: low <= f <= high)
+class FloatRange:
+    def __init__(self, low, high):
+        self._low = low
+        self._high = high
+
+    def validate(self, value):
+        if not isinstance(value, (float, int)):
+            raise SchemaError(f"should be instance of float or int, but was {value!r}")
+        if not self._low <= value <= self._high:
+            raise SchemaError(f"{value} is not between {self._low} and {self._high}")
+
 LuaBool = Or(bool, And(int, lambda n: n in (0, 1)))
 
 
diff --git a/worlds/factorio/test_file_validation.py b/worlds/factorio/test_file_validation.py
new file mode 100644
index 00000000..df56ec60
--- /dev/null
+++ b/worlds/factorio/test_file_validation.py
@@ -0,0 +1,39 @@
+"""Tests for error messages from YAML validation."""
+
+import os
+import unittest
+
+import WebHostLib.check
+
+FACTORIO_YAML="""
+game: Factorio
+Factorio:
+  world_gen:
+    autoplace_controls:
+      coal:
+        richness: 1
+        frequency: {}
+        size: 1
+"""
+
+def yamlWithFrequency(f):
+    return FACTORIO_YAML.format(f)
+
+
+class TestFileValidation(unittest.TestCase):
+    def test_out_of_range(self):
+        results, _ = WebHostLib.check.roll_options({"bob.yaml": yamlWithFrequency(1000)})
+        self.assertIn("between 0 and 6", results["bob.yaml"])
+
+    def test_bad_non_numeric(self):
+        results, _ = WebHostLib.check.roll_options({"bob.yaml": yamlWithFrequency("not numeric")})
+        self.assertIn("float", results["bob.yaml"])
+        self.assertIn("int", results["bob.yaml"])
+
+    def test_good_float(self):
+        results, _ = WebHostLib.check.roll_options({"bob.yaml": yamlWithFrequency(1.0)})
+        self.assertIs(results["bob.yaml"], True)
+
+    def test_good_int(self):
+        results, _ = WebHostLib.check.roll_options({"bob.yaml": yamlWithFrequency(1)})
+        self.assertIs(results["bob.yaml"], True)