From 96082e32b7e25c0393057c7b78a7b2c71b5d1a31 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Wed, 7 Feb 2024 16:18:33 +0100
Subject: [PATCH 1/5] Replace deprecated usage of Select bordered

---
 packages/web_ui/src/components/ModPackViewPage.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/web_ui/src/components/ModPackViewPage.tsx b/packages/web_ui/src/components/ModPackViewPage.tsx
index 97f13d03..840155f2 100644
--- a/packages/web_ui/src/components/ModPackViewPage.tsx
+++ b/packages/web_ui/src/components/ModPackViewPage.tsx
@@ -202,7 +202,7 @@ function SearchModsTable(props: SearchModsTableProps) {
 						align: "right",
 						render: (_, result) => <Select
 							size="small"
-							bordered={false}
+							variant="borderless"
 							value={modResultSelectedVersion.get(result.name) || 0}
 							onChange={index => {
 								const newVersions = new Map(modResultSelectedVersion);

From 1021e29c98d37674db78c4fb26c2e96fde4f4210 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Thu, 8 Feb 2024 18:30:59 +0100
Subject: [PATCH 2/5] Support shorthand notation for colours

Implement the shorthand notations that Factorio recognises for colours
where individual components may be left out or the colour specified as
an array of values instead of an object.
---
 .eslintrc.js                       |  2 +-
 CHANGELOG.md                       |  1 +
 packages/host/src/export.ts        | 13 ++------
 packages/lib/browser.ts            |  1 +
 packages/lib/src/data/ModPack.ts   |  7 ++++-
 packages/lib/src/lua_tools.ts      | 48 ++++++++++++++++++++++++++++++
 packages/lib/tsconfig.browser.json |  1 +
 test/lib/lua_tools.js              | 19 ++++++++++++
 8 files changed, 80 insertions(+), 12 deletions(-)

diff --git a/.eslintrc.js b/.eslintrc.js
index d4cbf768..0ff9374b 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -320,7 +320,7 @@ module.exports = {
 		"semi-style": "off",
 		"sort-imports": "off",
 		"sort-keys": "off",
-		"sort-vars": "error",
+		"sort-vars": "off",
 		"space-before-blocks": "error",
 		"space-before-function-paren": "off",
 		"space-in-parens": ["error", "never"],
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cac58b2a..944084bf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@
 - Fixed Web UI not showing instance saves when the permission to list and subscribe the saves is granted but the user is not a cluster admin.
 - Similarly fixed Web UI not showing the mod pack create, delete, list and host revoke token actions if the user is not a cluster admin.
 - Fixed Research Sync applying progress of a previous techology level to the level on the controller when it's the active research during instance startup.
+- Fixed data export with a mod using shorthand notation for the default value of a color-setting causing the ModPack data to be set to invalid data.
 
 
 ## Version 2.0.0-alpha.16
diff --git a/packages/host/src/export.ts b/packages/host/src/export.ts
index 938836d4..337a89f5 100644
--- a/packages/host/src/export.ts
+++ b/packages/host/src/export.ts
@@ -18,11 +18,10 @@ interface SimpleIconSpecification {
 	icon_size: number;
 	icon_mipmaps?: number;
 }
-type Color = { r: number, g: number, b: number, a: number };
 interface IconLayer {
 	icon: string;
 	icon_size: number;
-	tint?: Color,
+	tint?: lib.FactorioColor,
 	shift?: [number, number],
 	scale?: number,
 	icon_mipmaps?: number,
@@ -202,15 +201,9 @@ async function loadLayeredIcon(
 
 		iconLayer = iconLayer.clone();
 
-		let tint: Color;
+		let tint: lib.Color;
 		if (layer.tint) {
-			let divisor = (layer.tint.r > 1 || layer.tint.g > 1 || layer.tint.b > 1) ? 255 : 1;
-			tint = {
-				r: (layer.tint.r || 0) / divisor,
-				g: (layer.tint.g || 0) / divisor,
-				b: (layer.tint.b || 0) / divisor,
-				a: layer.tint.a || 1,
-			};
+			tint = lib.normalizeColor(layer.tint);
 		} else {
 			tint = { r: 1, b: 1, g: 1, a: 1 };
 		}
diff --git a/packages/lib/browser.ts b/packages/lib/browser.ts
index 5c430809..97525776 100644
--- a/packages/lib/browser.ts
+++ b/packages/lib/browser.ts
@@ -6,6 +6,7 @@ export * from "./src/factorio/exchange_string";
 export * from "./src/helpers";
 export * from "./src/ini";
 export * from "./src/link";
+export * from "./src/lua_tools";
 export * from "./src/logging";
 export * from "./src/permissions";
 export * from "./src/plugin";
diff --git a/packages/lib/src/data/ModPack.ts b/packages/lib/src/data/ModPack.ts
index 094addb4..85dba1fb 100644
--- a/packages/lib/src/data/ModPack.ts
+++ b/packages/lib/src/data/ModPack.ts
@@ -2,6 +2,7 @@ import zlib from "zlib";
 import { Type, Static } from "@sinclair/typebox";
 
 import * as libSchema from "../schema";
+import * as libLuaTools from "../lua_tools";
 
 import ExportManifest from "./ExportManifest";
 import { integerFactorioVersion } from "./version";
@@ -363,7 +364,11 @@ export default class ModPack {
 				continue;
 			}
 
-			this.settings[settingType].set(prototype.name, { value: prototype.default_value });
+			let value = prototype.default_value;
+			if (prototype.type === "color-setting") {
+				value = libLuaTools.normalizeColor(value);
+			}
+			this.settings[settingType].set(prototype.name, { value });
 		}
 	}
 }
diff --git a/packages/lib/src/lua_tools.ts b/packages/lib/src/lua_tools.ts
index b9dbba13..007f9a16 100644
--- a/packages/lib/src/lua_tools.ts
+++ b/packages/lib/src/lua_tools.ts
@@ -3,6 +3,54 @@
  * @module lib/lua_tools
  */
 
+/**
+ * A colour as it may originate from Factorio Lua code, see
+ * https://lua-api.factorio.com/latest/types/Color.html
+ */
+export type FactorioColor = { r?: number, g?: number, b?: number, a?: number } | [number, number, number, number?];
+
+export type Color = { r: number, g: number, b: number, a: number };
+
+/**
+ * Normalise a color as it may appear in Factorio Lua code to be an object
+ * with r, g, b, a properties in the range 0 to 1.
+ *
+ * @param color -
+ *     A colour that may be specified with left out components or as an
+ *     array and have the range 1 to 0 or 255 to 0.
+ * @returns
+ *     Normalised colour where all 4 components are present as properties in
+ *     the range 0 to 1.
+ */
+export function normalizeColor(color: FactorioColor) {
+	let r, g, b, a;
+	if (color instanceof Array) {
+		([r, g, b, a] = color);
+	} else {
+		({ r, g, b, a } = color);
+	}
+
+	r ??= NaN;
+	g ??= NaN;
+	b ??= NaN;
+	a ??= NaN;
+
+	// Note NaN > 1 is false and NaN / 255 is NaN
+	if (r > 1 || g > 1 || b > 1 || a > 1) {
+		r /= 255;
+		g /= 255;
+		b /= 255;
+		a /= 255;
+	}
+
+	return {
+		r: Number.isNaN(r) ? 0 : r,
+		g: Number.isNaN(g) ? 0 : g,
+		b: Number.isNaN(b) ? 0 : b,
+		a: Number.isNaN(a) ? 1 : a,
+	};
+}
+
 /**
  * Escapes a string for inclusion into a lua string
  *
diff --git a/packages/lib/tsconfig.browser.json b/packages/lib/tsconfig.browser.json
index a53b11d7..e3c20c4e 100644
--- a/packages/lib/tsconfig.browser.json
+++ b/packages/lib/tsconfig.browser.json
@@ -12,6 +12,7 @@
 		"./src/hash.ts",
 		"./src/ini.ts",
 		"./src/link/*.ts",
+		"./src/lua_tools.ts",
 		"./src/logging.ts",
 		"./src/permissions.ts",
 		"./src/prometheus.ts",
diff --git a/test/lib/lua_tools.js b/test/lib/lua_tools.js
index 8fcbac74..8b70b274 100644
--- a/test/lib/lua_tools.js
+++ b/test/lib/lua_tools.js
@@ -5,6 +5,25 @@ const lib = require("@clusterio/lib");
 
 
 describe("lib/lua_tools", function() {
+	describe("normalizeColor()", function() {
+		it("should normalise colours according to the documentation", function() {
+			const nc = lib.normalizeColor;
+			assert.deepEqual(nc({ r: 1, g: 0.2, b: 0.4, a: 0.8 }), { r: 1, g: 0.2, b: 0.4, a: 0.8 });
+			assert.deepEqual(nc({ r: 1, g: 0.2, b: 0.4 }), { r: 1, g: 0.2, b: 0.4, a: 1 });
+			assert.deepEqual(nc({ r: 1, b: 0.4 }), { r: 1, g: 0, b: 0.4, a: 1 });
+			assert.deepEqual(nc({ a: 0.4 }), { r: 0, g: 0, b: 0, a: 0.4 });
+			assert.deepEqual(nc({}), { r: 0, g: 0, b: 0, a: 1 });
+			assert.deepEqual(nc([1, 0.2, 0.4, 0.8]), { r: 1, g: 0.2, b: 0.4, a: 0.8 });
+			assert.deepEqual(nc([1, 0.2, 0.4]), { r: 1, g: 0.2, b: 0.4, a: 1 });
+			assert.deepEqual(nc({ r: 255, g: 51, b: 102, a: 204 }), { r: 1, g: 0.2, b: 0.4, a: 0.8 });
+			assert.deepEqual(nc({ r: 255, g: 51, b: 102 }), { r: 1, g: 0.2, b: 0.4, a: 1 });
+			assert.deepEqual(nc({ r: 255, b: 102 }), { r: 1, g: 0, b: 0.4, a: 1 });
+			assert.deepEqual(nc({ a: 102 }), { r: 0, g: 0, b: 0, a: 0.4 });
+			assert.deepEqual(nc({}), { r: 0, g: 0, b: 0, a: 1 });
+			assert.deepEqual(nc([255, 51, 102, 204]), { r: 1, g: 0.2, b: 0.4, a: 0.8 });
+			assert.deepEqual(nc([255, 51, 102]), { r: 1, g: 0.2, b: 0.4, a: 1 });
+		});
+	});
 	describe("escapeString()", function() {
 		it("should escape backslashes", function() {
 			assert.equal(lib.escapeString("\\a"), "\\\\a");

From 886550901172c80eca0b6b994e04b68c1c9c7e84 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Thu, 8 Feb 2024 19:06:21 +0100
Subject: [PATCH 3/5] Delete player statistics when deleting instances

Clear the statistics stored per user for the instance when an instance
is deleted.
---
 CHANGELOG.md                           |  1 +
 packages/controller/src/Controller.ts  |  1 +
 packages/controller/src/UserManager.ts |  9 +++++++++
 test/controller/UserManager.js         | 24 ++++++++++++++++++++++++
 4 files changed, 35 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 944084bf..80b4f549 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@
 - Similarly fixed Web UI not showing the mod pack create, delete, list and host revoke token actions if the user is not a cluster admin.
 - Fixed Research Sync applying progress of a previous techology level to the level on the controller when it's the active research during instance startup.
 - Fixed data export with a mod using shorthand notation for the default value of a color-setting causing the ModPack data to be set to invalid data.
+- Fixed per user per instance player stats not being deleted when the instance it's for is deleted.
 
 
 ## Version 2.0.0-alpha.16
diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts
index 7bd67a52..4c98efc3 100644
--- a/packages/controller/src/Controller.ts
+++ b/packages/controller/src/Controller.ts
@@ -964,6 +964,7 @@ export default class Controller {
 		this.instancesDirty = true;
 
 		this.clearSavesOfInstance(instanceId);
+		this.userManager.clearStatsOfInstance(instanceId);
 
 		let prev = instance.status;
 		instance.status = "deleted";
diff --git a/packages/controller/src/UserManager.ts b/packages/controller/src/UserManager.ts
index bb3cde0a..e9239428 100644
--- a/packages/controller/src/UserManager.ts
+++ b/packages/controller/src/UserManager.ts
@@ -113,4 +113,13 @@ export default class UserManager {
 			this.onlineUsers.delete(user);
 		}
 	}
+
+	clearStatsOfInstance(instanceId: number) {
+		for (const user of this.users.values()) {
+			this.notifyLeave(user, instanceId);
+			user.instanceStats.delete(instanceId);
+			user.recalculatePlayerStats();
+		}
+		this.dirty = true;
+	}
 }
diff --git a/test/controller/UserManager.js b/test/controller/UserManager.js
index 5294d551..a25067c3 100644
--- a/test/controller/UserManager.js
+++ b/test/controller/UserManager.js
@@ -1,5 +1,6 @@
 "use strict";
 const assert = require("assert").strict;
+const { PlayerStats } = require("@clusterio/lib");
 const { ControllerUser, UserManager } = require("@clusterio/controller");
 
 describe("controller/src/UserManager", function() {
@@ -31,5 +32,28 @@ describe("controller/src/UserManager", function() {
 			assert(!userManager.onlineUsers.has(user));
 			assert.deepEqual(user.instances, new Set());
 		});
+		describe(".clearStatsOfInstance()", function() {
+			it("should remove the instance stats from existing users", function() {
+				let user1 = ControllerUser.fromJSON({ name: "admin", roles: [1] });
+				let user2 = ControllerUser.fromJSON({ name: "player", roles: [1] });
+				userManager.users.set(user1.name, user1);
+				userManager.users.set(user2.name, user2);
+				user1.instanceStats.set(10, new PlayerStats({ join_count: 1 }));
+				user1.instanceStats.set(11, new PlayerStats({ join_count: 6 }));
+				user1.recalculatePlayerStats();
+				userManager.notifyJoin(user1, 11);
+				user2.instanceStats.set(10, new PlayerStats({ join_count: 3 }));
+				user2.recalculatePlayerStats();
+				assert.equal(user1.playerStats.joinCount, 7);
+				assert.equal(user2.playerStats.joinCount, 3);
+				assert(userManager.onlineUsers.has(user1));
+
+				userManager.clearStatsOfInstance(11);
+
+				assert.equal(user1.playerStats.joinCount, 1);
+				assert(!userManager.onlineUsers.has(user1));
+				assert.equal(user2.playerStats.joinCount, 3);
+			});
+		});
 	});
 });

From e8d9eb6916dda81357414286198c760d3ba68093 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Thu, 8 Feb 2024 21:09:56 +0100
Subject: [PATCH 4/5] Convert prototype icons to array during export

It's possible to set the .icons to an arbitrary key value mapping
instead of an array and the game will still accept it.  Convert the
icons to an array if it's an object mapping to handle this case.

Note that this doesn't preserve the order the game reads the keys in
because JavaScript reorders numeric key indexes on objects differently
to how the game orders them.
---
 CHANGELOG.md                |  1 +
 packages/host/src/export.ts | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 80b4f549..ea1ac02f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,7 @@
 - Fixed Research Sync applying progress of a previous techology level to the level on the controller when it's the active research during instance startup.
 - Fixed data export with a mod using shorthand notation for the default value of a color-setting causing the ModPack data to be set to invalid data.
 - Fixed per user per instance player stats not being deleted when the instance it's for is deleted.
+- Fixed mods setting the item prototype icons property to an object mapping instead of an array breaking the data export.
 
 
 ## Version 2.0.0-alpha.16
diff --git a/packages/host/src/export.ts b/packages/host/src/export.ts
index 337a89f5..530f8fc9 100644
--- a/packages/host/src/export.ts
+++ b/packages/host/src/export.ts
@@ -245,6 +245,18 @@ async function loadLayeredIcon(
 	return icon;
 }
 
+function fixIcons(item: ItemPrototype) {
+	const icons = item.icons;
+	if (typeof icons === "object" && !(icons instanceof Array) && icons !== null) {
+		// It's possible to specify icons as an object with arbitrary keys
+		// and the game will still accept it, cast the value to array if
+		// this is the case.
+		item = { ...item };
+		item.icons = Object.values(icons);
+	}
+	return item;
+}
+
 const itemTypes = new Set([
 	"item",
 	"ammo",
@@ -277,7 +289,8 @@ const itemTypes = new Set([
 function filterItems(prototypes: Prototypes): ItemPrototype[] {
 	return Object.entries(prototypes)
 		.filter(([type, _]) => itemTypes.has(type))
-		.flatMap(([_, typePrototypes]) => Object.values(typePrototypes)) as ItemPrototype[]
+		.flatMap(([_, typePrototypes]) => Object.values(typePrototypes) as ItemPrototype[])
+		.map(fixIcons)
 	;
 }
 

From c9609259db644cb04b4c3cf29ca796361e8b0ef4 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Sun, 11 Feb 2024 15:48:45 +0100
Subject: [PATCH 5/5] Ignore error loading mod in fetchMod

If a bad download or file transfer causes a mod in a host's mod folder
to become corrupt the attempt to load the mod will throw an error that's
not the file not existing.  This in turn causes starting the instance to
always fail as the mod is never attempted to be re-downloaded.

Fix by ignoring errors attempting to load mods already present in the
mods folder that aren't in the mod store so that a new download is
attempted after that fails.
---
 CHANGELOG.md              | 1 +
 packages/host/src/Host.ts | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ea1ac02f..485f1130 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@
 - Fixed data export with a mod using shorthand notation for the default value of a color-setting causing the ModPack data to be set to invalid data.
 - Fixed per user per instance player stats not being deleted when the instance it's for is deleted.
 - Fixed mods setting the item prototype icons property to an object mapping instead of an array breaking the data export.
+- Fixed a bad mod in the host's mods folder causing instance start to fail instead of the host re-downloading the mod.
 
 
 ## Version 2.0.0-alpha.16
diff --git a/packages/host/src/Host.ts b/packages/host/src/Host.ts
index c2fc5d67..75e80ffa 100644
--- a/packages/host/src/Host.ts
+++ b/packages/host/src/Host.ts
@@ -600,8 +600,9 @@ export default class Host extends lib.Link {
 		try {
 			modInfo = await this.modStore.loadMod(mod.name, mod.version);
 		} catch (err: any) {
+			// A broken zip file may have been left in the mods folder
 			if (err.code !== "ENOENT") {
-				throw err;
+				logger.error(`Error loading mod ${mod.name} ${mod.version}: ${err.message}`);
 			}
 		}
 		if (modInfo && (!mod.sha1 || mod.sha1 === modInfo.sha1)) {