From 80bd75ba16b17a12b93bac111446aa5893ecd0c9 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Sun, 29 Dec 2019 04:17:47 +0100
Subject: [PATCH 01/10] Rework module loading

Change module loading to use a module.json file describing the module
and patch in all files in a directory path passed to the patcher instead
of having to pass the files names and their content.  This allows the
specification of which files should be passed to the event_handler lib
and an easy implementation of loading stand alone modules.
---
 client.js                                     | 66 ++++++++++++-------
 docs/developing-for-clusterio.md              | 40 ++++++++---
 docs/writing-plugins.md                       |  9 ++-
 lib/factorio/patch.js                         | 29 +++++---
 .../{lua => module}/export.lua                |  0
 .../statistics_exporter/module/module.json    |  4 ++
 plugins/statistics_exporter/slave.js          |  0
 test/file/modules/subdir/dir/test.lua         |  1 +
 test/file/modules/test/test.lua               |  1 +
 test/integration/patch.js                     | 15 +++--
 test/lib/factorio/patch.js                    |  2 +-
 11 files changed, 120 insertions(+), 47 deletions(-)
 rename plugins/statistics_exporter/{lua => module}/export.lua (100%)
 create mode 100644 plugins/statistics_exporter/module/module.json
 delete mode 100644 plugins/statistics_exporter/slave.js
 create mode 100644 test/file/modules/subdir/dir/test.lua
 create mode 100644 test/file/modules/test/test.lua

diff --git a/client.js b/client.js
index 444d6c0d..d0e6f96d 100644
--- a/client.js
+++ b/client.js
@@ -155,37 +155,56 @@ class Instance extends link.Link{
 		// Patch save with lua modules from plugins
 		console.log("Clusterio | Patching save");
 
-		// For now it's assumed that all files in the lua folder of a plugin is
-		// to be patched in under the name of the plugin and loaded for all
-		// plugins that are not disabled.  This will most likely change in the
-		// future when the plugin refactor is done.
-		let modules = [];
-		for (let pluginName of await fs.readdir("plugins")) {
-			let pluginDir = path.join("plugins", pluginName);
-			if (await fs.pathExists(path.join(pluginDir, "DISABLED"))) {
+		// Find plugin modules to patch in
+		let modules = new Map();
+		for (let pluginName of this.plugins.keys()) {
+			let modulePath = path.join('plugins', pluginName, 'module');
+			if (!await fs.pathExists(modulePath)) {
 				continue;
 			}
 
-			if (!await fs.pathExists(path.join(pluginDir, "lua"))) {
-				continue;
+			let moduleJsonPath = path.join(modulePath, 'module.json');
+			if (!await fs.pathExists(moduleJsonPath)) {
+				throw new Error(`Module for plugin ${pluginName} is missing module.json`);
 			}
 
-			let module = {
-				"name": pluginName,
-				"files": [],
-			};
-
-			for (let fileName of await fs.readdir(path.join(pluginDir, "lua"))) {
-				module["files"].push({
-					path: pluginName+"/"+fileName,
-					content: await fs.readFile(path.join(pluginDir, "lua", fileName)),
-					load: true,
-				});
+			let module = JSON.parse(await fs.readFile(moduleJsonPath));
+			if (module.name !== pluginName) {
+				throw new Error(`Expected name of module for plugin ${pluginName} to match the plugin name`);
 			}
 
-			modules.push(module);
+			module = Object.assign({
+				path: modulePath,
+				load: [],
+			}, module);
+			modules.set(module.name, module);
 		}
-		await factorio.patch(this.path("saves", saveName), modules);
+
+		for (let entry of await fs.readdir('modules', { withFileTypes: true })) {
+			if (entry.isDirectory()) {
+				if (modules.has(entry.name)) {
+					throw new Error(`Module with name ${entry.name} already exists in a plugin`);
+				}
+
+				let moduleJsonPath = path.join('modules', entry.name, 'module.json');
+				if (!await fs.pathExists(moduleJsonPath)) {
+					throw new Error(`Module ${entry.name} is missing module.json`);
+				}
+
+				let module = JSON.parse(await fs.readFile(moduleJsonPath));
+				if (module.name !== entry.name) {
+					throw new Error(`Expected name of module ${entry.name} to match the directory name`);
+				}
+
+				module = Object.assign({
+					path: path.join('modules', entry.name),
+					load: [],
+				}, module);
+				modules.set(module.name, module);
+			}
+		}
+
+		await factorio.patch(this.path("saves", saveName), modules.values());
 
 		this.server.on('rcon-ready', () => {
 			console.log("Clusterio | RCON connection established");
@@ -805,6 +824,7 @@ async function startClient() {
 
 	await fs.ensureDir(slaveConfig.instanceDirectory);
 	await fs.ensureDir("sharedMods");
+	await fs.ensureDir("modules");
 
 	// Set the process title, shows up as the title of the CMD window on windows
 	// and as the process name in ps/top on linux.
diff --git a/docs/developing-for-clusterio.md b/docs/developing-for-clusterio.md
index 5cf4958f..6083285e 100644
--- a/docs/developing-for-clusterio.md
+++ b/docs/developing-for-clusterio.md
@@ -180,13 +180,40 @@ for some pointers if you get stuck.
 Clusterio Modules
 -----------------
 
-Modules are primarily used by plugins to inject code into Factorio
-games with the save patcher.  The save patcher puts modules into the
-`modules` folder of the Factorio save and adds code to `control.lua` to
+Modules are primarily used by plugins to inject code into Factorio games
+with the save patcher, though it's also possible to make stand alone
+modules that are loaded into the game if you don't need the capabilites
+of the plugin system.  The save patcher puts modules into the `modules`
+folder of the Factorio save and adds code to `control.lua` to
 load the modules by requiring it and passing the result of the require
 call to the `add_lib` function of the event_handler lib.  See the
 section on the [event_handler interface](#event_handler-interface) for a
-detailed description on how this works.
+detailed description on how the event_handler lib works.
+
+Stand alone modules are placed into the modules folder of Clusterio,
+plugin modules are located in the module folder of the plugin.  In
+either case a `module.json` file is required and has the following
+structure:
+
+```json
+{
+    "name": "my_module",
+    "load": ["foo.lua"]
+}
+```
+
+The following entries are supported in the module.json file:
+
+- `name`:
+    Name of the module, must match the folder the module is located in
+    for stand alone modules or the name of the plugin the module is
+    located in.
+- `load`:
+    Lua files to load with the `event_handler` lib.  They should be
+    specified as paths that's relative to the module and uses forward
+    slashes as directory sepparators.  Thus foo.lua will require a file
+    by that name and feed the result of that require to the event
+    handler lib.
 
 Because modules can be patched into an existing game you cannot rely on
 the `on_init` callback to be called in Clusterio Modules.  Nor can you
@@ -194,11 +221,6 @@ rely on the `on_configuration_changed` callback, as this is not called
 when level code changes.  Instead you will have to initialize whatever
 global variable you need when you first use them.
 
-Currently any files in a folder named `lua` in a Clusterio plugin is
-assumed to be a module and will be patched into the save before starting
-up Factorio.  This will most likely change with the planned plugin
-restructuring.
-
 
 Clusterio Plugins
 -----------------
diff --git a/docs/writing-plugins.md b/docs/writing-plugins.md
index 86b450d6..9926e845 100644
--- a/docs/writing-plugins.md
+++ b/docs/writing-plugins.md
@@ -18,7 +18,8 @@ The basic file structure of a plugin is the following.
       +- instance.js
       +- test/
       |  +- plugin.js
-      +- lua/
+      +- module/
+         +- module.json
          +- plugin.lua
 
 The most important file is the `info.js` file.  Without it the plugin
@@ -70,6 +71,12 @@ The following properties are recognized:
     Object with link messages definitions for this plugin.  See guide
     for [defining link messages](#defining-link-messages) below.
 
+The optional module folder contains a Clusterio module that will be
+patched into the save when the plugin is enabled.  See the section on
+[Clusterio Modules](developing-for-clusterio.md) in the Developing for
+Clusterio document.  The only restriction imposed on modules embedded
+into plugins is that they must be named the same as the plugin.
+
 While there is no standard for how to organize a plugin it's recommended
 to put the MasterPlugin class definition into master.js and the
 InstancePlugin class definition into instance.js.  You can put them into
diff --git a/lib/factorio/patch.js b/lib/factorio/patch.js
index 235d53b5..41ebf1c0 100644
--- a/lib/factorio/patch.js
+++ b/lib/factorio/patch.js
@@ -7,6 +7,7 @@
 const events = require("events");
 const fs = require("fs-extra");
 const jszip = require("jszip");
+const path = require("path");
 
 const hash = require("lib/hash");
 
@@ -81,7 +82,7 @@ function generateLoader(patchInfo) {
 	for (let module of patchInfo["modules"]) {
 		for (let file of module["files"]) {
 			if (file["load"]) {
-				let requirePath = "modules/" + file["path"].slice(0, -4);
+				let requirePath = `modules/${module.name}/${file["path"].slice(0, -4)}`;
 				lines.push('event_handler.add_lib(require("'+requirePath+'"))');
 			}
 		}
@@ -135,19 +136,31 @@ async function patch(savePath, modules) {
 	}
 
 	// Add the modules to the save.
-	let modulesDir = root.folder("modules");
 	for (let module of modules) {
+		let moduleDir = root.folder(`modules/${module.name}`);
 		let moduleInfo = {
 			"name": module.name,
 			"files": [],
 		}
 
-		for (let file of module.files) {
-			modulesDir.file(file.path, file.content);
-			moduleInfo["files"].push({
-				"path": file.path,
-				"load": Boolean(file.load),
-			});
+		let dirs = [[module.path, '']];
+		while (dirs.length) {
+			let [dir, relativeDir] = dirs.pop();
+			for (let entry of await fs.readdir(dir, { withFileTypes: true })) {
+				let fsPath = path.join(dir, entry.name);
+				let relativePath = path.posix.join(relativeDir, entry.name);
+
+				if (entry.isFile()) {
+					moduleDir.file(relativePath, await fs.readFile(fsPath));
+					moduleInfo["files"].push({
+						"path": relativePath,
+						"load": module.load.includes(relativePath),
+					});
+
+				} else if (entry.isDirectory()) {
+					dirs.push([fsPath, relativePath]);
+				}
+			}
 		}
 
 		patchInfo["modules"].push(moduleInfo);
diff --git a/plugins/statistics_exporter/lua/export.lua b/plugins/statistics_exporter/module/export.lua
similarity index 100%
rename from plugins/statistics_exporter/lua/export.lua
rename to plugins/statistics_exporter/module/export.lua
diff --git a/plugins/statistics_exporter/module/module.json b/plugins/statistics_exporter/module/module.json
new file mode 100644
index 00000000..253cd328
--- /dev/null
+++ b/plugins/statistics_exporter/module/module.json
@@ -0,0 +1,4 @@
+{
+    "name": "statistics_exporter",
+    "load": ["export.lua"]
+}
diff --git a/plugins/statistics_exporter/slave.js b/plugins/statistics_exporter/slave.js
deleted file mode 100644
index e69de29b..00000000
diff --git a/test/file/modules/subdir/dir/test.lua b/test/file/modules/subdir/dir/test.lua
new file mode 100644
index 00000000..181e93bd
--- /dev/null
+++ b/test/file/modules/subdir/dir/test.lua
@@ -0,0 +1 @@
+-- test
diff --git a/test/file/modules/test/test.lua b/test/file/modules/test/test.lua
new file mode 100644
index 00000000..181e93bd
--- /dev/null
+++ b/test/file/modules/test/test.lua
@@ -0,0 +1 @@
+-- test
diff --git a/test/integration/patch.js b/test/integration/patch.js
index 25e8b1f8..9897bc28 100644
--- a/test/integration/patch.js
+++ b/test/integration/patch.js
@@ -23,18 +23,23 @@ describe("Integration of lib/factorio/patch", function() {
 			slowTest(this);
 			await factorio.patch(savePath, [{
 				name: "test",
-				files: [{ path: "test.lua", content: "-- test" }]
+				path: "test/file/modules/test",
+				load: [],
+			}, {
+				name: "subdir",
+				path: "test/file/modules/subdir",
+				load: [],
 			}]);
 
 			let zip = await jszip.loadAsync(await fs.readFile(savePath));
-			let content = await zip.file("test/modules/test.lua").async("string");
-			assert.equal(content, "-- test");
+			assert.equal(await zip.file("test/modules/test/test.lua").async("string"), "-- test\n");
+			assert.equal(await zip.file("test/modules/subdir/dir/test.lua").async("string"), "-- test\n");
 		});
 		it("should remove old modules in a save", async function() {
 			slowTest(this);
-			await factorio.patch(savePath, [{ name: "test", files: [] }]);
+			await factorio.patch(savePath, []);
 			let zip = await jszip.loadAsync(await fs.readFile(savePath));
-			assert.equal(zip.file("test/modules/test.lua"), null);
+			assert.equal(zip.file("test/modules/test/test.lua"), null);
 		});
 	});
 });
diff --git a/test/lib/factorio/patch.js b/test/lib/factorio/patch.js
index c9f3900f..3815a852 100644
--- a/test/lib/factorio/patch.js
+++ b/test/lib/factorio/patch.js
@@ -59,7 +59,7 @@ describe("lib/factorio", function() {
 			'event_handler.add_lib(require("foo"))',
 			'',
 			'-- Clusterio modules',
-			'event_handler.add_lib(require("modules/bar"))',
+			'event_handler.add_lib(require("modules/spam/bar"))',
 			'',
 		].join('\n');
 		let patchInfo = {

From e89e54ee27955d5613e1bdce0c2393b5fefd27b1 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Mon, 30 Dec 2019 05:09:02 +0100
Subject: [PATCH 02/10] Implement dependencies specification for modules

Implement dependency ordering and version checking for modules.
---
 client.js                        |   9 ++-
 docs/developing-for-clusterio.md |  17 +++++
 docs/writing-plugins.md          |   4 +-
 lib/factorio/patch.js            |  97 ++++++++++++++++++++++++
 package.json                     |   1 +
 test/integration/patch.js        |   4 +
 test/lib/factorio/patch.js       | 126 +++++++++++++++++++++++++++++++
 7 files changed, 255 insertions(+), 3 deletions(-)

diff --git a/client.js b/client.js
index d0e6f96d..09c12488 100644
--- a/client.js
+++ b/client.js
@@ -157,7 +157,7 @@ class Instance extends link.Link{
 
 		// Find plugin modules to patch in
 		let modules = new Map();
-		for (let pluginName of this.plugins.keys()) {
+		for (let [pluginName, plugin] of this.plugins) {
 			let modulePath = path.join('plugins', pluginName, 'module');
 			if (!await fs.pathExists(modulePath)) {
 				continue;
@@ -174,12 +174,16 @@ class Instance extends link.Link{
 			}
 
 			module = Object.assign({
+				version: plugin.info.version,
+				dependencies: {},
 				path: modulePath,
 				load: [],
 			}, module);
 			modules.set(module.name, module);
 		}
 
+		// Find stand alone modules to load
+		// XXX for now it's assumed all available modules should be loaded.
 		for (let entry of await fs.readdir('modules', { withFileTypes: true })) {
 			if (entry.isDirectory()) {
 				if (modules.has(entry.name)) {
@@ -198,13 +202,14 @@ class Instance extends link.Link{
 
 				module = Object.assign({
 					path: path.join('modules', entry.name),
+					dependencies: {},
 					load: [],
 				}, module);
 				modules.set(module.name, module);
 			}
 		}
 
-		await factorio.patch(this.path("saves", saveName), modules.values());
+		await factorio.patch(this.path("saves", saveName), [...modules.values()]);
 
 		this.server.on('rcon-ready', () => {
 			console.log("Clusterio | RCON connection established");
diff --git a/docs/developing-for-clusterio.md b/docs/developing-for-clusterio.md
index 6083285e..41e9ac5f 100644
--- a/docs/developing-for-clusterio.md
+++ b/docs/developing-for-clusterio.md
@@ -198,6 +198,10 @@ structure:
 ```json
 {
     "name": "my_module",
+    "version": "1.2.0",
+    "dependencies": {
+        "foo": ">=0.4.2"
+    },
     "load": ["foo.lua"]
 }
 ```
@@ -208,6 +212,19 @@ The following entries are supported in the module.json file:
     Name of the module, must match the folder the module is located in
     for stand alone modules or the name of the plugin the module is
     located in.
+- `version`:
+    Version of the module.  Must be compatible with [Semantic Versioning
+    2.0.0](https://semver.org/).  In plugin modules this defaults to the
+    version of the plugin otherwise it's required.
+- `dependencies`:
+    Optional mapping of modules this module depends on and their
+    version.  In the example `>=0.4.2` means that the foo module must be
+    at least version 0.4.2.  See the [Ranges
+    syntax](https://www.npmjs.com/package/semver#ranges) of the
+    node-semver package for a full description of what operators are
+    supported.  The events for dependencies is invoked before the events
+    of the dependent, and starting an instance will fail if the
+    dependencies cannot be satisfied.
 - `load`:
     Lua files to load with the `event_handler` lib.  They should be
     specified as paths that's relative to the module and uses forward
diff --git a/docs/writing-plugins.md b/docs/writing-plugins.md
index 9926e845..9a0f65af 100644
--- a/docs/writing-plugins.md
+++ b/docs/writing-plugins.md
@@ -52,7 +52,9 @@ The following properties are recognized:
     Brief description of what the plugin does.  Currently not used.
 
 **version**:
-    Semver compatible version string of the plugin.  Currently not used.
+    Semver compatible version string of the plugin.  Currently only
+    servers as the default for the Clusterio module embedded in the
+    plugin, if any.
 
 **instanceEntrypoint**:
     Path to a Node.js module relative to the plugin directory which
diff --git a/lib/factorio/patch.js b/lib/factorio/patch.js
index 41ebf1c0..93c51bc0 100644
--- a/lib/factorio/patch.js
+++ b/lib/factorio/patch.js
@@ -8,6 +8,7 @@ const events = require("events");
 const fs = require("fs-extra");
 const jszip = require("jszip");
 const path = require("path");
+const semver = require("semver");
 
 const hash = require("lib/hash");
 
@@ -94,6 +95,99 @@ function generateLoader(patchInfo) {
 	return lines.join('\n');
 }
 
+/**
+ * Reorders modules to satisfy their dependencies
+ *
+ * Looks through and reorders the array of module definitions in order to
+ * satisfy the property that dependencies are earlier in the array than
+ * their dependents.  Throws an error if this is not possible.
+ *
+ * @param {Array<Object>} modules - Array of modules to reorder
+ */
+function reorderDependencies(modules) {
+	let index = 0;
+	let present = new Map();
+	let hold = new Map();
+	reorder: while (index < modules.length) {
+		let module = modules[index];
+		if (semver.valid(module.version) === null) {
+			throw new Error(`Invalid version '${module.version}' for module ${module.name}`);
+		}
+
+		for (let [dependency, requirement] of Object.entries(module.dependencies)) {
+			if (semver.validRange(requirement) === null) {
+				throw new Error(
+					`Invalid version range '${requirement}' for dependency ${dependency} on module ${module.name}`
+				);
+			}
+
+			if (present.has(dependency)) {
+				if (!semver.satisfies(present.get(dependency), requirement)) {
+					throw new Error(`Module ${module.name} requires ${dependency} ${requirement}`);
+				}
+
+			// We have an unmet dependency, take it out and continue
+			} else {
+				if (hold.has(dependency)) {
+					hold.get(dependency).push(module);
+				} else {
+					hold.set(dependency, [module]);
+				}
+				modules.splice(index, 1);
+				continue reorder;
+			}
+		}
+
+		// No unmet dependencies, record and continue
+		present.set(module.name, module.version);
+		index++;
+
+		if (hold.has(module.name)) {
+			modules.splice(index, 0, ...hold.get(module.name));
+			hold.delete(module.name);
+		}
+	}
+
+	if (!hold.size) {
+		return;
+	}
+
+	// There are three reasons for a module to end up being held: The module depends
+	// on a module that is missing, the module is part of a dependency loop, or the
+	// the module depends on a module that satisfy any of these conditions.
+
+	let remaining = new Map();
+	for (let modules of hold.values()) {
+		for (let module of modules) {
+			remaining.set(module.name, module);
+		}
+	}
+
+	// Start with a random module from the remaining modules
+	for (let module of remaining.values()) {
+		let cycle = [];
+		while (true) {
+			// Find an unmet dependency
+			let dependency = Object.keys(module.dependencies).find(name => !present.has(name));
+
+			if (!remaining.has(dependency)) {
+				// There's no module being held up by this dependency, the
+				// dependency is missing.
+				throw new Error(`Missing dependency ${dependency} for module ${module.name}`);
+			}
+
+			if (cycle.includes(module.name)) {
+				cycle.push(module.name);
+				cycle.splice(0, cycle.indexOf(module.name))
+				throw new Error(`Module dependency loop detected: ${cycle.join(" -> ")}`);
+			}
+
+			cycle.push(module.name);
+			module = remaining.get(dependency);
+		}
+	}
+}
+
 /**
  * Patch a save with the given modules
  *
@@ -135,6 +229,8 @@ async function patch(savePath, modules) {
 		zip.remove(file.name);
 	}
 
+	reorderDependencies(modules);
+
 	// Add the modules to the save.
 	for (let module of modules) {
 		let moduleDir = root.folder(`modules/${module.name}`);
@@ -182,4 +278,5 @@ module.exports = {
 	// For testing only
 	_findRoot: findRoot,
 	_generateLoader: generateLoader,
+	_reorderDependencies: reorderDependencies,
 }
diff --git a/package.json b/package.json
index 2ea38232..e596efdf 100644
--- a/package.json
+++ b/package.json
@@ -56,6 +56,7 @@
     "rcon-client": "^4.2.0",
     "request": "^2.83.0",
     "sanitizer": "^0.1.3",
+    "semver": "^7.1.1",
     "set-blocking": "^2.0.0",
     "simple-git": "^1.96.0",
     "single-line-log": "^1.1.2",
diff --git a/test/integration/patch.js b/test/integration/patch.js
index 9897bc28..8fc96c23 100644
--- a/test/integration/patch.js
+++ b/test/integration/patch.js
@@ -23,10 +23,14 @@ describe("Integration of lib/factorio/patch", function() {
 			slowTest(this);
 			await factorio.patch(savePath, [{
 				name: "test",
+				version: "1.0.0",
+				dependencies: {},
 				path: "test/file/modules/test",
 				load: [],
 			}, {
 				name: "subdir",
+				version: "1.0.0",
+				dependencies: {},
 				path: "test/file/modules/subdir",
 				load: [],
 			}]);
diff --git a/test/lib/factorio/patch.js b/test/lib/factorio/patch.js
index 3815a852..75f07553 100644
--- a/test/lib/factorio/patch.js
+++ b/test/lib/factorio/patch.js
@@ -78,6 +78,132 @@ describe("lib/factorio", function() {
 		});
 	});
 
+	describe("reorderDependencies()", function() {
+		it("should reorder to satisfy simple dependency", function() {
+			let modules = [
+				{ name: 'a', version: '1.0.0', dependencies: {'b': '*'} },
+				{ name: 'b', version: '1.0.0', dependencies: {} },
+			];
+			factorio._reorderDependencies(modules);
+			assert(modules[0].name === 'b', "Dependency was not reorederd");
+		});
+		it("should throw on invalid version", function() {
+			let modules = [
+				{ name: 'a', version: 'foo', dependencies: {} },
+			];
+			assert.throws(
+				() => factorio._reorderDependencies(modules),
+				new Error("Invalid version 'foo' for module a")
+			);
+		});
+		it("should throw on invalid version range", function() {
+			let modules = [
+				{ name: 'a', version: '1.0.0', dependencies: {'b': 'invalid'} },
+			];
+			assert.throws(
+				() => factorio._reorderDependencies(modules),
+				new Error("Invalid version range 'invalid' for dependency b on module a")
+			);
+		});
+		it("should throw on missing dependency", function() {
+			let modules = [
+				{ name: 'a', version: '1.0.0', dependencies: {'b': '*'} },
+			];
+			assert.throws(
+				() => factorio._reorderDependencies(modules),
+				new Error("Missing dependency b for module a")
+			);
+			modules = [
+				{ name: 'a', version: '1.0.0', dependencies: {'b': '*'} },
+				{ name: 'b', version: '1.0.0', dependencies: {'c': '*'} },
+				{ name: 'c', version: '1.0.0', dependencies: {'d': '*'} },
+			];
+			assert.throws(
+				() => factorio._reorderDependencies(modules),
+				new Error("Missing dependency d for module c")
+			);
+		});
+		it("should throw on outdated dependency", function() {
+			let modules = [
+				{ name: 'a', version: '1.0.0', dependencies: {} },
+				{ name: 'b', version: '1.0.0', dependencies: {'a': '>=2'} },
+			];
+			assert.throws(
+				() => factorio._reorderDependencies(modules),
+				new Error("Module b requires a >=2")
+			);
+		});
+		it("should throw on dependency loops", function() {
+			let modules = [
+				{ name: 'a', version: '1.0.0', dependencies: {'b': '*'} },
+				{ name: 'b', version: '1.0.0', dependencies: {'a': '*'} },
+			];
+			assert.throws(
+				() => factorio._reorderDependencies(modules),
+				new Error("Module dependency loop detected: a -> b -> a")
+			);
+			modules = [
+				{ name: 'd', version: '1.0.0', dependencies: {'b': '*'} },
+				{ name: 'a', version: '1.0.0', dependencies: {'b': '*'} },
+				{ name: 'b', version: '1.0.0', dependencies: {'c': '*'} },
+				{ name: 'c', version: '1.0.0', dependencies: {'a': '*'} },
+			];
+			assert.throws(
+				() => factorio._reorderDependencies(modules),
+				new Error("Module dependency loop detected: b -> c -> a -> b")
+			);
+		});
+		it("should reorder correctly for multiple dependencies", function() {
+			function validate(modules) {
+				let present = new Set();
+				for (let module of modules) {
+					for (let dependency of Object.keys(module.dependencies)) {
+						if (!present.has(dependency)) {
+							assert.fail(`${module.name} depends on ${dependency}, but was ordered before it`);
+						}
+					}
+					present.add(module.name);
+				}
+			}
+
+			let names = ['a', 'b', 'c'];
+			let successCount = 0;
+			const bitCount = names.length * (names.length - 1);
+			for (let i=0; i < 2**bitCount; i++) {
+				let value = i;
+				let bits = [];
+				for (let j=0; j < bitCount; j++) {
+					bits.push(value & 1);
+					value >>= 1;
+				}
+				let modules = [];
+				for (let j=0; j < names.length; j++) {
+					let name = names[j];
+					let dependencies = {};
+					for (let k=0; k < names.length - 1; k++) {
+						if (bits[k + j * (names.length - 1)]) {
+							dependencies[names[k + (k >= j)]] = '*';
+						}
+					}
+					modules.push({ name, version: '1.0.0', dependencies });
+				}
+				let success = false;
+				try {
+					factorio._reorderDependencies(modules);
+					success = true;
+				} catch (err) { }
+				if (success) {
+					validate(modules);
+					successCount++;
+				}
+			}
+
+			// Deriving the formula for the number of graphs without
+			// dependencies loops is left as an excercise for the reader.
+			assert(successCount === 25, "Incorrect number of successful orderings");
+		});
+	});
+
 	describe("patch()", function() {
 		it("should throw on unknown scenario", async function() {
 			let zip = new jszip();

From c091dc341efb4e55ce36ddc5f06c9ddd8a7a6d0a Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Mon, 30 Dec 2019 19:29:30 +0100
Subject: [PATCH 03/10] Implement require option for modules

Implement option allowing files to be required from control.lua be
specified in modules.json.
---
 client.js                        |  2 ++
 docs/developing-for-clusterio.md | 42 ++++++++++++++++++++++++--------
 lib/factorio/patch.js            |  6 ++++-
 test/integration/patch.js        |  2 ++
 test/lib/factorio/patch.js       |  6 +++--
 5 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/client.js b/client.js
index 09c12488..26841776 100644
--- a/client.js
+++ b/client.js
@@ -178,6 +178,7 @@ class Instance extends link.Link{
 				dependencies: {},
 				path: modulePath,
 				load: [],
+				require: [],
 			}, module);
 			modules.set(module.name, module);
 		}
@@ -204,6 +205,7 @@ class Instance extends link.Link{
 					path: path.join('modules', entry.name),
 					dependencies: {},
 					load: [],
+					require: [],
 				}, module);
 				modules.set(module.name, module);
 			}
diff --git a/docs/developing-for-clusterio.md b/docs/developing-for-clusterio.md
index 41e9ac5f..31aaafc5 100644
--- a/docs/developing-for-clusterio.md
+++ b/docs/developing-for-clusterio.md
@@ -184,11 +184,9 @@ Modules are primarily used by plugins to inject code into Factorio games
 with the save patcher, though it's also possible to make stand alone
 modules that are loaded into the game if you don't need the capabilites
 of the plugin system.  The save patcher puts modules into the `modules`
-folder of the Factorio save and adds code to `control.lua` to
-load the modules by requiring it and passing the result of the require
-call to the `add_lib` function of the event_handler lib.  See the
-section on the [event_handler interface](#event_handler-interface) for a
-detailed description on how the event_handler lib works.
+folder of the Factorio save and adds code to `control.lua` to load the
+module according to the `load` and `require` options to the module.json
+file.
 
 Stand alone modules are placed into the modules folder of Clusterio,
 plugin modules are located in the module folder of the plugin.  In
@@ -202,6 +200,7 @@ structure:
     "dependencies": {
         "foo": ">=0.4.2"
     },
+    "require": ["bar.lua"],
     "load": ["foo.lua"]
 }
 ```
@@ -225,12 +224,35 @@ The following entries are supported in the module.json file:
     supported.  The events for dependencies is invoked before the events
     of the dependent, and starting an instance will fail if the
     dependencies cannot be satisfied.
+- `require`:
+    Lua files to require in `control.lua`.  This should only really be
+    needed if you want to make a global function available for use in
+    Lua commands.  The paths are relative to the module's own folder and
+    should be specified using forward slashes as directory sepparators
+    if it's located in a sub directory in the module.
 - `load`:
-    Lua files to load with the `event_handler` lib.  They should be
-    specified as paths that's relative to the module and uses forward
-    slashes as directory sepparators.  Thus foo.lua will require a file
-    by that name and feed the result of that require to the event
-    handler lib.
+    Lua files to load with the `event_handler` lib, the result of
+    requiring the file will be passed to the `add_lib` function of the
+    `event_handler` lib in `control.lua`.  See the section on the
+    [event_handler interface](#event_handler-interface) for a detailed
+    description on how the `event_handler` lib works.  The paths are
+    relative to the module's own folder and should be specified using
+    forward slashes as directory sepparators if it's located in a sub
+    directory in the module.
+
+It's recommended to use the new style of defining modules in Lua, as
+well as avoid the use of global variables as much as possible.  This
+means always declearing your top level variables and functions local and
+exporting only the things you need in other files.  You can require
+other files in your module by prefixing your require paths with
+`modules/your_module_name/`.  It's also possible to require files from
+other modules this way too.
+
+The global variables and data stored in the global table is shared by
+all Clusterio modules as well as the scenario, so it's important that
+you use unique names.  It's recommended to prefix the global variables
+and data in the global table that your module has with the name of your
+module.
 
 Because modules can be patched into an existing game you cannot rely on
 the `on_init` callback to be called in Clusterio Modules.  Nor can you
diff --git a/lib/factorio/patch.js b/lib/factorio/patch.js
index 93c51bc0..0940a932 100644
--- a/lib/factorio/patch.js
+++ b/lib/factorio/patch.js
@@ -82,10 +82,13 @@ function generateLoader(patchInfo) {
 
 	for (let module of patchInfo["modules"]) {
 		for (let file of module["files"]) {
+			let requirePath = `modules/${module.name}/${file["path"].slice(0, -4)}`;
 			if (file["load"]) {
-				let requirePath = `modules/${module.name}/${file["path"].slice(0, -4)}`;
 				lines.push('event_handler.add_lib(require("'+requirePath+'"))');
 			}
+			if (file["require"]) {
+				lines.push('require("'+requirePath+'")');
+			}
 		}
 	}
 
@@ -251,6 +254,7 @@ async function patch(savePath, modules) {
 					moduleInfo["files"].push({
 						"path": relativePath,
 						"load": module.load.includes(relativePath),
+						"require": module.require.includes(relativePath),
 					});
 
 				} else if (entry.isDirectory()) {
diff --git a/test/integration/patch.js b/test/integration/patch.js
index 8fc96c23..2d342ffd 100644
--- a/test/integration/patch.js
+++ b/test/integration/patch.js
@@ -27,12 +27,14 @@ describe("Integration of lib/factorio/patch", function() {
 				dependencies: {},
 				path: "test/file/modules/test",
 				load: [],
+				require: [],
 			}, {
 				name: "subdir",
 				version: "1.0.0",
 				dependencies: {},
 				path: "test/file/modules/subdir",
 				load: [],
+				require: [],
 			}]);
 
 			let zip = await jszip.loadAsync(await fs.readFile(savePath));
diff --git a/test/lib/factorio/patch.js b/test/lib/factorio/patch.js
index 75f07553..bcbf2dbe 100644
--- a/test/lib/factorio/patch.js
+++ b/test/lib/factorio/patch.js
@@ -60,6 +60,7 @@ describe("lib/factorio", function() {
 			'',
 			'-- Clusterio modules',
 			'event_handler.add_lib(require("modules/spam/bar"))',
+			'require("modules/spam/spam")',
 			'',
 		].join('\n');
 		let patchInfo = {
@@ -67,8 +68,9 @@ describe("lib/factorio", function() {
 			"modules": [{
 				"name": "spam",
 				"files": [
-					{ "path": "bar.lua", "load": true },
-					{ "path": "excluded.lua", "load": false },
+					{ "path": "bar.lua", "load": true, "require": false },
+					{ "path": "spam.lua", "load": false, "require": true },
+					{ "path": "excluded.lua", "load": false, "require": false },
 				],
 			}],
 		};

From f88e1f1d3a7eb3e8fd8f333c1ab0de819aef4f33 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Tue, 31 Dec 2019 03:41:44 +0100
Subject: [PATCH 04/10] Remove dead code left over from 21d9fdf

Remove compatConfig definition after RCON is established which was left
over from 21d9fdf.
---
 client.js | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/client.js b/client.js
index 26841776..91476886 100644
--- a/client.js
+++ b/client.js
@@ -215,18 +215,6 @@ class Instance extends link.Link{
 
 		this.server.on('rcon-ready', () => {
 			console.log("Clusterio | RCON connection established");
-			// Temporary measure for backwards compatibility
-			let compatConfig = {
-				id: this.config.id,
-				unique: this.config.id,
-				name: this.config.name,
-
-				// FactorioServer.init may have generated a random port or password
-				// if they were null.
-				factorioPort: this.server.gamePort,
-				clientPort: this.server.rconPort,
-				clientPassword: this.server.rconPassword,
-			}
 		});
 
 		await this.server.start(saveName);

From 221dc855f496b6e2f3f9678a028629522c3207ce Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Tue, 31 Dec 2019 04:28:14 +0100
Subject: [PATCH 05/10] Add option to throw on non-empty RCON responses

Add option to FactorioServer.sendRcon to throw on non-empty responses
from Factorio.  This is useful for detecting errors returned by commands
that should not have a response.
---
 lib/factorio/server.js          | 11 +++++++++--
 plugins/global_chat/instance.js |  2 +-
 test/integration/server.js      |  7 +++++++
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/factorio/server.js b/lib/factorio/server.js
index 831f1173..39effbab 100644
--- a/lib/factorio/server.js
+++ b/lib/factorio/server.js
@@ -625,15 +625,22 @@ class FactorioServer extends events.EventEmitter {
 	 * wait until it is establied and then send the message.
 	 *
 	 * @param {string} message - message to send to server over RCON.
+	 * @param {boolean} expectEmpty -
+	 *     if true throw if the response is not empty.  Useful for detecting
+	 *     errors that might have been sent in response.
 	 * @returns {string} response from server.
 	 */
-	async sendRcon(message) {
+	async sendRcon(message, expectEmpty) {
 		this._check(["running"])
 		if (!this._rconReady) {
 			await events.once(this, 'rcon-ready');
 		}
 
-		return await this._rconClient.send(message);
+		let response = await this._rconClient.send(message);
+		if (expectEmpty && response !== '') {
+			throw new Error(`Expected empty response but got "${response}"`);
+		}
+		return response;
 	}
 
 	/**
diff --git a/plugins/global_chat/instance.js b/plugins/global_chat/instance.js
index 8d85d1c1..40661eac 100644
--- a/plugins/global_chat/instance.js
+++ b/plugins/global_chat/instance.js
@@ -34,7 +34,7 @@ class InstancePlugin extends plugin.BaseInstancePlugin {
 	async chatEventHandler(message) {
 		// TODO check if cross server chat is enabled
 		let content = `[${message.data.instance_name}] ${removeTags(message.data.content)}`;
-		await this.instance.server.sendRcon(`/sc game.print('${escapeLuaString(content)}')`);
+		await this.instance.server.sendRcon(`/sc game.print('${escapeLuaString(content)}')`, true);
 	}
 
 	async onOutput(output) {
diff --git a/test/integration/server.js b/test/integration/server.js
index 327ed2d0..488b7a02 100644
--- a/test/integration/server.js
+++ b/test/integration/server.js
@@ -129,6 +129,13 @@ describe("Integration of lib/factorio/server", function() {
 				let result = await server.sendRcon("/sc rcon.print('success')");
 				assert.equal(result, 'success\n');
 			});
+			it("throws on non-empty response when enabled", async function() {
+				slowTest(this);
+				await assert.rejects(
+					server.sendRcon("/sc rcon.print('fail')", true),
+					new Error('Expected empty response but got "fail\n"')
+				);
+			});
 		});
 
 		describe(".stop()", function() {

From 66aa7e1e4330a3ea421fa9f5f29f1f50d03a7e25 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Tue, 31 Dec 2019 19:20:46 +0100
Subject: [PATCH 06/10] Add clusterio module with API for lua code

Add a shared API for lua code through a module named clusterio.
---
 client.js                          | 15 +++++++--
 docs/developing-for-clusterio.md   |  3 +-
 lib/luaTools.js                    | 17 ++++++++++
 modules/clusterio/api.lua          | 17 ++++++++++
 modules/clusterio/impl.lua         | 52 ++++++++++++++++++++++++++++++
 modules/clusterio/module.json      |  6 ++++
 plugins/global_chat/instance.js    | 18 ++---------
 plugins/global_chat/test/plugin.js | 24 --------------
 test/lib/luaTools.js               | 30 +++++++++++++++++
 9 files changed, 139 insertions(+), 43 deletions(-)
 create mode 100644 lib/luaTools.js
 create mode 100644 modules/clusterio/api.lua
 create mode 100644 modules/clusterio/impl.lua
 create mode 100644 modules/clusterio/module.json
 create mode 100644 test/lib/luaTools.js

diff --git a/client.js b/client.js
index 91476886..5a2f64a7 100644
--- a/client.js
+++ b/client.js
@@ -11,6 +11,7 @@ const link = require("lib/link");
 const plugin = require("lib/plugin");
 const errors = require("lib/errors");
 const prometheus = require('lib/prometheus');
+const luaTools = require('lib/luaTools');
 
 
 /**
@@ -175,7 +176,7 @@ class Instance extends link.Link{
 
 			module = Object.assign({
 				version: plugin.info.version,
-				dependencies: {},
+				dependencies: { 'clusterio': '*' },
 				path: modulePath,
 				load: [],
 				require: [],
@@ -203,7 +204,7 @@ class Instance extends link.Link{
 
 				module = Object.assign({
 					path: path.join('modules', entry.name),
-					dependencies: {},
+					dependencies: { 'clusterio': '*' },
 					load: [],
 					require: [],
 				}, module);
@@ -218,12 +219,22 @@ class Instance extends link.Link{
 		});
 
 		await this.server.start(saveName);
+		await this.server.disableAchievements()
+		await this.updateInstanceData();
 
 		for (let pluginInstance of this.plugins.values()) {
 			await pluginInstance.onStart();
 		}
 	}
 
+	/**
+	 * Update instance information on the Factorio side
+	 */
+	async updateInstanceData() {
+		let name = luaTools.escapeString(this.config.name);
+		await this.server.sendRcon(`/sc clusterio_private.update_instance(${this.config.id}, "${name}")`, true);
+	}
+
 	/**
 	 * Stop the instance
 	 */
diff --git a/docs/developing-for-clusterio.md b/docs/developing-for-clusterio.md
index 31aaafc5..6fee72e7 100644
--- a/docs/developing-for-clusterio.md
+++ b/docs/developing-for-clusterio.md
@@ -223,7 +223,8 @@ The following entries are supported in the module.json file:
     node-semver package for a full description of what operators are
     supported.  The events for dependencies is invoked before the events
     of the dependent, and starting an instance will fail if the
-    dependencies cannot be satisfied.
+    dependencies cannot be satisfied.  If not specified it will default
+    to depending on `clusterio`.
 - `require`:
     Lua files to require in `control.lua`.  This should only really be
     needed if you want to make a global function available for use in
diff --git a/lib/luaTools.js b/lib/luaTools.js
new file mode 100644
index 00000000..8165beb5
--- /dev/null
+++ b/lib/luaTools.js
@@ -0,0 +1,17 @@
+/**
+ * Escapes a string for inclusion into a lua string
+ */
+function escapeString(content) {
+	return content
+		.replace(/\\/g, "\\\\")
+		.replace(/"/g, '\\"')
+		.replace(/'/g, "\\'")
+		.replace(/\0/g, "\\0")
+		.replace(/\n/g, "\\n")
+		.replace(/\r/g, "\\r")
+	;
+}
+
+module.exports = {
+	escapeString,
+}
diff --git a/modules/clusterio/api.lua b/modules/clusterio/api.lua
new file mode 100644
index 00000000..47339457
--- /dev/null
+++ b/modules/clusterio/api.lua
@@ -0,0 +1,17 @@
+local api = {}
+
+
+-- Table of events raised by clusterio
+api.events = {
+    -- Raised after the name and id of an instance has been updated
+    -- This may occur even if the id and name didn't change.
+
+    -- Event data:
+    --   instance_id: The id of the instance.
+    --   instance_name: the name of the instance.
+    on_instance_updated = script.generate_event_name(),
+}
+
+
+return api
+
diff --git a/modules/clusterio/impl.lua b/modules/clusterio/impl.lua
new file mode 100644
index 00000000..2f97ffba
--- /dev/null
+++ b/modules/clusterio/impl.lua
@@ -0,0 +1,52 @@
+local api = require('modules/clusterio/api')
+
+
+local impl = {}
+
+local function init()
+    if not global.clusterio then
+        global.clusterio = {
+            instance_id = nil,
+            instance_name = nil,
+        }
+    end
+end
+
+-- Internal API
+clusterio_private = {}
+function clusterio_private.update_instance(new_id, new_name)
+    init()
+    global.clusterio.instance_id = new_id
+    global.clusterio.instance_name = new_name
+    script.raise_event(api.events.on_instance_updated, {
+        instance_id = new_id,
+        instance_name = new_name,
+    })
+end
+
+
+function impl.add_remote_interface()
+    remote.add_interface('clusterio_api', {
+        -- Returns a table of events raised by clusterio
+        get_events = function()
+            return api.events
+        end,
+
+        -- Returns the instance id the game is run under
+        -- This may change over time and/or be nil.
+        get_instance_id = function()
+            init()
+            return global.clusterio.instance_id
+        end,
+
+        -- Returns the name of the instance the game runs under.
+        -- This may change over time and/or be nil.
+        get_instance_name = function()
+            init()
+            return global.clusterio.instance_name
+        end,
+    })
+end
+
+
+return impl
diff --git a/modules/clusterio/module.json b/modules/clusterio/module.json
new file mode 100644
index 00000000..5ba34a15
--- /dev/null
+++ b/modules/clusterio/module.json
@@ -0,0 +1,6 @@
+{
+    "name": "clusterio",
+    "version": "0.0.0",
+    "dependencies": {},
+    "load": ["impl.lua"]
+}
diff --git a/plugins/global_chat/instance.js b/plugins/global_chat/instance.js
index 40661eac..e44ed210 100644
--- a/plugins/global_chat/instance.js
+++ b/plugins/global_chat/instance.js
@@ -1,4 +1,5 @@
 const plugin = require("lib/plugin");
+const luaTools = require("lib/luaTools");
 
 
 /**
@@ -8,20 +9,6 @@ function removeTags(content) {
 	return content.replace(/(\[gps=-?\d+,-?\d+\]|\[train=\d+\])/g, "");
 }
 
-/**
- * Escapes a string for inclusion into a lua string
- */
-function escapeLuaString(content) {
-	return content
-		.replace(/\\/g, "\\\\")
-		.replace(/"/g, '\\"')
-		.replace(/'/g, "\\'")
-		.replace(/\0/g, "\\0")
-		.replace(/\n/g, "\\n")
-		.replace(/\r/g, "\\r")
-	;
-}
-
 class InstancePlugin extends plugin.BaseInstancePlugin {
 	constructor(...args) {
 		super(...args);
@@ -34,7 +21,7 @@ class InstancePlugin extends plugin.BaseInstancePlugin {
 	async chatEventHandler(message) {
 		// TODO check if cross server chat is enabled
 		let content = `[${message.data.instance_name}] ${removeTags(message.data.content)}`;
-		await this.instance.server.sendRcon(`/sc game.print('${escapeLuaString(content)}')`, true);
+		await this.instance.server.sendRcon(`/sc game.print('${luaTools.escapeString(content)}')`, true);
 	}
 
 	async onOutput(output) {
@@ -54,5 +41,4 @@ module.exports = {
 
 	// For testing only
 	_removeTags: removeTags,
-	_escapeLuaString: escapeLuaString,
 }
diff --git a/plugins/global_chat/test/plugin.js b/plugins/global_chat/test/plugin.js
index 0e1d3de0..28590068 100644
--- a/plugins/global_chat/test/plugin.js
+++ b/plugins/global_chat/test/plugin.js
@@ -19,30 +19,6 @@ describe("global_chat plugin", function() {
 		});
 	});
 
-	describe("escapeLuaSrting", function() {
-		it("should escape backslashes", function() {
-			assert.equal(instance._escapeLuaString("\\a"), "\\\\a");
-		});
-		it("should escape double quotes", function() {
-			assert.equal(instance._escapeLuaString('"a"'), '\\"a\\"');
-		});
-		it("should escape single quotes", function() {
-			assert.equal(instance._escapeLuaString("'a'"), "\\'a\\'");
-		});
-		it("should escape nul bytes", function() {
-			assert.equal(instance._escapeLuaString("a\0b"), "a\\0b");
-		});
-		it("should escape newlines", function() {
-			assert.equal(instance._escapeLuaString("a\nb"), "a\\nb");
-		});
-		it("should escape carriage return", function() {
-			assert.equal(instance._escapeLuaString("a\rb"), "a\\rb");
-		});
-		it("should escape all combined", function() {
-			assert.equal(instance._escapeLuaString("a\\b\"c'd\0e\nf\rg"), "a\\\\b\\\"c\\'d\\0e\\nf\\rg");
-		});
-	});
-
 	describe("class InstancePlugin", function() {
 		let instancePlugin;
 
diff --git a/test/lib/luaTools.js b/test/lib/luaTools.js
new file mode 100644
index 00000000..c1dd8d36
--- /dev/null
+++ b/test/lib/luaTools.js
@@ -0,0 +1,30 @@
+const assert = require('assert').strict;
+
+const luaTools = require('lib/luaTools');
+
+
+describe("lib/luaTools", function() {
+	describe("escapeString()", function() {
+		it("should escape backslashes", function() {
+			assert.equal(luaTools.escapeString("\\a"), "\\\\a");
+		});
+		it("should escape double quotes", function() {
+			assert.equal(luaTools.escapeString('"a"'), '\\"a\\"');
+		});
+		it("should escape single quotes", function() {
+			assert.equal(luaTools.escapeString("'a'"), "\\'a\\'");
+		});
+		it("should escape nul bytes", function() {
+			assert.equal(luaTools.escapeString("a\0b"), "a\\0b");
+		});
+		it("should escape newlines", function() {
+			assert.equal(luaTools.escapeString("a\nb"), "a\\nb");
+		});
+		it("should escape carriage return", function() {
+			assert.equal(luaTools.escapeString("a\rb"), "a\\rb");
+		});
+		it("should escape all combined", function() {
+			assert.equal(luaTools.escapeString("a\\b\"c'd\0e\nf\rg"), "a\\\\b\\\"c\\'d\\0e\\nf\\rg");
+		});
+	});
+});

From 0e9cd35c30c856ccbc2a6b854b71aabf1b523168 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Thu, 2 Jan 2020 00:11:52 +0100
Subject: [PATCH 07/10] Add table of contents to Writing Plugins

Add table of contents for the Writing plugins document.
---
 docs/writing-plugins.md | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/docs/writing-plugins.md b/docs/writing-plugins.md
index 9a0f65af..ba3b821c 100644
--- a/docs/writing-plugins.md
+++ b/docs/writing-plugins.md
@@ -6,6 +6,18 @@ Node.js.  The plugin classes have pre-defined hooks that are called
 during various stages and operations of Clusterio.
 
 
+Contents
+--------
+
+- [Plugin Structure](#plugin-structure)
+- [Defining the plugin class](#defining-the-plugin-class)
+- [Defining Link Messages](#defining-link-messages)
+    - [Defining Events](#defining-events)
+    - [Defining Requests](#defining-requests)
+- [Sending Link Messages](#sending-link-messages)
+- [Collecting Statistics](#collecting-statistics)
+
+
 Plugin Structure
 ----------------
 

From 6cf16cc09efa394357d19e0be4dbce803807e90e Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Thu, 2 Jan 2020 03:41:15 +0100
Subject: [PATCH 08/10] Reference by global in statistics_exporter

Store export function in a global table in the lua module for statistics
exporter in order to avoid a remote.call.
---
 plugins/statistics_exporter/instance.js        | 2 +-
 plugins/statistics_exporter/module/export.lua  | 9 ++-------
 plugins/statistics_exporter/module/module.json | 2 +-
 plugins/statistics_exporter/test/plugin.js     | 4 ++--
 4 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/plugins/statistics_exporter/instance.js b/plugins/statistics_exporter/instance.js
index d9b74927..aa158231 100644
--- a/plugins/statistics_exporter/instance.js
+++ b/plugins/statistics_exporter/instance.js
@@ -22,7 +22,7 @@ const instanceGameFlowStatistics = new Gauge(
 
 class InstancePlugin extends plugin.BaseInstancePlugin {
 	async onMetrics() {
-		let string = await this.instance.server.sendRcon('/sc remote.call("statistics_exporter", "export")');
+		let string = await this.instance.server.sendRcon('/sc statistics_exporter.export()');
 		let stats;
 		try {
 			stats = JSON.parse(string);
diff --git a/plugins/statistics_exporter/module/export.lua b/plugins/statistics_exporter/module/export.lua
index babb44da..fb7fc164 100644
--- a/plugins/statistics_exporter/module/export.lua
+++ b/plugins/statistics_exporter/module/export.lua
@@ -5,7 +5,8 @@ local statistics = {
     "entity_build_count_statistics",
 }
 
-local function export()
+statistics_exporter = {}
+function statistics_exporter.export()
     local stats = {
         game_tick = game.tick,
         player_count = #game.connected_players,
@@ -31,9 +32,3 @@ local function export()
 
     rcon.print(game.table_to_json(stats))
 end
-
-return {
-    add_remote_interface = function()
-        remote.add_interface("statistics_exporter", { export = export })
-    end
-}
diff --git a/plugins/statistics_exporter/module/module.json b/plugins/statistics_exporter/module/module.json
index 253cd328..7da2637e 100644
--- a/plugins/statistics_exporter/module/module.json
+++ b/plugins/statistics_exporter/module/module.json
@@ -1,4 +1,4 @@
 {
     "name": "statistics_exporter",
-    "load": ["export.lua"]
+    "require": ["export.lua"]
 }
diff --git a/plugins/statistics_exporter/test/plugin.js b/plugins/statistics_exporter/test/plugin.js
index 2716e7b8..2112a441 100644
--- a/plugins/statistics_exporter/test/plugin.js
+++ b/plugins/statistics_exporter/test/plugin.js
@@ -16,7 +16,7 @@ describe("statistics_exporter plugin", function() {
 		describe(".onMetrics()", function() {
 			it("should record statistics", async function() {
 				instancePlugin.instance.server.rconCommandResults.set(
-					'/sc remote.call("statistics_exporter", "export")',
+					'/sc statistics_exporter.export()',
 					JSON.stringify({
 						game_tick: 100,
 						player_count: 3,
@@ -51,7 +51,7 @@ describe("statistics_exporter plugin", function() {
 			it("should pass on JSON parse errors", async function() {
 				let string = 'An error occured\n';
 				instancePlugin.instance.server.rconCommandResults.set(
-					'/sc remote.call("statistics_exporter", "export")', string
+					'/sc statistics_exporter.export()', string
 				);
 
 				let errorMessage;

From b8fb73f86390219059c4c7bab131b4d38b359ff5 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Thu, 2 Jan 2020 04:14:41 +0100
Subject: [PATCH 09/10] Add Factorio to Clusterio communication API

Add send_json to the Clusterio module's API which allows sending
arbitrary tables serialized as JSON to plugins that listen on the
channel specified.
---
 docs/developing-for-clusterio.md | 10 -----
 docs/writing-plugins.md          | 69 ++++++++++++++++++++++++++++++++
 lib/factorio/server.js           | 49 +++++++++++++++++++++++
 modules/clusterio/api.lua        | 36 +++++++++++++++++
 test/lib/factorio/server.js      | 49 +++++++++++++++++++++++
 5 files changed, 203 insertions(+), 10 deletions(-)

diff --git a/docs/developing-for-clusterio.md b/docs/developing-for-clusterio.md
index 6fee72e7..2f14f529 100644
--- a/docs/developing-for-clusterio.md
+++ b/docs/developing-for-clusterio.md
@@ -12,7 +12,6 @@ Contents
 - [Factorio Mods](#factorio-mods)
 - [Factorio Scenarios](#factorio-scenarios)
     - [event_handler interface](#event_handler-interface)
-- [Communicating between Clusterio and Factorio](#communicating-between-clusterio-and-factorio)
 - [Communicating with Clusterio](#communicating-with-clusterio)
 - [Clusterio Modules](#clusterio-modules)
 - [Clusterio Plugins](#clusterio-plugins)
@@ -158,15 +157,6 @@ There's also the `add_libraries` function exported by event_handler,
 which accepts a table and calls `add_lib` for each value in the table.
 
 
-Communicating between Clusterio and Factorio
---------------------------------------------
-
-There is currently no standard interface for moving data between
-Clusterio and Factorio.  It's mostly a mix of RCON commands initiated
-from Clusterio plugins and writing out files with `game.write_file` that
-is picked up and parsed by plugins.
-
-
 Communicating with Clusterio
 ----------------------------
 
diff --git a/docs/writing-plugins.md b/docs/writing-plugins.md
index ba3b821c..ac534027 100644
--- a/docs/writing-plugins.md
+++ b/docs/writing-plugins.md
@@ -11,6 +11,7 @@ Contents
 
 - [Plugin Structure](#plugin-structure)
 - [Defining the plugin class](#defining-the-plugin-class)
+- [Communicating with Factorio](#communicating-with-factorio)
 - [Defining Link Messages](#defining-link-messages)
     - [Defining Events](#defining-events)
     - [Defining Requests](#defining-requests)
@@ -141,6 +142,74 @@ result in unpredicatable behaviour.  The async init method always called
 immediatly after the constructor so there's little reason to do this.
 
 
+Communicating with Factorio
+---------------------------
+
+For pushing data into Factorio there's RCON which lets you send
+arbitrary Lua command to invoke whatever code you want in the game.
+This is done by calling sendRcon method on the instance's server object.
+For example:
+
+    async onStart() {
+        let response = await this.instance.server.sendRcon(
+            "/sc rcon.print('data')"
+        );
+
+        // Do stuff with response.
+    }
+
+
+Because data into Factorio is streamed at a rate of 3-6 kB/s by default
+it is recommended to avoid sending large commands as much as possible,
+and to strip down the data on the ones you send to only what's strictly
+necessary.  You can have lua code injected into the game via the module
+system and call that from RCON to avoid having to send code through the
+commands.
+
+For getting data out from Factorio there's both RCON and the `send_json`
+API of the Clusterio module.  Returning data via RCON is prefered if the
+action is initiated from the Node.js side.  The `send_json` API allows
+sending JSON payloads on channels that plugins can listen to.  From a
+plugin you listen for an event named ipc-channel_name in order to get
+data sent by `send_json`.  For example in the plugin code:
+
+    async init() {
+        this.server.on('ipc-my_plugin_foo', content =>
+            this.handleFoo(content).catch(err => console.log(
+                "Error handling foo:", foo
+            ))
+        );
+    }
+
+    async handleFoo(content) {
+        // Do stuff with content
+    }
+
+And then in the module for the plugin:
+
+    local clusterio_api = require('modules/clusterio/api')
+
+    -- inside some event handler
+    clusterio_api.send('my_plugin_foo', { data = 123 })
+
+It's recommended to either use the plugin name as the channel name or to
+prefix the channel name with the name of the plugin if you need multiple
+channels.  It's also important to catch any errors that might occur as
+they will otherwise be propogated to the instance code and kill the
+server.
+
+Data out from Factorio does not have the same limits as data into
+Factorio, RCON responses can be in 100kB range without causing issues,
+and payloads to the `send_json` API can be in the 4MB range provided the
+server has a fast enough storage system.
+
+**Note:** both `send_json` and RCON can operate out of order.  For
+`send_json` it's possible that payloads greater than 4kB are received
+after payloads that was sent at a later point in time.  For RCON,
+commands longer than 50 characters may end up being executed after
+shorter commands sent after it.
+
+
 Defining Link Messages
 ----------------------
 
diff --git a/lib/factorio/server.js b/lib/factorio/server.js
index 39effbab..ee62b935 100644
--- a/lib/factorio/server.js
+++ b/lib/factorio/server.js
@@ -377,7 +377,56 @@ class FactorioServer extends events.EventEmitter {
 		}
 	}
 
+	async _handleIpc(line) {
+		let channelEnd = line.indexOf('?');
+		if (channelEnd === -1) {
+			throw new Error(`Malformed IPC line "${line.toString()}"`);
+		}
+
+		let channel = line
+			.slice(6, channelEnd)
+			.toString('utf-8')
+			.replace(/\\x([0-9a-f]{2})/g, (match, p1) =>
+				String.fromCharCode(parseInt(p1, 16))
+			)
+		;
+
+		let type = line.slice(channelEnd + 1, channelEnd + 2).toString('utf-8');
+		let content;
+		if (type === 'j') {
+			content = JSON.parse(line.slice(channelEnd + 2).toString('utf-8'));
+
+		} else if (type === 'f') {
+			let fileName = line.slice(channelEnd + 2).toString('utf-8');
+			let filePath = this.writePath('script-output', fileName);
+
+			// Prevent malicious names
+			if (/[\\/\0]/.test(fileName)) {
+				throw new Error(`Invalid IPC file name '${fileName}'`)
+			}
+
+			if (fileName.endsWith('.json')) {
+				content = JSON.parse(await fs.readFile(filePath, 'utf-8'));
+				await fs.unlink(filePath);
+
+			} else {
+				let format = fileName.slice(fileName.lastIndexOf('.') + 1);
+				throw new Error(`Unknown IPC file format '${format}'`)
+			}
+
+		} else {
+			throw new Error(`Unknown IPC type '${type}'`);
+		}
+
+		this.emit(`ipc-${channel}`, content);
+	}
+
 	_handleOutput(line, source) {
+		if (line.slice(0, 6).equals(Buffer.from("\f$ipc:"))) {
+			this._handleIpc(line).catch(err => this.emit('error', err));
+			return;
+		}
+
 		this.emit(source, line);
 
 		let output = parseOutput(line.toString('utf-8'), source);
diff --git a/modules/clusterio/api.lua b/modules/clusterio/api.lua
index 47339457..9d94e19a 100644
--- a/modules/clusterio/api.lua
+++ b/modules/clusterio/api.lua
@@ -12,6 +12,42 @@ api.events = {
     on_instance_updated = script.generate_event_name(),
 }
 
+-- Send a table as json to Clusterio
+-- Send data to Clusterio over the given channel.  Clusterio plugins can
+-- listen to channels and will receive an event with the data sent here.
+-- See docs/writing-plugins.md for more information.
+
+-- Note: Payloads greater than 4 MB will cause stuttering.
+
+-- Note: This is not a binary safe way of sending data.  Strings embedded
+-- into the tables sent must be valid UTF-8 text.
+
+-- Parameters:
+--   channel: string identifying which channel to send it on.
+--   data: table that can be converted to json with game.table_to_json
+function api.send_json(channel, data)
+
+    -- Escape bad characters.  The question mark is used for separating the
+    -- channel name from the payload.
+    channel = channel:gsub("([\x00-\x1f?\\])", function(match)
+        return "\\x" .. string.format("%02x", match:byte())
+    end)
+
+    data = game.table_to_json(data)
+
+    -- If there's more than about 4kB of data users running with the Windows
+    -- console open will start to experience stuttering, otherwise we could
+    -- output about 1 MB of data at a time through stdout.
+    if #data < 4000 then
+        print("\f$ipc:" .. channel .. "?j" .. data)
+    else
+        global.clusterio_file_no = (global.clusterio_file_no or 0) + 1
+        local file_name = "clst_" .. global.clusterio_file_no .. ".json"
+        game.write_file(file_name, data, false, 0)
+        print("\f$ipc:" .. channel .. "?f" .. file_name)
+    end
+end
+
 
 return api
 
diff --git a/test/lib/factorio/server.js b/test/lib/factorio/server.js
index 1923508f..f891e835 100644
--- a/test/lib/factorio/server.js
+++ b/test/lib/factorio/server.js
@@ -1,4 +1,5 @@
 const assert = require("assert").strict;
+const events = require("events");
 const fs = require("fs-extra");
 const path = require("path");
 
@@ -122,5 +123,53 @@ describe("lib/factorio/server", function() {
 				assert.equal(server.version, "0.1.1");
 			});
 		});
+
+		describe("._handleIpc()", function() {
+			it("should emit the correct ipc event", async function() {
+				let waiter = events.once(server, 'ipc-channel');
+				await server._handleIpc(Buffer.from('\f$ipc:channel?j"value"'));
+				let result = await waiter;
+				assert.equal(result[0], "value");
+			});
+			it("should handle special characters in channel name", async function() {
+				let waiter = events.once(server, 'ipc-$ ?\x00\x0a:');
+				await server._handleIpc(Buffer.from('\f$ipc:$ \\x3f\\x00\\x0a:?j"value"'));
+				let result = await waiter;
+				assert.equal(result[0], "value");
+			});
+			it("should throw on malformed ipc line", async function() {
+				await assert.rejects(
+					server._handleIpc(Buffer.from('\f$ipc:blah')),
+					new Error('Malformed IPC line "\f$ipc:blah"')
+				);
+			})
+			it("should throw on unknown type", async function() {
+				await assert.rejects(
+					server._handleIpc(Buffer.from('\f$ipc:channel??')),
+					new Error("Unknown IPC type '?'")
+				);
+			})
+			it("should throw on unknown file type", async function() {
+				await assert.rejects(
+					server._handleIpc(Buffer.from('\f$ipc:channel?ffoo.invalid')),
+					new Error("Unknown IPC file format 'invalid'")
+				);
+			})
+			it("should throw on file name with slash", async function() {
+				await assert.rejects(
+					server._handleIpc(Buffer.from('\f$ipc:channel?fa/b')),
+					new Error("Invalid IPC file name 'a/b'")
+				);
+			})
+			it("should load and delete json file", async function() {
+				let filePath = server.writePath('script-output', 'data.json');
+				await fs.outputFile(filePath, '{"data":"spam"}');
+				let waiter = events.once(server, 'ipc-channel');
+				await server._handleIpc(Buffer.from('\f$ipc:channel?fdata.json'));
+				let result = await waiter;
+				assert.deepEqual(result[0], { "data": "spam" });
+				assert(!await fs.pathExists(filePath), "File was not deleted");
+			})
+		});
 	});
 });

From 892c8034960e76fbe2977b2e6c743caed05090d2 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Sun, 5 Jan 2020 23:19:34 +0100
Subject: [PATCH 10/10] Add on_server_startup event

Add startup event to the Clusterio module API which is raised at startup
of a Factorio server.  This is to server as a replacement for the
on_init and on_configuration_changed events which cannot be used in
modules.

Adds clusterio_patch_number global from the patcher in order to
implement this.  This is a number that's incremented every time the save
is patched.
---
 docs/developing-for-clusterio.md | 19 +++++++++++++++++--
 docs/writing-plugins.md          |  2 +-
 lib/factorio/patch.js            |  4 ++++
 modules/clusterio/api.lua        | 13 ++++++++++++-
 modules/clusterio/impl.lua       | 15 ++++++++++-----
 test/lib/factorio/patch.js       |  2 ++
 6 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/docs/developing-for-clusterio.md b/docs/developing-for-clusterio.md
index 2f14f529..33a83730 100644
--- a/docs/developing-for-clusterio.md
+++ b/docs/developing-for-clusterio.md
@@ -14,6 +14,7 @@ Contents
     - [event_handler interface](#event_handler-interface)
 - [Communicating with Clusterio](#communicating-with-clusterio)
 - [Clusterio Modules](#clusterio-modules)
+    - [Clusterio Module API](#clusterio-module-api)
 - [Clusterio Plugins](#clusterio-plugins)
 
 
@@ -188,6 +189,7 @@ structure:
     "name": "my_module",
     "version": "1.2.0",
     "dependencies": {
+        "clusterio": "*",
         "foo": ">=0.4.2"
     },
     "require": ["bar.lua"],
@@ -248,8 +250,21 @@ module.
 Because modules can be patched into an existing game you cannot rely on
 the `on_init` callback to be called in Clusterio Modules.  Nor can you
 rely on the `on_configuration_changed` callback, as this is not called
-when level code changes.  Instead you will have to initialize whatever
-global variable you need when you first use them.
+when level code changes.  The `clusterio` module provides the custom
+`on_server_startup` event that can be used as a substitute, see the next
+section.
+
+
+### Clusterio Module API
+
+There's a builtin clusterio module that provides some tools for getting
+instance info and communicating to plugins, script events to listen for
+and a Factorio remote call API.  The available interfaces is documented
+in [modules/clusterio/api.lua](modules/clusterio/api.lua).  To use this
+API from another module, require it using something along the lines of
+`local clusterio_api = require("modules/clusterio/api")`.  Make sure to
+add `clusterio` as a dependency in your module.json if you use the
+dependency entry in it.
 
 
 Clusterio Plugins
diff --git a/docs/writing-plugins.md b/docs/writing-plugins.md
index ac534027..8f71771a 100644
--- a/docs/writing-plugins.md
+++ b/docs/writing-plugins.md
@@ -190,7 +190,7 @@ And then in the module for the plugin:
     local clusterio_api = require('modules/clusterio/api')
 
     -- inside some event handler
-    clusterio_api.send('my_plugin_foo', { data = 123 })
+    clusterio_api.send_json('my_plugin_foo', { data = 123 })
 
 It's recommended to either use the plugin name as the channel name or to
 prefix the channel name with the name of the plugin if you need multiple
diff --git a/lib/factorio/patch.js b/lib/factorio/patch.js
index 0940a932..2d5c1957 100644
--- a/lib/factorio/patch.js
+++ b/lib/factorio/patch.js
@@ -65,6 +65,7 @@ function generateLoader(patchInfo) {
 	let lines = [
 		'-- Auto generated scenario module loader created by Clusterio',
 		'-- Modifications to this file will be lost when loaded in Clusterio',
+		`clusterio_patch_number = ${patchInfo.patch_number}`,
 		'',
 		'local event_handler = require("event_handler")',
 		'',
@@ -226,6 +227,9 @@ async function patch(savePath, modules) {
 		}
 	}
 
+	// Increment patch number
+	patchInfo["patch_number"] = (patchInfo["patch_number"] || 0) + 1;
+
 	// Remove any existing modules from the save
 	patchInfo["modules"] = [];
 	for (let file of root.file(/^modules\//)) {
diff --git a/modules/clusterio/api.lua b/modules/clusterio/api.lua
index 9d94e19a..2132f4fa 100644
--- a/modules/clusterio/api.lua
+++ b/modules/clusterio/api.lua
@@ -10,6 +10,18 @@ api.events = {
     --   instance_id: The id of the instance.
     --   instance_name: the name of the instance.
     on_instance_updated = script.generate_event_name(),
+
+    -- Raised when the server is starting up
+    -- This event serves as the stand-in for the on_init and
+    -- on_configuration_changed events for modules.  It is invoked as early
+    -- as possible, before most but not all other events, every time the
+    -- server is started up.
+
+    -- Use this event to initialize and and/or migrate the data structures
+    -- you need.  Keep in mind that Clusterio can switch from any version of
+    -- your module to any version of your module so it should be able to
+    -- handle both forwards and backwards migrations.
+    on_server_startup = script.generate_event_name(),
 }
 
 -- Send a table as json to Clusterio
@@ -50,4 +62,3 @@ end
 
 
 return api
-
diff --git a/modules/clusterio/impl.lua b/modules/clusterio/impl.lua
index 2f97ffba..d4989221 100644
--- a/modules/clusterio/impl.lua
+++ b/modules/clusterio/impl.lua
@@ -1,9 +1,17 @@
 local api = require('modules/clusterio/api')
 
-
 local impl = {}
 
-local function init()
+
+impl.events = {}
+impl.events[defines.events.on_tick] = function()
+    if global.clusterio_patch_number ~= clusterio_patch_number then
+        global.clusterio_patch_number = clusterio_patch_number
+        script.raise_event(api.events.on_server_startup, {})
+    end
+end
+
+impl.events[api.events.on_server_startup] = function()
     if not global.clusterio then
         global.clusterio = {
             instance_id = nil,
@@ -15,7 +23,6 @@ end
 -- Internal API
 clusterio_private = {}
 function clusterio_private.update_instance(new_id, new_name)
-    init()
     global.clusterio.instance_id = new_id
     global.clusterio.instance_name = new_name
     script.raise_event(api.events.on_instance_updated, {
@@ -35,14 +42,12 @@ function impl.add_remote_interface()
         -- Returns the instance id the game is run under
         -- This may change over time and/or be nil.
         get_instance_id = function()
-            init()
             return global.clusterio.instance_id
         end,
 
         -- Returns the name of the instance the game runs under.
         -- This may change over time and/or be nil.
         get_instance_name = function()
-            init()
             return global.clusterio.instance_name
         end,
     })
diff --git a/test/lib/factorio/patch.js b/test/lib/factorio/patch.js
index bcbf2dbe..3a1f890e 100644
--- a/test/lib/factorio/patch.js
+++ b/test/lib/factorio/patch.js
@@ -52,6 +52,7 @@ describe("lib/factorio", function() {
 		let reference = [
 			'-- Auto generated scenario module loader created by Clusterio',
 			'-- Modifications to this file will be lost when loaded in Clusterio',
+			'clusterio_patch_number = 1',
 			'',
 			'local event_handler = require("event_handler")',
 			'',
@@ -64,6 +65,7 @@ describe("lib/factorio", function() {
 			'',
 		].join('\n');
 		let patchInfo = {
+			"patch_number": 1,
 			"scenario": { "modules": ["foo"] },
 			"modules": [{
 				"name": "spam",