From 3cf9f9623321adcd3cc582bffae26126593cdb01 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sun, 28 Apr 2024 16:35:08 +0200 Subject: [PATCH 1/2] Coerce mod settings types during data export Use the settings prototypes to coerce any incorrectly typed mod settings to the correct type either by casting or by replacing the value with the default value from the prototype. This will fix cases where an update to a mod changes the type of ono of its defined settings. --- CHANGELOG.md | 1 + packages/lib/src/data/ModPack.ts | 29 ++++++++---- test/lib/data/ModPack.js | 78 ++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5b001ac..eb8459f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ 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. Many thanks to the following for contributing to this release: [@Cooldude2606](https://github.com/Cooldude2606) 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/test/lib/data/ModPack.js b/test/lib/data/ModPack.js index 14616da3..d1fc713e 100644 --- a/test/lib/data/ModPack.js +++ b/test/lib/data/ModPack.js @@ -179,6 +179,84 @@ describe("lib/data/ModPack", function() { "runtime-per-user": {}, }); }); + it("should coerce values of the incorrect type into the correct type", function() { + const boolDefault = true; + const intDefault = 123; + const doubleDefault = 1234.5; + const stringDefault = "default"; + const colorDefault = { r: 1, g: 0, b: 1, a: 1}; + const settingDefaultValues = { + "bool-setting": boolDefault, + "int-setting": intDefault, + "double-setting": doubleDefault, + "string-setting": stringDefault, + "color-setting": colorDefault, + }; + const possibleValues = ["bool", "int", "double", "string", "color"]; + const settingPrototypes = { + "bool-setting": {}, + "int-setting": {}, + "double-setting": {}, + "string-setting": {}, + "color-setting": {}, + }; + for (const [type, default_value] of Object.entries(settingDefaultValues)) { + for (const value of possibleValues) { + const settingName = `${type}-with-${value}-value`; + settingPrototypes[type][settingName] = { + type, + name: settingName, + setting_type: "startup", + default_value, + }; + } + }; + + const pack = new ModPack(); + const settingValues = { + "bool": false, + "int": 1, + "double": 2.5, + "string": "str", + "color": { r: 0, g: 1, b: 0, a: 1 }, + }; + for (const type of Object.keys(settingDefaultValues)) { + for (const [valueType, value] of Object.entries(settingValues)) { + pack.settings.startup.set(`${type}-with-${valueType}-value`, { value }); + } + } + pack.fillDefaultSettings(settingPrototypes); + assert.deepEqual( + pack.settings["startup"], + new Map([ + ["bool-setting-with-bool-value", false], + ["bool-setting-with-int-value", boolDefault], + ["bool-setting-with-double-value", boolDefault], + ["bool-setting-with-string-value", boolDefault], + ["bool-setting-with-color-value", boolDefault], + ["int-setting-with-bool-value", intDefault], + ["int-setting-with-int-value", 1], + ["int-setting-with-double-value", 2.5], + ["int-setting-with-string-value", intDefault], + ["int-setting-with-color-value", intDefault], + ["double-setting-with-bool-value", doubleDefault], + ["double-setting-with-int-value", 1], + ["double-setting-with-double-value", 2.5], + ["double-setting-with-string-value", doubleDefault], + ["double-setting-with-color-value", doubleDefault], + ["string-setting-with-bool-value", "false"], + ["string-setting-with-int-value", "1"], + ["string-setting-with-double-value", "2.5"], + ["string-setting-with-string-value", "str"], + ["string-setting-with-color-value", stringDefault], + ["color-setting-with-bool-value", colorDefault], + ["color-setting-with-int-value", colorDefault], + ["color-setting-with-double-value", colorDefault], + ["color-setting-with-string-value", colorDefault], + ["color-setting-with-color-value", { r: 0, g: 1, b: 0, a: 1}], + ].map(([k, v]) => [k, { value: v }])), + ); + }); }); describe(".fromModPackString()", function() { From 4e1116ae2db0cef7fc1cd4b22a3731142ad0b431 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sun, 28 Apr 2024 16:39:33 +0200 Subject: [PATCH 2/2] 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