diff --git a/CHANGELOG.md b/CHANGELOG.md index f5b001ac..9dd176b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,8 @@ Many thanks to the following for contributing to this release: - Fixed plugin events being invoked while a host connection was in an invalid state. - 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/lib/src/data/ModPack.ts b/packages/lib/src/data/ModPack.ts index 85dba1fb..4321fee5 100644 --- a/packages/lib/src/data/ModPack.ts +++ b/packages/lib/src/data/ModPack.ts @@ -334,10 +334,11 @@ export default class ModPack { } /** - * Fill in missing settings with their default values + * Fill in missing and incorrect settings with their default values * - * Uses the provided setting prototypes to add any missing mod settings - * in the mod pack with the default value from the prototype. + * Uses the provided setting prototypes to correct any missing and or + * incorrect mod settings in the mod pack with the default value from + * the prototype. * * @param settingPrototypes - * Setting prototypes exported from the game. @@ -360,13 +361,23 @@ export default class ModPack { logger.warn(`Ignoring ${prototype.name} with missing default_value`); continue; } - if (this.settings[settingType].get(prototype.name)) { - continue; + let value = this.settings[settingType].get(prototype.name)?.value ?? prototype.default_value; + const type = prototype.type; + if ( + type === "bool-setting" && typeof value !== "boolean" + || type === "color-setting" && typeof value !== "object" + || (type === "int-setting" || type === "double-setting") && typeof value !== "number" + ) { + value = prototype.default_value; } - - let value = prototype.default_value; - if (prototype.type === "color-setting") { - value = libLuaTools.normalizeColor(value); + if (type === "string-setting" && typeof value !== "string") { + value = typeof value === "object" ? prototype.default_value : String(value ?? ""); + } + if (type === "color-setting") { + if (typeof value !== "object") { + value = prototype.default_value; + } + value = libLuaTools.normalizeColor(value as object); } this.settings[settingType].set(prototype.name, { value }); } 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 [k, { value: v }])), + ); + }); }); describe(".fromModPackString()", function() { 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());