From 4e1116ae2db0cef7fc1cd4b22a3731142ad0b431 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Sun, 28 Apr 2024 16:39:33 +0200
Subject: [PATCH] Gracefully handle bad mod setting types in Web UI

Make sure all cobinations of missmatched types and missing mod settings
are gracefully handled by the Web UI.

Fixes #609.
---
 CHANGELOG.md                                  |  1 +
 package.json                                  |  2 +-
 .../web_ui/src/components/ModPackViewPage.tsx | 12 +++++--
 test/manual/README.md                         | 35 +++++++++++++++++++
 test/manual/test_mod/info.json                |  7 ++++
 test/manual/test_mod/settings.lua             | 22 ++++++++++++
 test/manual/test_settings_mod_pack.js         | 28 +++++++++++++++
 7 files changed, 104 insertions(+), 3 deletions(-)
 create mode 100644 test/manual/README.md
 create mode 100644 test/manual/test_mod/info.json
 create mode 100644 test/manual/test_mod/settings.lua
 create mode 100644 test/manual/test_settings_mod_pack.js

diff --git a/CHANGELOG.md b/CHANGELOG.md
index eb8459f9..9dd176b9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -38,6 +38,7 @@ Many thanks to the following for contributing to this release:
 - Suppresed bogus warning about event listener leak in the browser's console [#600](https://github.com/clusterio/clusterio/pull/600).
 - Fixed installer breaking on Windows after Node.js released a security fix [#614](https://github.com/clusterio/clusterio/pull/614).
 - During a data export the mod settings in mod packs will now be corrected to the type of the settings prototype if the type is incorrect.
+- Fixed missing color setting causing the Mod Pack view in the Web UI to show an error [#609](https://github.com/clusterio/clusterio/issues/609).
 
 Many thanks to the following for contributing to this release:  
 [@Cooldude2606](https://github.com/Cooldude2606)
diff --git a/package.json b/package.json
index a5f4a506..e62b1c7f 100644
--- a/package.json
+++ b/package.json
@@ -4,7 +4,7 @@
 	"scripts": {
 		"build": "tsc --build",
 		"watch": "tsc --build --watch",
-		"test": "pnpm run build && mocha --check-leaks -R spec --recursive test \"plugins/*/test/**\"",
+		"test": "pnpm run build && mocha --check-leaks -R spec --recursive test \"plugins/*/test/**\" --exclude \"test/manual/**\"",
 		"fast-test": "FAST_TEST=y pnpm test",
 		"cover": "nyc pnpm test",
 		"fast-cover": "FAST_TEST=y nyc pnpm test",
diff --git a/packages/web_ui/src/components/ModPackViewPage.tsx b/packages/web_ui/src/components/ModPackViewPage.tsx
index 840155f2..93f07889 100644
--- a/packages/web_ui/src/components/ModPackViewPage.tsx
+++ b/packages/web_ui/src/components/ModPackViewPage.tsx
@@ -424,7 +424,15 @@ function InputColor(props: {
 	value: lib.ModSettingColor,
 	onChange: (value: lib.ModSettingColor) => void,
 }) {
-	const value = props.value;
+	// In certain edge cases it's possible a value for this mod setting does not exist
+	// or is of the wrong type. Fallback to black if this is the case.
+	function isColor(input: unknown) {
+		if (typeof input !== "object" || input === null) {
+			return false;
+		}
+		return "r" in input && "g" in input && "b" in input && "a" in input;
+	}
+	const value = isColor(props.value) ? props.value : { r: 0, g: 0, b: 0, a: 1 };
 	return <ColorPicker
 		defaultFormat="rgb"
 		defaultValue={`rgba(${value.r * 255}, ${value.g * 255}, ${value.b * 255}, ${value.a}`}
@@ -774,7 +782,7 @@ export default function ModPackViewPage() {
 						continue;
 					}
 				}
-				if (modifiedModPack!.settings[scope].get(settingName)!.value !== value) {
+				if (modifiedModPack!.settings[scope].get(settingName)?.value !== value) {
 					pushChange({ type: "settings.set", scope, name: settingName, value: { value }});
 				}
 			}
diff --git a/test/manual/README.md b/test/manual/README.md
new file mode 100644
index 00000000..3b24dc23
--- /dev/null
+++ b/test/manual/README.md
@@ -0,0 +1,35 @@
+# Manual Tests
+
+This folder contains files used for performing manual tests that are non-trivial to automate.
+
+## Setup test mod
+
+Run `node packages/lib/dist/node/build_mod.js --source-dir test/manual/test_mod/ --output-dir mods/` to create the test mod.
+
+## Test handling of bad setting values
+
+1.  Create a ModPack with missmatched values with `node test/manual/test_settings_mod_pack.js` and import it.
+2.  Run a data export on an instance with the mod pack assigned, the values will get coerced to the correct types.
+3.  Use ctl to set the settings back to their wrong values:
+
+    ```bash
+    SETTING_TYPES=(bool-setting int-setting double-setting string-setting color-setting)
+    VALUE_TYPES=(bool int double string color missing)
+    VALUES=(
+        --bool-setting false
+        --int-setting 1
+        --double-setting 0.5
+        --string-setting str
+        --color-setting '{"r":0,"g":1,"b":0,"a":1}'
+        --remove-setting
+    )
+    node packages/ctl mod-pack edit test-settings $(
+        for TYPE in "${SETTING_TYPES[@]}" ; do
+            for (( I = 0; I < ${#VALUE_TYPES[@]}; I++ )); do
+                printf "%s startup %s-with-%s-value %s " ${VALUES[((I * 2))]} $TYPE ${VALUE_TYPES[I]} ${VALUES[((I * 2 + 1))]}
+            done
+        done
+    )
+    ```
+
+4.  Verify that the Web UI gracefully handles all combinations of wrong settings types.
diff --git a/test/manual/test_mod/info.json b/test/manual/test_mod/info.json
new file mode 100644
index 00000000..7d3cf54d
--- /dev/null
+++ b/test/manual/test_mod/info.json
@@ -0,0 +1,7 @@
+{
+	"name": "test_mod",
+	"version": "0.0.0",
+	"title": "test_mod",
+	"author": "",
+	"factorio_version": "1.1"
+}
diff --git a/test/manual/test_mod/settings.lua b/test/manual/test_mod/settings.lua
new file mode 100644
index 00000000..efcdd49c
--- /dev/null
+++ b/test/manual/test_mod/settings.lua
@@ -0,0 +1,22 @@
+local settings = {
+	["bool-setting"] = true,
+	["int-setting"] = 123,
+	["double-setting"] = 1234.5,
+	["string-setting"] = "string",
+	["color-setting"] = { r = 1, g = 0, b = 1, a = 1 },
+}
+
+local possible_values = { "missing", "bool", "int", "double", "string", "color" }
+
+for name, default_value in pairs(settings) do
+	for _, value in ipairs(possible_values) do
+		data:extend({
+			{
+				type = name,
+				name = name .. "-with-" .. value .. "-value",
+				setting_type = "startup",
+				default_value = default_value,
+			}
+		})
+	end
+end
diff --git a/test/manual/test_settings_mod_pack.js b/test/manual/test_settings_mod_pack.js
new file mode 100644
index 00000000..fe2a699f
--- /dev/null
+++ b/test/manual/test_settings_mod_pack.js
@@ -0,0 +1,28 @@
+"use strict";
+const { ModPack } = require("@clusterio/lib");
+
+const pack = new ModPack();
+pack.name = "test-settings";
+pack.mods.set("test_mod", { name: "test_mod", enabled: true, version: "0.0.0" });
+const settingTypes = [
+	"bool-setting",
+	"int-setting",
+	"double-setting",
+	"string-setting",
+	"color-setting",
+];
+const settingValues = {
+	"bool": false,
+	"int": 1,
+	"double": 0.5,
+	"string": "str",
+	"color": { r: 0, g: 1, b: 0, a: 1 },
+	// "missing":
+};
+for (const type of settingTypes) {
+	for (const [valueType, value] of Object.entries(settingValues)) {
+		pack.settings.startup.set(`${type}-with-${valueType}-value`, { value });
+	}
+}
+/* eslint-disable no-console */
+console.log(pack.toModPackString());