Merge pull request #298 from Hornwitser/config-event

Config events

clusterctl.js

@@ -223,6 +223,26 @@ commands.push(new Command({
 	},
 }));
 
+commands.push(new Command({
+	definition: ["set-instance-config-prop", "Set property of field in instance config", (yargs) => {
+		yargs.option({
+			"instance": { describe: "Instance to set config on", nargs: 1, type: "string", demandOption: true },
+			"field": { describe: "Field to set", nargs: 1, type: "string", demandOption: true },
+			"prop": { describe: "Property to set", nargs: 1, type: "string", demandOption: true },
+			"value": { describe: "JSON parsed value to set", nargs: 1, type: "string", demandOption: true },
+		});
+	}],
+	handler: async function(args, control) {
+		let instanceId = await resolveInstance(control, args.instance);
+		await link.messages.setInstanceConfigProp.send(control, {
+			instance_id: instanceId,
+			field: args.field,
+			prop: args.prop,
+			value: JSON.parse(args.value),
+		});
+	},
+}));
+
 commands.push(new Command({
 	definition: ['assign-instance', "Assign instance to a slave", (yargs) => {
 		yargs.options({

lib/config/classes.js

@@ -1,6 +1,9 @@
 // Configuration classes
 "use strict";
 
+const events = require("events");
+const util = require("util");
+
 const { basicType } = require("lib/helpers");
 
 
@@ -55,10 +58,20 @@ function splitOn(separator, string) {
  * Represents a collection of ConfigGroup instances that can be
  * managed as one.
  *
+ * This is an EventEmitter with the following events:
+ * - fieldChanged -
+ *     invoked after a field has been updated in one of the config groups
+ *     belonging to this config.  Takes three parameters: group, field,
+ *     prev.
+ *
+ * @extends events.EventEmitter
+ *
  * @memberof module:lib/config
  */
-class Config {
+class Config extends events.EventEmitter {
 	constructor() {
+		super();
+
 		if (!this.constructor._finalized) {
 			throw new Error(`Cannot instantiate incomplete Config class ${this.constructor.name}`)
 		}
@@ -71,7 +84,7 @@ class Config {
 		for (let [name, GroupClass] of this.constructor._groups) {
 			if (!this._groups.has(name)) {
 				let group = new GroupClass();
-				await group.init();
+				await group.init(this);
 				this._groups.set(name, group);
 			}
 		}
@@ -98,8 +111,8 @@ class Config {
 			}
 
 			let group = new GroupClass();
-			await group.load(serializedGroup);
-			this._groups.set(GroupClass.groupName, group);
+			await group.load(this, serializedGroup);
+			this._groups.set(group.name, group);
 		}
 
 		await this.init();
@@ -123,12 +136,12 @@ class Config {
 	 * Updates all groups in the config with groups in the serialized
 	 * config passed.
 	 */
-	update(serializedConfig) {
+	update(serializedConfig, notify) {
 		this._validate(serializedConfig);
 		for (let serializedGroup of serializedConfig.groups) {
 			let group = this._groups.get(serializedGroup.name);
 			if (group) {
-				group.update(serializedGroup);
+				group.update(serializedGroup, notify);
 			} else {
 				this._unknownGroups.set(serializedGroup.name, serializedGroup);
 			}
@@ -151,6 +164,14 @@ class Config {
 		return this.group(group).set(field, value);
 	}
 
+	/**
+	 * Set property of an object value for a config field
+	 */
+	setProp(name, prop, value) {
+		let [group, field] = splitOn('.', name);
+		return this.group(group).setProp(field, prop, value);
+	}
+
 	/**
 	 * Get the config group instance with the given name
 	 */
@@ -226,6 +247,11 @@ class Config {
 /**
  * Collection of related config entries
  *
+ * Type checks and stores a collection of related config entries.  Attempt
+ * to set invalid values for fields will result in an error thrown while
+ * loading a config with invalid values will result in those being replaced
+ * by the innitial value for those fields.
+ *
  * @memberof module:lib/config
  */
 class ConfigGroup {
@@ -242,6 +268,7 @@ class ConfigGroup {
 
 		this._fields = new Map();
 		this._unknownFields = new Map();
+		this._config = null;
 	}
 
 	/**
@@ -250,7 +277,9 @@ class ConfigGroup {
 	 * Computes and assigns the initial values for all fields of the
 	 * config group.
 	 */
-	async init() {
+	async init(config) {
+		this._config = config;
+
 		for (let [name, def] of this.constructor._definitions) {
 			if (this._fields.has(name)) {
 				continue;
@@ -275,9 +304,18 @@ class ConfigGroup {
 	 * group and then initializes any possible missing fields from it to
 	 * their default values.
 	 */
-	async load(serializedGroup) {
-		this.update(serializedGroup);
-		await this.init();
+	async load(config, serializedGroup) {
+		this.update(serializedGroup, false);
+		await this.init(config);
+	}
+
+	/**
+	 * Name of the group
+	 *
+	 * Shortcut for `group.constructor.groupName`
+	 */
+	get name() {
+		return this.constructor.groupName;
 	}
 
 	/**
@@ -297,12 +335,13 @@ class ConfigGroup {
 	 * The field must be defined for the config group and the value is
 	 * type checked against the field definition.
 	 *
+	 * @throws {module:lib/config.InvalidField} if field is not defined.
 	 * @throws {module:lib/config.InvalidValue} if value is not allowed for the field.
 	 */
 	set(name, value) {
 		let def = this.constructor._definitions.get(name);
 		if (!def) {
-			throw new Error(`No field named '${name}'`);
+			throw new InvalidField(`No field named '${name}'`);
 		}
 
 		// Empty strings are treates as null
@@ -345,7 +384,45 @@ class ConfigGroup {
 			}
 		}
 
+		let prev = this._fields.get(name);
 		this._fields.set(name, value);
+		if (this._config && !util.isDeepStrictEqual(value, prev)) {
+			this._config.emit("fieldChanged", this, name, prev);
+		}
+	}
+
+	/**
+	 * Set property of object field
+	 *
+	 * Update value of stored object field by setting the specified property
+	 * on it.  The field must be defined as an object for the config group.
+	 *
+	 * @throws {module:lib/config.InvalidField} if field is not defined.
+	 * @throws {module:lib/config.InvalidValue} if field is not an object.
+	 */
+	setProp(name, prop, value) {
+		let def = this.constructor._definitions.get(name);
+		if (!def) {
+			throw new InvalidField(`No field named '${name}'`);
+		}
+
+		if (def.type !== "object") {
+			throw new InvalidValue(`Cannot set property on non-object field '${name}'`);
+		}
+
+		let prev = this._fields.get(name);
+		let updated = {};
+		if (prev !== null) {
+			for (let [key, value] of Object.entries(prev)) {
+				updated[key] = value;
+			}
+		}
+
+		updated[prop] = value;
+		this._fields.set(name, updated);
+		if (this._config && !util.isDeepStrictEqual(updated, prev)) {
+			this._config.emit("fieldChanged", this, name, prev);
+		}
 	}
 
 	/**
@@ -354,13 +431,13 @@ class ConfigGroup {
 	 * Overwrites the values of all the fields in this config group that
 	 * has a valid value in the passed serialized group with that value.
 	 */
-	update(serializedGroup) {
+	update(serializedGroup, notify) {
 		if (basicType(serializedGroup) !== "object") {
 			throw new Error(`Expected object, not ${basicType(serializedGroup)} for ConfigGroup`);
 		}
 
-		if (serializedGroup.name !== this.constructor.groupName) {
-			throw new Error(`Expected group name ${this.constructor.groupName}, not ${serializedGroup.name}`);
+		if (serializedGroup.name !== this.name) {
+			throw new Error(`Expected group name ${this.name}, not ${serializedGroup.name}`);
 		}
 
 		if (basicType(serializedGroup.fields) !== "object") {
@@ -388,7 +465,11 @@ class ConfigGroup {
 				continue;
 			}
 
+			let prev = this._fields.get(name);
 			this._fields.set(name, value);
+			if (notify && this._config && !util.isDeepStrictEqual(value, prev)) {
+				this._config.emit("fieldChanged", this, name, prev);
+			}
 		}
 	}
 
@@ -406,7 +487,7 @@ class ConfigGroup {
 		}
 
 		return {
-			name: this.constructor.groupName,
+			name: this.name,
 			fields,
 		};
 	}

lib/link/messages.js

@@ -312,6 +312,17 @@ messages.setInstanceConfigField = new Request({
 	},
 });
 
+messages.setInstanceConfigProp = new Request({
+	type: "set_instance_config_prop",
+	links: ["control-master"],
+	requestProperties: {
+		"instance_id": { type: "integer" },
+		"field": { type: "string" },
+		"prop": { type: "string" },
+		"value": {},
+	},
+});
+
 messages.assignInstanceCommand = new Request({
 	type: 'assign_instance_command',
 	links: ['control-master'],

lib/plugin.js

@@ -42,6 +42,19 @@ class BaseInstancePlugin {
 	 */
 	async init() { }
 
+	/**
+	 * Called when the value of a config field changed.
+	 *
+	 * Invoked after the value of the config field given by `field` has
+	 * changed.
+	 *
+	 * @param {module:lib/config.ConfigGroup} group -
+	 *     The group who's field got changed on.
+	 * @param {string} field - Name of the field that changed.
+	 * @param prev - The previous value of the field.
+	 */
+	async onInstanceConfigFieldChanged(group, field, prev) { }
+
 	/**
 	 * Called before collecting Prometheus metrics
 	 *
@@ -334,6 +347,30 @@ function attachPluginMessages(link, pluginInfo, plugin) {
 	}
 }
 
+/**
+ * Invokes the given hook on all plugins
+ *
+ * @param {Map<string, Object} plugins -
+ *     Mapping of plugin names to plugins to invoke the hook on.
+ * @param {string} hook - Name of hook to invoke.
+ * @param {...*} args - Arguments to pass on to the hook.
+ * @returns {Array} Non-empty return values from the hooks.
+ */
+async function invokeHook(plugins, hook, ...args) {
+	let results = [];
+	for (let [name, plugin] of plugins) {
+		try {
+			let result = await plugin[hook](...args);
+			if (result !== undefined) {
+				results.push(result);
+			}
+		} catch (err) {
+			console.error(`Ignoring error from plugin ${name} in ${hook}:`, err);
+		}
+	}
+	return results;
+}
+
 
 module.exports = {
 	BaseInstancePlugin,
@@ -341,4 +378,5 @@ module.exports = {
 
 	loadPluginInfos,
 	attachPluginMessages,
+	invokeHook,
 };

master.js

@@ -128,12 +128,10 @@ async function getMetrics(req, res, next) {
 	endpointHitCounter.labels(req.route.path).inc();
 
 	let results = []
-	for (let masterPlugin of masterPlugins.values()) {
-		let pluginResults = await masterPlugin.onMetrics();
-		if (pluginResults !== undefined) {
-			for await (let result of pluginResults) {
-				results.push(result)
-			}
+	let pluginResults = await plugin.invokeHook(masterPlugins, "onMetrics");
+	for (let metricIterator of pluginResults) {
+		for await (let metric of metricIterator) {
+			results.push(metric)
 		}
 	}
 
@@ -291,11 +289,7 @@ async function shutdown() {
 		await saveMap(masterConfig.get('master.database_directory'), "slaves.json", db.slaves);
 		await saveInstances(masterConfig.get('master.database_directory'), "instances.json", db.instances);
 
-		for (let [pluginName, masterPlugin] of masterPlugins) {
-			let startTime = Date.now();
-			await masterPlugin.onShutdown();
-			console.log(`Plugin ${pluginName} shutdown in ${Date.now() - startTime}ms`);
-		}
+		await plugin.invokeHook(masterPlugins, "onShutdown");
 
 		stopAcceptingNewSessions = true;
 
@@ -427,9 +421,7 @@ class BaseConnection extends link.Link {
 	}
 
 	async prepareDisconnectRequestHandler(message, request) {
-		for (let masterPlugin of masterPlugins.values()) {
-			await masterPlugin.onPrepareSlaveDisconnect(this);
-		}
+		await plugin.invokeHook(masterPlugins, "onPrepareSlaveDisconnect", this);
 		this.connector.setClosing();
 		return await super.prepareDisconnectRequestHandler(message, request);
 	}
@@ -537,6 +529,19 @@ class ControlConnection extends BaseConnection {
 		}
 	}
 
+	async updateInstanceConfig(instance) {
+		let slaveId = instance.config.get("instance.assigned_slave");
+		if (slaveId) {
+			let connection = slaveConnections.get(slaveId);
+			if (connection) {
+				await link.messages.assignInstance.send(connection, {
+					instance_id: instance.config.get("instance.id"),
+					serialized_config: instance.config.serialize(),
+				});
+			}
+		}
+	}
+
 	async setInstanceConfigFieldRequestHandler(message) {
 		let instance = db.instances.get(message.data.instance_id);
 		if (!instance) {
@@ -553,18 +558,18 @@ class ControlConnection extends BaseConnection {
 		}
 
 		instance.config.set(message.data.field, message.data.value);
+		await this.updateInstanceConfig(instance);
+	}
 
-		// Push updated config to slave
-		let slaveId = instance.config.get('instance.assigned_slave');
-		if (slaveId) {
-			let connection = slaveConnections.get(slaveId);
-			if (connection) {
-				await link.messages.assignInstance.send(connection, {
-					instance_id: instance.config.get("instance.id"),
-					serialized_config: instance.config.serialize(),
-				});
-			}
+	async setInstanceConfigPropRequestHandler(message) {
+		let instance = db.instances.get(message.data.instance_id);
+		if (!instance) {
+			throw new errors.RequestError(`Instance with ID ${message.data.instance_id} does not exist`);
 		}
+
+		let {field, prop, value} = message.data;
+		instance.config.setProp(field, prop, value);
+		await this.updateInstanceConfig(instance);
 	}
 
 	async assignInstanceCommandRequestHandler(message, request) {
@@ -652,9 +657,7 @@ class SlaveConnection extends BaseConnection {
 		let prev = instance.status;
 		instance.status = "initialized";
 		console.log(`Clusterio | Instance ${instance.config.get("instance.name")} Initialized`)
-		for (let masterPlugin of masterPlugins.values()) {
-			await masterPlugin.onInstanceStatusChanged(instance, prev);
-		}
+		await plugin.invokeHook(masterPlugins, "onInstanceStatusChanged", instance, prev);
 	}
 
 	async instanceStartedEventHandler(message, event) {
@@ -662,9 +665,7 @@ class SlaveConnection extends BaseConnection {
 		let prev = instance.status;
 		instance.status = "running";
 		console.log(`Clusterio | Instance ${instance.config.get("instance.name")} Started`)
-		for (let masterPlugin of masterPlugins.values()) {
-			await masterPlugin.onInstanceStatusChanged(instance, prev);
-		}
+		await plugin.invokeHook(masterPlugins, "onInstanceStatusChanged", instance, prev);
 	}
 
 	async instanceStoppedEventHandler(message, event) {
@@ -672,9 +673,7 @@ class SlaveConnection extends BaseConnection {
 		let prev = instance.status;
 		instance.status = "stopped";
 		console.log(`Clusterio | Instance ${instance.config.get("instance.name")} Stopped`)
-		for (let masterPlugin of masterPlugins.values()) {
-			await masterPlugin.onInstanceStatusChanged(instance, prev);
-		}
+		await plugin.invokeHook(masterPlugins, "onInstanceStatusChanged", instance, prev);
 	}
 
 	async updateInstancesEventHandler(message) {
@@ -700,9 +699,7 @@ class SlaveConnection extends BaseConnection {
 					let prev = masterInstance.status;
 					masterInstance.status = instance.status;
 					if (prev !== undefined) {
-						for (let masterPlugin of masterPlugins.values()) {
-							await masterPlugin.onInstanceStatusChanged(instance, prev);
-						}
+						await plugin.invokeHook(masterPlugins, "onInstanceStatusChanged", instance, prev);
 					}
 				}
 				continue;

slave.js

@@ -17,6 +17,7 @@ const events = require("events");
 const pidusage = require("pidusage");
 const setBlocking = require("set-blocking");
 const phin = require("phin");
+const util = require("util");
 const version = require("./package").version;
 
 // internal libraries
@@ -55,6 +56,49 @@ const instanceFactorioAutosaveSize = new prometheus.Gauge(
 	{ labels: ["instance_id"] }
 );
 
+function applyAsConfig(name) {
+	return async function action(server, value) {
+		if (name === "tags" && value instanceof Array) {
+			// Replace spaces with non-break spaces and delimit by spaces.
+			// This does change the defined tags, but there doesn't seem to
+			// be a way to include a space into a tag from the console.
+			value = value.map(tag => tag.replace(/ /g, "\u00a0")).join(" ");
+		}
+		try {
+			await server.sendRcon(`/config set ${name} ${value}`);
+		} catch (err) {
+			console.error(`Error applying server setting ${name}`, err);
+		}
+	}
+}
+
+const serverSettingsActions = {
+	"afk_autokick_interval": applyAsConfig("afk-auto-kick"),
+	"allow_commands": applyAsConfig("allow-commands"),
+	"autosave_interval": applyAsConfig("autosave-interval"),
+	"autosave_only_on_server": applyAsConfig("autosave-only-on-server"),
+	"description": applyAsConfig("description"),
+	"ignore_player_limit_for_returning_players": applyAsConfig("ignore-player-limit-for-returning-players"),
+	"max_players": applyAsConfig("max-players"),
+	"max_upload_slots": applyAsConfig("max-upload-slots"),
+	"max_upload_in_kilobytes_per_second": applyAsConfig("max-upload-speed"),
+	"name": applyAsConfig("name"),
+	"only_admins_can_pause_the_game": applyAsConfig("only-admins-can-pause"),
+	"game_password": applyAsConfig("password"),
+	"require_user_verification": applyAsConfig("require-user-verification"),
+	"tags": applyAsConfig("tags"),
+	"visibility": async function action(server, value) {
+		for (let scope of ["lan", "public", "steam"]) {
+			try {
+				let enabled = Boolean(value[scope]);
+				await server.sendRcon(`/config set visibility-${scope} ${enabled}`);
+			} catch (err) {
+				console.error(`Error applying visibility ${scope}`, err);
+			}
+		}
+	},
+};
+
 /**
  * Keeps track of the runtime parameters of an instance
  */
@@ -68,6 +112,17 @@ class Instance extends link.Link{
 		this.plugins = new Map();
 		this.config = instanceConfig;
 
+		this._configFieldChanged = (group, field, prev) => {
+			let hook = () => plugin.invokeHook(this.plugins, "onInstanceConfigFieldChanged", group, field, prev);
+
+			if (group.name === "factorio" && field === "settings") {
+				this.updateFactorioSettings(group.get(field), prev).finally(hook);
+			} else {
+				hook();
+			}
+		};
+		this.config.on("fieldChanged", this._configFieldChanged);
+
 		let serverOptions = {
 			version: this.config.get("factorio.version"),
 			gamePort: this.config.get('factorio.game_port'),
@@ -89,11 +144,7 @@ class Instance extends link.Link{
 		this.server.on('output', (output) => {
 			link.messages.instanceOutput.send(this, { instance_id: this.config.get("instance.id"), output })
 
-			for (let [name, plugin] of this.plugins) {
-				plugin.onOutput(output).catch(err => {
-					console.error(`Plugin ${name} raised error in onOutput:`, err);
-				});
-			}
+			plugin.invokeHook(this.plugins, "onOutput", output);
 		});
 
 		this.server.on("error", err => {
@@ -116,6 +167,8 @@ class Instance extends link.Link{
 		this._running = false;
 		link.messages.instanceStopped.send(this, { instance_id: this.config.get("instance.id") });
 
+		this.config.off("fieldChanged", this._configFieldChanged);
+
 		// Clear metrics this instance is exporting
 		for (let collector of prometheus.defaultRegistry.collectors) {
 			if (
@@ -172,15 +225,16 @@ class Instance extends link.Link{
 	}
 
 	/**
-	 * Write the server-settings.json file
+	 * Resolve the effective Factorio server settings
 	 *
-	 * Generate the server-settings.json file from the example file in the
-	 * data directory and override any settings configured in the instance's
-	 * factorio_settings config entry.
+	 * Use the example settings as the basis and override it with all the
+	 * entries from the given settings object.
+	 *
+	 * @param {Object} overrides - Server settings to override.
+	 * @returns server example settings with the given settings applied over it.
 	 */
-	async writeServerSettings() {
+	async resolveServerSettings(overrides) {
 		let serverSettings = await this.server.exampleSettings();
-		let overrides = this.config.get('factorio.settings');
 
 		for (let [key, value] of Object.entries(overrides)) {
 			if (!Object.hasOwnProperty.call(serverSettings, key)) {
@@ -189,6 +243,18 @@ class Instance extends link.Link{
 			serverSettings[key] = value;
 		}
 
+		return serverSettings;
+	}
+
+	/**
+	 * Write the server-settings.json file
+	 *
+	 * Generate the server-settings.json file from the example file in the
+	 * data directory and override any settings configured in the instance's
+	 * factorio_settings config entry.
+	 */
+	async writeServerSettings() {
+		let serverSettings = await this.resolveServerSettings(this.config.get("factorio.settings"));
 		await fs.writeFile(
 			this.server.writePath("server-settings.json"),
 			JSON.stringify(serverSettings, null, 4)
@@ -337,9 +403,7 @@ class Instance extends link.Link{
 		await this.server.disableAchievements()
 		await this.updateInstanceData();
 
-		for (let pluginInstance of this.plugins.values()) {
-			await pluginInstance.onStart();
-		}
+		await plugin.invokeHook(this.plugins, "onStart");
 
 		this._running = true;
 		link.messages.instanceStarted.send(this, { instance_id: this.config.get("instance.id") });
@@ -354,6 +418,17 @@ class Instance extends link.Link{
 		await this.server.sendRcon(`/sc clusterio_private.update_instance(${id}, "${name}")`, true);
 	}
 
+	async updateFactorioSettings(current, previous) {
+		current = await this.resolveServerSettings(current);
+		previous = await this.resolveServerSettings(previous);
+
+		for (let [key, action] of Object.entries(serverSettingsActions)) {
+			if (current[key] !== undefined && !util.isDeepStrictEqual(current[key], previous[key])) {
+				await action(this.server, current[key]);
+			}
+		}
+	}
+
 	/**
 	 * Stop the instance
 	 */
@@ -362,35 +437,26 @@ class Instance extends link.Link{
 
 		// XXX this needs more thought to it
 		if (this.server._state === "running") {
-			for (let pluginInstance of this.plugins.values()) {
-				await pluginInstance.onStop();
-			}
-
+			await plugin.invokeHook(this.plugins, "onStop");
 			await this.server.stop();
 		}
 	}
 
 	async masterConnectionEventEventHandler(message) {
-		for (let pluginInstance of this.plugins.values()) {
-			pluginInstance.onMasterConnectionEvent(message.data.event);
-		}
+		await plugin.invokeHook(this.plugins, "onMasterConnectionEvent", message.data.event);
 	}
 
 	async prepareMasterDisconnectRequestHandler() {
-		for (let pluginInstance of this.plugins.values()) {
-			await pluginInstance.onPrepareMasterDisconnect();
-		}
+		await plugin.invokeHook(this.plugins, "onPrepareMasterDisconnect");
 	}
 
 	async getMetricsRequestHandler() {
 		let results = []
 		if (this._running) {
-			for (let pluginInstance of this.plugins.values()) {
-				let pluginResults = await pluginInstance.onMetrics();
-				if (pluginResults !== undefined) {
-					for await (let result of pluginResults) {
-						results.push(prometheus.serializeResult(result))
-					}
+			let pluginResults = await plugin.invokeHook(this.plugins, "onMetrics");
+			for (let metricIterator of pluginResults) {
+				for await (let metric of metricIterator) {
+					results.push(prometheus.serializeResult(metric))
 				}
 			}
 		}
@@ -505,13 +571,15 @@ class Instance extends link.Link{
  * Searches for instances in the provided directory
  *
  * Looks through all sub-dirs of the provided directory for valid
- * instance definitions and returns a mapping of instance id to
- * instance config objects.
+ * instance definitions and updates the mapping of instance id to
+ * instanceInfo objects.
  *
- * @returns {Map<integer, Object>} mapping between instance
+ * @param {Map<integer, Object>} instanceInfos -
+ *     mapping between instance id and information about this instance.
+ * @param {string} instancesDir - Directory containing instances
+ * @param logger - console like logging interface.
  */
-async function discoverInstances(instancesDir, logger) {
-	let instanceInfos = new Map();
+async function discoverInstances(instanceInfos, instancesDir, logger) {
 	for (let entry of await fs.readdir(instancesDir, { withFileTypes: true })) {
 		if (entry.isDirectory()) {
 			let instanceConfig = new config.InstanceConfig();
@@ -529,6 +597,11 @@ async function discoverInstances(instancesDir, logger) {
 				continue;
 			}
 
+			// Ignore instances we have already discovered before.
+			if (instanceInfos.has(instanceConfig.get("instance.id"))) {
+				continue;
+			}
+
 			let instancePath = path.join(instancesDir, entry.name);
 			logger.log(`found instance ${instanceConfig.get("instance.name")} in ${instancePath}`);
 			instanceInfos.set(instanceConfig.get("instance.id"), {
@@ -537,8 +610,6 @@ async function discoverInstances(instancesDir, logger) {
 			});
 		}
 	}
-
-	return instanceInfos;
 }
 
 class InstanceConnection extends link.Link {
@@ -771,7 +842,7 @@ class Slave extends link.Link {
 		let { instance_id, serialized_config } = message.data;
 		let instanceInfo = this.instanceInfos.get(instance_id);
 		if (instanceInfo) {
-			instanceInfo.config.update(serialized_config);
+			instanceInfo.config.update(serialized_config, true);
 			console.log(`Clusterio | Updated config for ${instanceInfo.path}`);
 			// TODO: Notify of update if instance is running
 
@@ -899,7 +970,7 @@ class Slave extends link.Link {
 	 * the slave and master server with the new list of instances.
 	 */
 	async updateInstances() {
-		this.instanceInfos = await discoverInstances(this.config.get("slave.instances_directory"), console);
+		await discoverInstances(this.instanceInfos, this.config.get("slave.instances_directory"), console);
 		let list = [];
 		for (let [instanceId, instanceInfo] of this.instanceInfos) {
 			let instanceConnection = this.instanceConnections.get(instanceId);

test/integration/clusterio.js

@@ -4,7 +4,7 @@ const path = require("path");
 const validateHTML = require('html5-validator');
 const parallel = require('mocha.parallel');
 
-const { slowTest, get, exec, controlConfigPath, instancesDir } = require("./index");
+const { slowTest, get, exec, sendRcon, controlConfigPath, instancesDir } = require("./index");
 
 
 describe("Integration of Clusterio", function() {
@@ -82,6 +82,91 @@ describe("Integration of Clusterio", function() {
 			});
 		});
 
+		describe("set-instance-config-prop", function() {
+			it("applies factorio settings while running", async function() {
+				slowTest(this);
+
+				let testConfigs = [
+					// json name, value to set,
+					// /config name, expected result
+					[
+						"afk_autokick_interval", 2,
+						"afk-auto-kick", "Kick if AFK for more than 2 minutes.",
+					],
+					[
+						"allow_commands", true,
+						"allow-commands", "Yes",
+					],
+					[
+						"autosave_interval", 17,
+						"autosave-interval", "Autosave every 17 minutes.",
+					],
+					[
+						"autosave_only_on_server", false,
+						"autosave-only-on-server", "Autosave only on server: false.",
+					],
+					[
+						"description", "A test server blah blah",
+						"description", "Server description: A test server blah blah",
+					],
+					[
+						"ignore_player_limit_for_returning_players", true,
+						"ignore-player-limit-for-returning-players", "Ignore player limit for returning players: true."
+					],
+					[
+						"max_players", 11,
+						"max-players", "11",
+					],
+					[
+						"max_upload_slots", 7,
+						"max-upload-slots", "7 slots.",
+					],
+					[
+						"max_upload_in_kilobytes_per_second", 123,
+						"max-upload-speed", "123 kilobytes per second.",
+					],
+					[
+						"name", "A test",
+						"name", "Server name: A test",
+					],
+					[
+						"only_admins_can_pause_the_game", false,
+						"only-admins-can-pause", "Only admins can pause: false.",
+					],
+					[
+						"game_password", "secret",
+						"password", "The server currently has a password.",
+					],
+					[
+						"tags", ["clusterio", "test-tag"],
+						"tags", "Server tags: clusterio test-tag",
+					],
+					[
+						"visibility", { lan: false, public: false },
+						"visibility-lan", "LAN visibility: false.",
+					],
+
+					// Public visibility must be reset before verify can be reset
+					[
+						"require_user_verification", false,
+						"require-user-verification", "Verify user identity: false.",
+					],
+				];
+
+				for (let [prop, value, , ] of testConfigs) {
+					value = `"'${JSON.stringify(value).replace(/"/g, process.platform === "win32" ? '""' : '\\"')}'"`;
+					let args = `--instance test --field factorio.settings --prop ${prop} --value ${value}`;
+					await exec(`node clusterctl --config ${controlConfigPath} set-instance-config-prop ${args}`);
+				}
+
+				// Do this afterwards to leave enough to time for the
+				// changes to have propogated.
+				for (let [, , configName, expectedResult] of testConfigs) {
+					assert.equal(await sendRcon(44, `/config get ${configName}`), `${expectedResult}\n`);
+				}
+			});
+		});
+
 		describe("stop-instance", function() {
 			it("stops the instance", async function() {
 				slowTest(this);

test/integration/index.js

@@ -6,9 +6,31 @@ const needle = require("needle");
 const util = require("util");
 const events = require("events");
 
+const link = require("lib/link");
 const server = require("lib/factorio/server");
 
 
+class TestControl extends link.Link {
+	constructor(connector) {
+		super("control", "master", connector);
+		link.attachAllMessages(this);
+	}
+
+	async prepareDisconnectRequestHandler(message, request) {
+		this.connector.setClosing();
+		return await super.prepareDisconnectRequestHandler(message, request);
+	}
+
+	async debugWsMessageEventHandler() { }
+	async instanceOutputEventHandler() { }
+}
+
+class TestControlConnector extends link.WebSocketClientConnector {
+	register() {
+		this.sendHandshake("register_control", { token: this.token, agent: "clusterctl", version: "test" });
+	}
+}
+
 // Mark that this test takes a lot of time, or depeneds on a test
 // that takes a lot of time.
 function slowTest(test) {
@@ -29,6 +51,7 @@ async function get(path) {
 
 let masterProcess;
 let slaveProcess;
+let control;
 
 let url = "https://localhost:4443/";
 let token = jwt.sign({ id: "api" }, "TestSecretDoNotUse");
@@ -43,6 +66,11 @@ async function exec(...args) {
 	return await util.promisify(child_process.exec)(...args);
 }
 
+async function sendRcon(instanceId, command) {
+	let response = await link.messages.sendRcon.send(control, { instance_id: instanceId, command })
+	return response.result;
+}
+
 function spawn(name, cmd, waitFor) {
 	return new Promise((resolve, reject) => {
 		console.log(cmd);
@@ -94,6 +122,11 @@ before(async function() {
 
 	masterProcess = await spawn("master:", `node master --config ${masterConfigPath} run`, "All plugins loaded");
 	slaveProcess = await spawn("slave:", `node slave --config ${slaveConfigPath} run`, "SOCKET | received ready from master");
+
+	let controlConnector = new TestControlConnector(url, 2);
+	controlConnector.token = token;
+	control = new TestControl(controlConnector);
+	await controlConnector.connect();
 });
 
 after(async function() {
@@ -118,9 +151,13 @@ process.on("exit", () => {
 
 
 module.exports = {
+	TestControl,
+	TestControlConnector,
 	slowTest,
 	get,
 	exec,
+	sendRcon,
+	control,
 
 	url,
 	token,

test/integration/link.js

@@ -1,31 +1,8 @@
 const assert = require("assert").strict;
 const events = require("events");
 
-const link = require("lib/link");
+const { TestControlConnector, TestControl, get, exec, url, token } = require("./index");
 
-const { get, exec, url, token } = require("./index");
-
-
-class TestControl extends link.Link {
-	constructor(connector) {
-		super("control", "master", connector);
-		link.attachAllMessages(this);
-	}
-
-	async prepareDisconnectRequestHandler(message, request) {
-		this.connector.setClosing();
-		return await super.prepareDisconnectRequestHandler(message, request);
-	}
-
-	async debugWsMessageEventHandler() { }
-	async instanceOutputEventHandler() { }
-}
-
-class TestControlConnector extends link.WebSocketClientConnector {
-	register() {
-		this.sendHandshake("register_control", { token: this.token, agent: "clusterctl", version: "test" });
-	}
-}
 
 describe("Integration of lib/link", function() {
 	let control;

test/lib/config/classes.js

@@ -73,7 +73,7 @@ describe("lib/config/classes", function() {
 				TestGroup.define({ name: "test", type: "string", optional: true });
 				TestGroup.define({ name: "func", type: "number", initial_value: () => 42 });
 				TestGroup.define({ name: "bool", type: "boolean", initial_value: false, optional: true });
-				TestGroup.define({ name: "json", type: "object", initial_value: {} });
+				TestGroup.define({ name: "json", type: "object", initial_value: {}, optional: true });
 
 				assert(TestGroup._definitions.has("test"), "field test was not defined");
 				assert(TestGroup._definitions.has("func"), "field func was not defined");
@@ -156,6 +156,14 @@ describe("lib/config/classes", function() {
 			});
 		});
 
+		describe("get name", function() {
+			it("should give the name of the group", async function() {
+				let testInstance = new TestGroup();
+				await testInstance.init();
+				assert.equal(testInstance.name, "test_group");
+			});
+		});
+
 		describe(".serialize()", function() {
 			it("should serialize a basic group", async function() {
 				let testInstance = new TestGroup();
@@ -177,7 +185,7 @@ describe("lib/config/classes", function() {
 		describe(".load()", function() {
 			it("should load a basic group", async function() {
 				let testInstance = new TestGroup();
-				await testInstance.load({ name: "test_group", fields: {
+				await testInstance.load(null, { name: "test_group", fields: {
 					enum: "c",
 					test: "blah",
 					func: 22,
@@ -193,7 +201,7 @@ describe("lib/config/classes", function() {
 			});
 			it("should load defaults for missing serialized fields", async function() {
 				let testInstance = new TestGroup();
-				await testInstance.load({ name: "test_group", fields: { enum: "a" } });
+				await testInstance.load(null, { name: "test_group", fields: { enum: "a" } });
 
 				assert.equal(testInstance.get("enum"), "a");
 				assert.equal(testInstance.get("test"), null);
@@ -213,7 +221,7 @@ describe("lib/config/classes", function() {
 				}
 
 				let testInstance = new TestGroup();
-				await testInstance.load({ name: "test_group", fields: testFields });
+				await testInstance.load(null, { name: "test_group", fields: testFields });
 
 				assert.deepEqual(testInstance.serialize().fields, testFields);
 			});
@@ -223,7 +231,7 @@ describe("lib/config/classes", function() {
 			it("should skip updating invalid values", async function() {
 				let testInstance = new TestGroup();
 				await testInstance.init();
-				testInstance.update({ name: "test_group", fields: { enum: "wrong", test: 3, func: null }});
+				testInstance.update({ name: "test_group", fields: { enum: "wrong", test: 3, func: null }}, false);
 
 				assert.equal(testInstance.get("enum"), "b");
 				assert.equal(testInstance.get("test"), null);
@@ -237,19 +245,19 @@ describe("lib/config/classes", function() {
 					new Error("Expected object, not undefined for ConfigGroup")
 				);
 				assert.throws(
-					() => testInstance.update({}),
+					() => testInstance.update({}, false),
 					new Error("Expected group name test_group, not undefined")
 				);
 				assert.throws(
-					() => testInstance.update({ name: "test_group" }),
+					() => testInstance.update({ name: "test_group" }, false),
 					new Error("Expected fields to be an object, not undefined")
 				);
 				assert.throws(
-					() => testInstance.update({ name: "test_group", fields: []}),
+					() => testInstance.update({ name: "test_group", fields: []}, false),
 					new Error("Expected fields to be an object, not array")
 				);
 				assert.throws(
-					() => testInstance.update({ name: "test_group", fields: null}),
+					() => testInstance.update({ name: "test_group", fields: null}, false),
 					new Error("Expected fields to be an object, not null")
 				);
 			});
@@ -257,9 +265,9 @@ describe("lib/config/classes", function() {
 
 		describe(".set()", function() {
 			let testInstance;
-			before(function() {
+			before(async function() {
 				testInstance = new TestGroup();
-				testInstance.update({ name: "test_group", fields: {
+				await testInstance.load(null, { name: "test_group", fields: {
 					enum: "a", test: "blah", func: 27,
 				}});
 			});
@@ -267,7 +275,7 @@ describe("lib/config/classes", function() {
 			it("should throw if field does not exist", function() {
 				assert.throws(
 					() => testInstance.set("bar", 1),
-					new Error("No field named 'bar'")
+					new classes.InvalidField("No field named 'bar'")
 				);
 			})
 
@@ -351,6 +359,42 @@ describe("lib/config/classes", function() {
 				);
 			});
 		});
+
+		describe(".setProp()", function() {
+			let testInstance;
+			before(async function() {
+				testInstance = new TestGroup();
+				await testInstance.load(null, { name: "test_group", fields: {
+					enum: "a", test: "blah", func: 27,
+				}});
+			});
+
+			it("should throw if field does not exist", function() {
+				assert.throws(
+					() => testInstance.setProp("bar", 1),
+					new classes.InvalidField("No field named 'bar'")
+				);
+			})
+
+			it("should throw if field is not an object", function() {
+				assert.throws(
+					() => testInstance.setProp("enum", "a"),
+					new classes.InvalidField("Cannot set property on non-object field 'enum'")
+				);
+			})
+
+			it("should work if field is an object", function() {
+				testInstance.set("json", { prev: 32, test: false });
+				testInstance.setProp("json", "test", true);
+				assert.deepEqual(testInstance.get("json"), { prev: 32, test: true });
+			});
+
+			it("should handle field being null", function() {
+				testInstance.set("json", null);
+				testInstance.setProp("json", "test", true);
+				assert.deepEqual(testInstance.get("json"), { test: true });
+			});
+		});
 	});
 
 	describe("Config", function() {
@@ -364,7 +408,7 @@ describe("lib/config/classes", function() {
 			AlphaGroup.define({ name: "foo", type: "string", optional: true });
 			AlphaGroup.finalize();
 			TestConfig.registerGroup(AlphaGroup);
-			BetaGroup.define({ name: "bar", type: "number", initial_value: 2 });
+			BetaGroup.define({ name: "bar", type: "object", initial_value: {} });
 			BetaGroup.finalize();
 			TestConfig.registerGroup(BetaGroup);
 		});
@@ -416,7 +460,7 @@ describe("lib/config/classes", function() {
 				let testInstance = new TestConfig();
 				await testInstance.init();
 				assert.equal(testInstance.get("alpha.foo"), null);
-				assert.equal(testInstance.get("beta.bar"), 2);
+				assert.deepEqual(testInstance.get("beta.bar"), {});
 			})
 		});
 
@@ -432,7 +476,7 @@ describe("lib/config/classes", function() {
 						},
 						{
 							name: "beta",
-							fields: { bar: 2 },
+							fields: { bar: {} },
 						}
 					]
 				});
@@ -461,7 +505,7 @@ describe("lib/config/classes", function() {
 				await testInstance.load({ groups: [{ name: "alpha", fields: { foo: "a" }}] });
 
 				assert.equal(testInstance.get("alpha.foo"), "a");
-				assert.equal(testInstance.get("beta.bar"), 2);
+				assert.deepEqual(testInstance.get("beta.bar"), {});
 			});
 
 			it("should preserve unknown groups when serialized back", async function() {
@@ -476,7 +520,7 @@ describe("lib/config/classes", function() {
 					},
 					{
 						name: "beta",
-						fields: { bar: 20 },
+						fields: { bar: { value: 20 }},
 					}
 				]
 
@@ -498,9 +542,9 @@ describe("lib/config/classes", function() {
 					},
 					{
 						name: "beta",
-						fields: { bar: 30 },
+						fields: { bar: { value: 30 }},
 					}
-				]});
+				]}, false);
 
 				assert.deepEqual(testInstance.serialize(), {
 					groups: [
@@ -514,7 +558,7 @@ describe("lib/config/classes", function() {
 						},
 						{
 							name: "beta",
-							fields: { bar: 30 },
+							fields: { bar: { value: 30 }},
 						}
 					],
 				});
@@ -563,5 +607,35 @@ describe("lib/config/classes", function() {
 			});
 		});
 
+		describe("fieldChanged event", function() {
+			let testInstance;
+			let called;
+			beforeEach(async function() {
+				testInstance = new TestConfig();
+				await testInstance.init();
+
+				called = false;
+				testInstance.once("fieldChanged", (group, field, prev) => {
+					if (group.name === "beta" && field === "bar") {
+						called = true;
+					}
+				});
+			});
+
+			it("should be called when setting a field", async function() {
+				testInstance.set("beta.bar", { value: 1 });
+				assert(called, "fieldChanged was not called");
+			});
+
+			it("should be called when updating a config", async function() {
+				testInstance.update({ groups: [{ name: "beta", fields: { bar: { value: 1 }}}]}, true);
+				assert(called, "fieldChanged was not called");
+			});
+
+			it("should be called when setting a prop", async function() {
+				testInstance.setProp("beta.bar", "value", 2);
+				assert(called, "fieldChanged was not called");
+			});
+		});
 	});
 });

test/lib/plugin.js

@@ -119,4 +119,31 @@ describe("lib/plugin", function() {
 			);
 		});
 	});
+
+	describe("invokeHook()", function() {
+		let betaTestCalled = false;
+		let plugins = new Map([
+			["alpha", {
+				test: async function() { },
+				pass: async function(arg) { return arg; },
+				error: async function() { throw new Error("Test"); },
+			}],
+			["beta", {
+				test: async function() { betaTestCalled = true; },
+				pass: async function() { },
+				error: async function() { },
+			}],
+		]);
+		it("should invoke the hook on the plugin", async function() {
+			await plugin.invokeHook(plugins, "test");
+			assert(betaTestCalled, "Hook was not called");
+		});
+		it("should pass and return args", async function() {
+			let result = await plugin.invokeHook(plugins, "pass", 1234);
+			assert.deepEqual(result, [1234]);
+		});
+		it("should ignore errors", async function() {
+			await plugin.invokeHook(plugins, "error");
+		});
+	});
 });

test/slave.js

@@ -157,7 +157,8 @@ describe("Slave testing", function() {
 		it("should discover test instance", async function() {
 			let logger = { log: () => {}, error: () => {} };
 			let instancePath = path.join("test", "file", "instances");
-			let instanceInfos = await slave._discoverInstances(instancePath, logger);
+			let instanceInfos = new Map();
+			await slave._discoverInstances(instanceInfos, instancePath, logger);
 
 			let referenceConfig = new config.InstanceConfig();
 			await referenceConfig.init();