From b5c372f109f874614bea8a6bc16fdc92d1aeb6c7 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Thu, 29 Oct 2026 16:19:08 +0000 Subject: [PATCH] Rename bases to templates and add per property inheritance Review changes for #1142: - base becomes template everywhere (is_template, template_id, --template, --is-template, --templates, InstanceDetails.isTemplate). - Object fields inherit per property. inherited_fields lists the overridden properties for object fields and the template object is merged with them. - Linking a template also inherits fields that hold the default value. The server name stays per instance. - Ports are never inherited. - InstanceDetails carries templateId. Web UI: inherited values are locked until unlocked, overridden values can be inherited again, templates are listed separately, the create modal can create templates and instances from templates, and the instance page has "Create template" which can link the source. Part of #200 Co-Authored-By: Claude Opus 5.5 --- docs/configuration.md | 26 +- docs/managing-a-cluster.md | 14 +- packages/controller/src/ControlConnection.ts | 29 +- packages/controller/src/InstanceManager.ts | 287 ++++++++++++------ packages/controller/src/InstanceRecord.ts | 3 +- packages/ctl/src/commands_instance.ts | 56 ++-- packages/lib/src/config/definitions.ts | 49 ++- packages/lib/src/data/InstanceDetails.ts | 16 +- packages/lib/src/data/messages_instance.ts | 4 +- packages/web_ui/src/bootstrap.tsx | 2 + .../web_ui/src/components/BaseConfigTree.tsx | 94 +++++- .../web_ui/src/components/InputInstance.tsx | 9 +- .../src/components/InstanceConfigTree.tsx | 35 ++- .../web_ui/src/components/InstanceList.tsx | 3 +- .../src/components/InstanceViewPage.tsx | 92 ++++-- .../web_ui/src/components/InstancesPage.tsx | 131 ++++++-- test/controller/ControlConnection.test.js | 77 +++-- test/controller/InstanceManager.test.js | 177 +++++++---- test/integration/clusterio.js | 49 +-- test/lib/config/definitions.js | 22 +- test/lib/data/InstanceDetails.test.js | 2 + 21 files changed, 865 insertions(+), 312 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 54e1f015..d6398ba7 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -521,26 +521,28 @@ This allows you to prevent certain instances from being started when using the b Defaults to false. -### instance.is_base +### instance.is_template -If enabled this instance is a base other instances can inherit config values from. -Base instances cannot be assigned to a host and therefore never start. -Hidden from `instance list` in clusterioctl unless `--bases` is passed. -An instance cannot stop being a base while other instances inherit from it. +If enabled this instance is a template other instances can inherit config values from. +Templates cannot be assigned to a host and therefore never start. +Hidden from `instance list` in clusterioctl unless `--templates` is passed, and listed separately in the web interface. +An instance cannot stop being a template while other instances inherit from it. Defaults to false. -### instance.base_id +### instance.template_id -ID of the base instance to inherit config values from. -Inherited fields take their value from the base and follow it when it changes, setting a field on this instance stops inheriting it. -When set on an existing instance only fields with the same value as the base become inherited. -`instance.name`, `instance.is_base`, `instance.base_id` and hidden fields are never inherited. -A base may itself inherit from another base, but not from itself through a chain of bases. +ID of the template to inherit config values from. +Inherited fields take their value from the template and follow it when it changes, setting a field on this instance stops inheriting it. +Object fields like `factorio.settings` are inherited per property, properties set on this instance override the ones from the template. +When set on an existing instance fields with the same value as the template or the default value become inherited, except the server name in `factorio.settings`. +`instance.name`, `instance.is_template`, `instance.template_id`, the port fields and hidden fields are never inherited. +A template may itself inherit from another template, but not from itself through a chain of templates. The fields currently inherited are stored in the hidden `instance.inherited_fields` field. +In the web interface inherited values are locked and must be unlocked before they can be changed. -Defaults to null meaning no base. +Defaults to null meaning no template. ### factorio.version diff --git a/docs/managing-a-cluster.md b/docs/managing-a-cluster.md index 14073d9d..67cd5a13 100644 --- a/docs/managing-a-cluster.md +++ b/docs/managing-a-cluster.md @@ -79,16 +79,16 @@ Marks the host as requiring a restart, the same way a plugin update does. Use it ### List Instances - ctl> instance list [--bases] + ctl> instance list [--templates] List instances. -Base instances are only listed if `--bases` is passed. +Templates are only listed if `--templates` is passed. ### Create instance - ctl> instance create [--id ] [--from ] [--base ] [--is-base] + ctl> instance create [--id ] [--from ] [--template ] [--is-template] -Create a new instance. `name` is the instance name. `--id` is the optional id. `--from` copies the config of an existing instance. `--base` makes the new instance inherit all config values from the given base instance. `--is-base` creates the instance as a base, see `instance.is_base` in the [configuration docs](configuration.md). +Create a new instance. `name` is the instance name. `--id` is the optional id. `--from` copies the config of an existing instance. `--template` makes the new instance inherit all config values from the given template. `--is-template` creates the instance as a template, see `instance.is_template` in the [configuration docs](configuration.md). Use `--is-template --from ` to make a template out of an existing instance. ### Kill instance @@ -121,7 +121,7 @@ Most notably Clusterio records wall clock time while the save time is recorded i ctl> instance config list -List config values for an instance. `instance` is the instance whose config values to list. Values inherited from a base instance are marked with `(inherited)`. +List config values for an instance. `instance` is the instance whose config values to list. Values inherited from a template are marked with `(inherited)`, object fields also list the properties set on the instance. #### Set config value @@ -135,9 +135,9 @@ To be written. #### Inherit config value - ctl> instance config inherit + ctl> instance config inherit [prop] -Make a field in the instance config take its value from the base instance again, undoing a previous `set`. `instance` is the instance to update, `field` is the field to inherit. +Make a field in the instance config take its value from the template again, undoing a previous `set`. `instance` is the instance to update, `field` is the field to inherit, `prop` limits it to one property of an object field. #### Edit instance config diff --git a/packages/controller/src/ControlConnection.ts b/packages/controller/src/ControlConnection.ts index 72b8ae88..aa618603 100644 --- a/packages/controller/src/ControlConnection.ts +++ b/packages/controller/src/ControlConnection.ts @@ -317,11 +317,11 @@ export default class ControlConnection extends BaseConnection { instanceConfig.set("instance.assigned_host", null); // New instances are unassigned } instanceConfig.update(request.config, false, "control"); - if (Object.hasOwn(request.config, "instance.base_id")) { - this._controller.instances.checkInheritance( - instanceConfig, instanceConfig.get("instance.is_base"), instanceConfig.get("instance.base_id") - ); - this._controller.instances.applyBase(instanceConfig, Object.keys(request.config)); + this._controller.instances.checkInheritance( + instanceConfig, instanceConfig.get("instance.is_template"), instanceConfig.get("instance.template_id") + ); + if (Object.hasOwn(request.config, "instance.template_id")) { + this._controller.instances.applyTemplate(instanceConfig, Object.keys(request.config)); } await this._controller.instances.createInstance(instanceConfig); } @@ -365,7 +365,12 @@ export default class ControlConnection extends BaseConnection { this.checkStagedInheritance(instance); instance.config.commitStaging(); - this._controller.instances.overrideFields(instance, Object.keys(request.fields)); + for (const [fieldName, fieldValue] of Object.entries(request.fields)) { + const props = typeof fieldValue === "object" + ? Object.keys(fieldValue).filter(prop => fieldValue[prop] !== undefined) + : undefined; + this._controller.instances.overrideField(instance, fieldName, props); + } await this._controller.instanceConfigUpdated(instance); } finally { instance.config.revertStaging(); @@ -375,7 +380,7 @@ export default class ControlConnection extends BaseConnection { private checkStagedInheritance(instance: InstanceRecord) { const staged = instance.config.staging; this._controller.instances.checkInheritance( - instance.config, staged.get("instance.is_base"), staged.get("instance.base_id") + instance.config, staged.get("instance.is_template"), staged.get("instance.template_id") ); } @@ -383,7 +388,7 @@ export default class ControlConnection extends BaseConnection { const instance = this._controller.instances.getForRequest(request.instanceId); const field = request.field as keyof lib.InstanceConfigFields; ControlConnection.validateInstanceConfigSetField(field); - if (["instance.is_base", "instance.base_id"].includes(field)) { + if (["instance.is_template", "instance.template_id"].includes(field)) { try { instance.config.stage(field, request.value, "control"); this.checkStagedInheritance(instance); @@ -392,7 +397,7 @@ export default class ControlConnection extends BaseConnection { } } instance.config.set(field, request.value, "control"); - this._controller.instances.overrideFields(instance, [field]); + this._controller.instances.overrideField(instance, field); await this._controller.instanceConfigUpdated(instance); } @@ -401,13 +406,15 @@ export default class ControlConnection extends BaseConnection { let { field, prop, value } = request; ControlConnection.validateInstanceConfigSetField(field); instance.config.setProp(field as keyof lib.InstanceConfigFields, prop, value, "control"); - this._controller.instances.overrideFields(instance, [field]); + if (value !== undefined) { + this._controller.instances.overrideField(instance, field, [prop]); + } await this._controller.instanceConfigUpdated(instance); } async handleInstanceConfigInheritFieldRequest(request: lib.InstanceConfigInheritFieldRequest) { ControlConnection.validateInstanceConfigSetField(request.field); - this._controller.instances.inheritField(request.instanceId, request.field); + this._controller.instances.inheritField(request.instanceId, request.field, request.prop); await this._controller.instanceConfigUpdated(this._controller.instances.getForRequest(request.instanceId)); } diff --git a/packages/controller/src/InstanceManager.ts b/packages/controller/src/InstanceManager.ts index 38802a96..ffbeddc9 100644 --- a/packages/controller/src/InstanceManager.ts +++ b/packages/controller/src/InstanceManager.ts @@ -6,6 +6,9 @@ import type Controller from "./Controller.js"; import type HostConnection from "./HostConnection.js"; import InstanceRecord from "./InstanceRecord.js"; +type InheritedFields = lib.InstanceConfigFields["instance.inherited_fields"]; +type InheritableField = keyof lib.InstanceConfigFields & string; + export default class InstanceManager { private static readonly DefaultFactorioSettings = { tags: ["clusterio"], @@ -26,7 +29,7 @@ export default class InstanceManager { non_blocking_saving: false, } as const; - /** Non-zero while base values are being copied into inheriting instances */ + /** Non-zero while template values are being copied into inheriting instances */ private _inheriting = 0; constructor( @@ -111,10 +114,10 @@ export default class InstanceManager { */ addInstanceHooks(instance: InstanceRecord): void { instance.config.on("fieldChanged", (field: string, curr: unknown, prev: unknown) => { - if (field === "instance.base_id") { - this._rebase(instance); + if (field === "instance.template_id") { + this._link(instance); } else if (!this._inheriting && instance.config.isInherited(field)) { - this._setInherited(instance, [field], false); + this._override(instance, field, curr, prev); } this.records.set(instance); this._notifyConfigFieldChanged(instance, field, curr, prev); @@ -127,168 +130,249 @@ export default class InstanceManager { /* ------------------------------------------------------------------------- */ /** - * Get the instances that inherit directly from the given base. + * Get the instances that inherit directly from the given template. * - * @param baseId - ID of the base instance. - * @returns Instances with `instance.base_id` set to baseId. + * @param templateId - ID of the template. + * @returns Instances with `instance.template_id` set to templateId. */ - inheritorsOf(baseId: number): InstanceRecord[] { + inheritorsOf(templateId: number): InstanceRecord[] { return [...this.records.values()].filter( - instance => instance.config.get("instance.base_id") === baseId + instance => instance.config.get("instance.template_id") === templateId ); } /** - * Check that new values for `instance.is_base` and `instance.base_id` are allowed. + * Check that new values for `instance.is_template` and `instance.template_id` are allowed. * * @param config - Current config of the instance. - * @param isBase - New value for `instance.is_base`. - * @param baseId - New value for `instance.base_id`. - * @throws {module:lib.ValidationError} if the base is not a base or would create a cycle. + * @param isTemplate - New value for `instance.is_template`. + * @param templateId - New value for `instance.template_id`. + * @throws {module:lib.ValidationError} if the template is not a template or would create a cycle. * @throws {module:lib.ConflictError} if the instance is assigned or still has inheritors. */ - checkInheritance(config: lib.InstanceConfig, isBase: boolean, baseId: number | null): void { + checkInheritance(config: lib.InstanceConfig, isTemplate: boolean, templateId: number | null): void { const instanceId = config.get("instance.id"); - if (isBase && config.get("instance.assigned_host") !== null) { - throw new lib.ConflictError("Instance must be unassigned before it can be made a base"); + if (isTemplate && config.get("instance.assigned_host") !== null) { + throw new lib.ConflictError("Instance must be unassigned before it can be made a template"); } - if (!isBase && config.get("instance.is_base")) { + if (!isTemplate && config.get("instance.is_template")) { const count = this.inheritorsOf(instanceId).length; if (count) { - throw new lib.ConflictError(`Instance is the base of ${count} other instance(s)`); + throw new lib.ConflictError(`Instance is the template of ${count} other instance(s)`); } } - if (baseId === null) { + if (templateId === null) { return; } - const base = this.records.get(baseId); - if (!base) { - throw new lib.NotFoundError(`Instance with ID ${baseId} does not exist`); + const template = this.records.get(templateId); + if (!template) { + throw new lib.NotFoundError(`Instance with ID ${templateId} does not exist`); } - if (!base.config.get("instance.is_base")) { - throw new lib.ValidationError(`Instance ${base.config.get("instance.name")} is not a base`); + if (!template.config.get("instance.is_template")) { + throw new lib.ValidationError(`Instance ${template.config.get("instance.name")} is not a template`); } const visited = new Set(); - for (let id: number | null = baseId; id !== null && !visited.has(id);) { + for (let id: number | null = templateId; id !== null && !visited.has(id);) { if (id === instanceId) { - throw new lib.ValidationError("Base instances cannot inherit from themselves"); + throw new lib.ValidationError("Templates cannot inherit from themselves"); } visited.add(id); - id = this.records.get(id)?.config.get("instance.base_id") ?? null; + id = this.records.get(id)?.config.get("instance.template_id") ?? null; } } /** - * Copy all inheritable values from the base set in `instance.base_id` + * Copy the inheritable values from the template set in `instance.template_id` * into a config that is not yet part of the cluster and mark them inherited. * * @param config - Config of the instance to be created. - * @param keep - Fields to keep the current value of. + * @param own - Fields set by the creator, object fields keep their properties as overrides. */ - applyBase(config: lib.InstanceConfig, keep: string[] = []): void { - const base = this.records.get(config.get("instance.base_id")!); - if (!base) { + applyTemplate(config: lib.InstanceConfig, own: string[] = []): void { + const template = this._templateOf(config); + if (!template) { return; } - const values: Record = {}; - const inherited: Record = {}; - for (const name of Object.keys(lib.InstanceConfig.fieldDefinitions)) { - if (lib.InstanceConfig.isInheritable(name) && !keep.includes(name)) { - values[name] = structuredClone(base.config.get(name as keyof lib.InstanceConfigFields)); + const inherited: InheritedFields = {}; + for (const name of InstanceManager._inheritableFields()) { + const def = lib.InstanceConfig.fieldDefinitions[name]; + if (def.type === "object") { + inherited[name] = own.includes(name) ? Object.keys(config.get(name) as object ?? {}) : []; + } else if (!own.includes(name)) { inherited[name] = true; } } - config.update(values, false); config.set("instance.inherited_fields", inherited); + config.update(InstanceManager._resolve(config, template.config, Object.keys(inherited)), false); } /** - * Make a field take its value from the base instance again. + * Make a field, or a property of an object field, take its value from the template again. * * @param instanceId - ID of the instance to update. * @param field - Name of the field to inherit. - * @throws {module:lib.ValidationError} if the instance has no base or the field cannot be inherited. + * @param prop - Property of an object field to inherit, inherits all properties if not given. + * @throws {module:lib.ValidationError} if the instance has no template or the field cannot be inherited. */ - inheritField(instanceId: number, field: string): void { + inheritField(instanceId: number, field: string, prop?: string): void { const instance = this.getForRequest(instanceId); - const baseId = instance.config.get("instance.base_id"); - const base = baseId !== null ? this.records.get(baseId) : undefined; - if (!base) { - throw new lib.ValidationError("Instance does not have a base"); + if (!this._templateOf(instance.config)) { + throw new lib.ValidationError("Instance does not have a template"); } if (!lib.InstanceConfig.isInheritable(field)) { throw new lib.ValidationError(`Field ${field} cannot be inherited`); } - this._setInherited(instance, [field], true); + const name = field as InheritableField; + const inherited = { ...instance.config.get("instance.inherited_fields") }; + if (lib.InstanceConfig.fieldDefinitions[name].type !== "object") { + inherited[field] = true; + } else if (prop === undefined) { + inherited[field] = []; + } else { + const entry = inherited[field]; + const overrides = entry instanceof Array ? entry : Object.keys(instance.config.get(name) as object ?? {}); + inherited[field] = overrides.filter(p => p !== prop); + } + instance.config.set("instance.inherited_fields", inherited); + this._resolveInto(instance, [field]); + } + + /** + * Stop inheriting the given field or properties, used when a value is explicitly set. + * + * @param instance - Instance the field was set on. + * @param field - Name of the field set. + * @param props - Properties set on an object field. + */ + overrideField(instance: InstanceRecord, field: string, props?: string[]): void { + const entry = instance.config.get("instance.inherited_fields")[field]; + if (!instance.config.isInherited(field)) { + return; + } + const inherited = { ...instance.config.get("instance.inherited_fields") }; + if (entry instanceof Array && props) { + inherited[field] = [...new Set([...entry, ...props])]; + } else { + delete inherited[field]; + } + instance.config.set("instance.inherited_fields", inherited); + } + + private _templateOf(config: lib.InstanceConfig) { + const templateId = config.get("instance.template_id"); + return templateId !== null ? this.records.get(templateId) : undefined; + } + + private static _inheritableFields() { + return Object.keys(lib.InstanceConfig.fieldDefinitions).filter( + name => lib.InstanceConfig.isInheritable(name) + ) as InheritableField[]; + } + + /** Default value of a field, or undefined if it is generated */ + private static _defaultValue(name: InheritableField): unknown { + const def = lib.InstanceConfig.fieldDefinitions[name]; + if (name === "factorio.settings") { + return InstanceManager.DefaultFactorioSettings; + } + return typeof def.initialValue === "function" ? undefined : def.initialValue ?? null; + } + + /** Values the given inherited fields take from the template, with object overrides applied */ + private static _resolve(config: lib.InstanceConfig, template: lib.InstanceConfig, fields: string[]) { + const inherited = config.get("instance.inherited_fields"); + const values: Record = {}; + for (const field of fields) { + const name = field as InheritableField; + const entry = inherited[field]; + if (!entry) { + continue; + } + const value = structuredClone(template.get(name)) as any; + if (entry instanceof Array) { + const own = config.get(name) as Record ?? {}; + for (const prop of entry) { + if (Object.hasOwn(own, prop)) { + value[prop] = structuredClone(own[prop]); + } + } + } + values[field] = value; + } + return values; + } + + /** Update the given inherited fields of a live instance from its template */ + private _resolveInto(instance: InstanceRecord, fields: string[]): void { + const template = this._templateOf(instance.config); + if (!template) { + return; + } + const values = InstanceManager._resolve(instance.config, template.config, fields); this._inheriting += 1; try { - instance.config.update({ - [field]: structuredClone(base.config.get(field as keyof lib.InstanceConfigFields)), - }, true); + instance.config.update(values, true); } finally { this._inheriting -= 1; } } - /** - * Stop inheriting the given fields, used when a value is explicitly set. - * - * @param instance - Instance the fields were set on. - * @param fields - Names of fields set. - */ - overrideFields(instance: InstanceRecord, fields: string[]): void { - const inherited = fields.filter(field => instance.config.isInherited(field)); - if (inherited.length) { - this._setInherited(instance, inherited, false); + /** Track a change to an inherited field made outside of inheritance */ + private _override(instance: InstanceRecord, field: string, curr: unknown, prev: unknown): void { + const entry = instance.config.get("instance.inherited_fields")[field]; + if (!(entry instanceof Array)) { + this.overrideField(instance, field); + return; + } + const currObj = curr as Record ?? {}; + const prevObj = prev as Record ?? {}; + const changed = Object.keys(currObj).filter(p => !isDeepStrictEqual(currObj[p], prevObj[p])); + const removed = Object.keys(prevObj).filter(p => !Object.hasOwn(currObj, p)); + const overrides = [...new Set([...entry, ...changed])].filter(p => !removed.includes(p)); + const inherited = { ...instance.config.get("instance.inherited_fields"), [field]: overrides }; + instance.config.set("instance.inherited_fields", inherited); + if (removed.length) { + // Removed properties fall back to the template value + this._resolveInto(instance, [field]); } } - private _setInherited(instance: InstanceRecord, fields: string[], inherited: boolean): void { - const value = { ...instance.config.get("instance.inherited_fields") }; - for (const field of fields) { - if (inherited) { - value[field] = true; - } else { - delete value[field]; - } - } - instance.config.set("instance.inherited_fields", value); - } - - /** Recompute inherited fields after base_id changed, values equal to the base are inherited */ - private _rebase(instance: InstanceRecord): void { - const baseId = instance.config.get("instance.base_id"); - const base = baseId !== null ? this.records.get(baseId) : undefined; - const inherited: Record = {}; - if (base) { - for (const name of Object.keys(lib.InstanceConfig.fieldDefinitions)) { - const field = name as keyof lib.InstanceConfigFields; - if ( - lib.InstanceConfig.isInheritable(name) - && isDeepStrictEqual(instance.config.get(field), base.config.get(field)) + /** Recompute inherited fields after template_id changed, values equal to the template or default are inherited */ + private _link(instance: InstanceRecord): void { + const template = this._templateOf(instance.config); + const inherited: InheritedFields = {}; + if (template) { + for (const name of InstanceManager._inheritableFields()) { + const value = instance.config.get(name) as any; + const templateValue = template.config.get(name) as any; + const defaultValue = InstanceManager._defaultValue(name) as any; + if (lib.InstanceConfig.fieldDefinitions[name].type === "object") { + inherited[name] = Object.keys(value ?? {}).filter(prop => !( + isDeepStrictEqual(value[prop], templateValue?.[prop]) + || defaultValue && isDeepStrictEqual(value[prop], defaultValue[prop]) + ) || name === "factorio.settings" && prop === "name"); + } else if ( + isDeepStrictEqual(value, templateValue) + || defaultValue !== undefined && isDeepStrictEqual(value, defaultValue) ) { inherited[name] = true; } } } instance.config.set("instance.inherited_fields", inherited); + this._resolveInto(instance, Object.keys(inherited)); } - /** Copy a changed value from a base to the instances inheriting it */ - private _propagate(base: InstanceRecord, field: string, value: unknown): void { + /** Copy a changed value from a template to the instances inheriting it */ + private _propagate(template: InstanceRecord, field: string, value: unknown): void { if (!lib.InstanceConfig.isInheritable(field)) { return; } - for (const instance of this.inheritorsOf(base.id)) { + for (const instance of this.inheritorsOf(template.id)) { if (!instance.config.isInherited(field)) { continue; } - this._inheriting += 1; - try { - instance.config.update({ [field]: structuredClone(value) as any }, true); - } finally { - this._inheriting -= 1; - } + this._resolveInto(instance, [field]); this._controller.instanceConfigUpdated(instance).catch(err => { logger.error(`Failed to send config of instance ${instance.id} to its host:\n${err.stack}`); }); @@ -320,13 +404,22 @@ export default class InstanceManager { throw new lib.ConflictError(`Instance with ID ${instanceId} already exists`); } + const controllerName = this._controller.config.get("controller.name"); + const serverName = `${controllerName} - ${instanceConfig.get("instance.name")}`; + const settings = instanceConfig.get("factorio.settings"); if (!instanceConfig.isInherited("factorio.settings")) { - const controllerName = this._controller.config.get("controller.name"); instanceConfig.set("factorio.settings", { - name: `${controllerName} - ${instanceConfig.get("instance.name")}`, + name: serverName, description: `Clusterio instance for ${controllerName}`, ...InstanceManager.DefaultFactorioSettings, - ...instanceConfig.get("factorio.settings"), + ...settings, + }); + } else if (instanceConfig.isInherited("factorio.settings", "name")) { + // Each instance gets its own server name + instanceConfig.setProp("factorio.settings", "name", serverName); + const inherited = instanceConfig.get("instance.inherited_fields"); + instanceConfig.set("instance.inherited_fields", { + ...inherited, "factorio.settings": [...inherited["factorio.settings"] as string[], "name"], }); } @@ -372,8 +465,8 @@ export default class InstanceManager { if (currentAssignedHost === hostId) { return; } - if (hostId !== null && instance.config.get("instance.is_base")) { - throw new lib.ValidationError("Base instances cannot be assigned to a host"); + if (hostId !== null && instance.config.get("instance.is_template")) { + throw new lib.ValidationError("Templates cannot be assigned to a host"); } let newHostConnection: HostConnection | undefined; @@ -443,7 +536,7 @@ export default class InstanceManager { const hostId = instance.config.get("instance.assigned_host"); const inheritors = this.inheritorsOf(instanceId).length; if (inheritors) { - throw new lib.ConflictError(`Instance is the base of ${inheritors} other instance(s)`); + throw new lib.ConflictError(`Instance is the template of ${inheritors} other instance(s)`); } if (hostId !== null) { diff --git a/packages/controller/src/InstanceRecord.ts b/packages/controller/src/InstanceRecord.ts index a441a88a..02f3aa4e 100644 --- a/packages/controller/src/InstanceRecord.ts +++ b/packages/controller/src/InstanceRecord.ts @@ -83,7 +83,8 @@ export default class InstanceRecord { this.startedAtMs, this.updatedAtMs, this.config.get("instance.exclude_from_start_all"), - this.config.get("instance.is_base"), + this.config.get("instance.is_template"), + this.config.get("instance.template_id") ?? undefined, ); } diff --git a/packages/ctl/src/commands_instance.ts b/packages/ctl/src/commands_instance.ts index c575827b..c52610c6 100644 --- a/packages/ctl/src/commands_instance.ts +++ b/packages/ctl/src/commands_instance.ts @@ -22,12 +22,12 @@ export const instanceCommands = new lib.CommandTree({ instanceCommands.add(new lib.Command({ definition: [["list", "l"], "List instances known to the controller", (yargs) => { yargs.options({ - "bases": { describe: "Include base instances", nargs: 0, type: "boolean", default: false }, + "templates": { describe: "Include templates", nargs: 0, type: "boolean", default: false }, }); }], - handler: async function(args: { bases: boolean }, ctl: Ctl) { + handler: async function(args: { templates: boolean }, ctl: Ctl) { let list = await ctl.send(new lib.InstanceDetailsListRequest()); - print(asTable(list.filter(instance => args.bases || !instance.isBase))); + print(asTable(list.filter(instance => args.templates || !instance.isTemplate))); }, })); @@ -38,12 +38,12 @@ instanceCommands.add(new lib.Command({ yargs.options({ "id": { type: "number", nargs: 1, describe: "Instance id" }, "from": { type: "number", nargs: 1, describe: "Clone config from instance id" }, - "base": { type: "string", nargs: 1, describe: "Base instance to inherit config from" }, - "is-base": { type: "boolean", nargs: 0, describe: "Create the instance as a base" }, + "template": { type: "string", nargs: 1, describe: "Template to inherit config from" }, + "is-template": { type: "boolean", nargs: 0, describe: "Create the instance as a template" }, }); }], handler: async function( - args: { name: string, id?: number, from?: number, base?: string, isBase?: boolean }, + args: { name: string, id?: number, from?: number, template?: string, isTemplate?: boolean }, ctl: Ctl, ) { let instanceConfig = new lib.InstanceConfig("control"); @@ -52,13 +52,13 @@ instanceCommands.add(new lib.Command({ instanceConfig.set("instance.id", args.id); } instanceConfig.set("instance.name", args.name); - if (args.base !== undefined) { - instanceConfig.set("instance.base_id", await lib.resolveInstance(ctl, args.base)); - fields.push("instance.base_id"); + if (args.template !== undefined) { + instanceConfig.set("instance.template_id", await lib.resolveInstance(ctl, args.template)); + fields.push("instance.template_id"); } - if (args.isBase) { - instanceConfig.set("instance.is_base", true); - fields.push("instance.is_base"); + if (args.isTemplate) { + instanceConfig.set("instance.is_template", true); + fields.push("instance.is_template"); } await ctl.send(new lib.InstanceCreateRequest( instanceConfig.toRemote("controller", fields), @@ -77,27 +77,39 @@ instanceConfigCommands.add(new lib.Command({ handler: async function(args: { instance: string }, ctl: Ctl) { let instanceId = await lib.resolveInstance(ctl, args.instance); let config = await ctl.send(new lib.InstanceConfigGetRequest(instanceId)); - const inherited = config["instance.base_id"] !== null - ? config["instance.inherited_fields"] as Record ?? {} + const inherited = config["instance.template_id"] !== null + ? config["instance.inherited_fields"] as Record ?? {} : {}; for (let [name, value] of Object.entries(config)) { if (name === "instance.inherited_fields") { continue; } - print(`${name} ${JSON.stringify(value)}${inherited[name] ? " (inherited)" : ""}`); + const entry = inherited[name]; + let note = ""; + if (entry instanceof Array && entry.length) { + note = ` (inherited except ${entry.join(", ")})`; + } else if (entry) { + note = " (inherited)"; + } + print(`${name} ${JSON.stringify(value)}${note}`); } }, })); instanceConfigCommands.add(new lib.Command({ - definition: ["inherit ", "Make field in instance config use the value of its base", (yargs) => { - yargs.positional("instance", { describe: "Instance to update config on", type: "string" }); - yargs.positional("field", { describe: "Field to inherit", type: "string" }); - }], - handler: async function(args: { instance: string, field: string }, ctl: Ctl) { + definition: [ + "inherit [prop]", + "Make field or property in instance config use the value of its template", + (yargs) => { + yargs.positional("instance", { describe: "Instance to update config on", type: "string" }); + yargs.positional("field", { describe: "Field to inherit", type: "string" }); + yargs.positional("prop", { describe: "Property of object field to inherit", type: "string" }); + }, + ], + handler: async function(args: { instance: string, field: string, prop?: string }, ctl: Ctl) { let instanceId = await lib.resolveInstance(ctl, args.instance); - await ctl.send(new lib.InstanceConfigInheritFieldRequest(instanceId, args.field)); + await ctl.send(new lib.InstanceConfigInheritFieldRequest(instanceId, args.field, args.prop)); }, })); @@ -205,7 +217,7 @@ instanceConfigCommands.add(new lib.Command({ const final = await configToKeyVal(data); const original = await configToKeyVal(allConfigElements); for (let index in final) { - // Unchanged fields are skipped so they keep inheriting from the base + // Unchanged fields are skipped so they keep inheriting from the template if (index in final && final[index] !== original[index]) { try { await ctl.send(new lib.InstanceConfigSetFieldRequest( diff --git a/packages/lib/src/config/definitions.ts b/packages/lib/src/config/definitions.ts index 061d24dd..a76698f0 100644 --- a/packages/lib/src/config/definitions.ts +++ b/packages/lib/src/config/definitions.ts @@ -488,9 +488,9 @@ export interface InstanceConfigFields { "instance.assigned_host": number | null; "instance.auto_start": boolean; "instance.exclude_from_start_all": boolean; - "instance.is_base": boolean; - "instance.base_id": number | null; - "instance.inherited_fields": Record; + "instance.is_template": boolean; + "instance.template_id": number | null; + "instance.inherited_fields": Record; [key: `${string}.load_plugin`]: boolean; [key: `${string}.version`]: string; @@ -542,10 +542,13 @@ export class InstanceConfig extends classes.Config { return config; } - /** Fields that are never inherited from a base instance */ - static uninheritableFields = ["instance.name", "instance.is_base", "instance.base_id"]; + /** Fields that are never inherited from a template, ports would clash on a shared host */ + static uninheritableFields = [ + "instance.name", "instance.is_template", "instance.template_id", + "factorio.game_port", "factorio.rcon_port", "factorio.lua_udp_port", + ]; - /** True if the field can take its value from a base instance */ + /** True if the field can take its value from a template */ static isInheritable(name: string) { const def = this.fieldDefinitions[name as keyof InstanceConfigFields]; return Boolean(def) @@ -554,9 +557,21 @@ export class InstanceConfig extends classes.Config { && !this.uninheritableFields.includes(name); } - /** True if the field currently takes its value from the base instance */ - isInherited(name: string) { - return this.get("instance.base_id") !== null && Boolean(this.get("instance.inherited_fields")[name]); + /** + * True if the field, or the given property of an object field, takes its value from the template + * + * @param name - Name of field to check. + * @param prop - Property of an object field to check. + */ + isInherited(name: string, prop?: string) { + if (this.get("instance.template_id") === null) { + return false; + } + const entry = this.get("instance.inherited_fields")[name]; + if (entry instanceof Array) { + return prop === undefined || !entry.includes(prop); + } + return Boolean(entry); } static fieldDefinitions: classes.ConfigDefs = { @@ -586,24 +601,26 @@ export class InstanceConfig extends classes.Config { type: "boolean", initialValue: false, }, - "instance.is_base": { + "instance.is_template": { description: - "Mark this instance as a base other instances can inherit config from. " + - "Bases cannot be assigned to a host or started.", + "Mark this instance as a template other instances can inherit config from. " + + "Templates cannot be assigned to a host or started.", access: ["controller", "control"], type: "boolean", initialValue: false, }, - "instance.base_id": { - title: "Base Instance", - description: "ID of the base instance to inherit config values from", + "instance.template_id": { + title: "Template", + description: "ID of the template to inherit config values from", access: ["controller", "control"], + inputComponent: "instance_template", type: "number", optional: true, // Checked by the controller }, "instance.inherited_fields": { - description: "Fields whose value comes from the base instance", + description: + "Fields whose value comes from the template, for object fields the properties set on this instance", access: ["controller", "control"], readonly: ["controller"], type: "object", diff --git a/packages/lib/src/data/InstanceDetails.ts b/packages/lib/src/data/InstanceDetails.ts index ce008c9b..cad0761d 100644 --- a/packages/lib/src/data/InstanceDetails.ts +++ b/packages/lib/src/data/InstanceDetails.ts @@ -42,7 +42,8 @@ export default class InstanceDetails { /** Millisecond Unix timestamp this entry was last updated at */ public updatedAtMs = 0, public excludeFromStartAll = false, - public isBase = false, + public isTemplate = false, + public templateId?: number, ) {} get isDeleted() { @@ -62,7 +63,8 @@ export default class InstanceDetails { "startedAtMs": Type.Optional(Type.Number()), "updatedAtMs": Type.Optional(Type.Number()), "excludeFromStartAll": Type.Optional(Type.Boolean()), - "isBase": Type.Optional(Type.Boolean()), + "isTemplate": Type.Optional(Type.Boolean()), + "templateId": Type.Optional(Type.Integer()), }); static fromJSON(json: Static) { @@ -76,7 +78,8 @@ export default class InstanceDetails { json.startedAtMs, json.updatedAtMs, json.excludeFromStartAll, - json.isBase, + json.isTemplate, + json.templateId, ); } @@ -105,8 +108,11 @@ export default class InstanceDetails { if (this.excludeFromStartAll !== false) { json.excludeFromStartAll = this.excludeFromStartAll; } - if (this.isBase !== false) { - json.isBase = this.isBase; + if (this.isTemplate !== false) { + json.isTemplate = this.isTemplate; + } + if (this.templateId !== undefined) { + json.templateId = this.templateId; } return json; diff --git a/packages/lib/src/data/messages_instance.ts b/packages/lib/src/data/messages_instance.ts index 5cde39b7..77500fab 100644 --- a/packages/lib/src/data/messages_instance.ts +++ b/packages/lib/src/data/messages_instance.ts @@ -186,15 +186,17 @@ export class InstanceConfigInheritFieldRequest { constructor( public instanceId: number, public field: string, + public prop?: string, ) { } static jsonSchema = Type.Object({ "instanceId": Type.Integer(), "field": Type.String(), + "prop": Type.Optional(Type.String()), }); static fromJSON(json: Static) { - return new this(json.instanceId, json.field); + return new this(json.instanceId, json.field, json.prop); } } diff --git a/packages/web_ui/src/bootstrap.tsx b/packages/web_ui/src/bootstrap.tsx index d3e9f739..b87d4852 100644 --- a/packages/web_ui/src/bootstrap.tsx +++ b/packages/web_ui/src/bootstrap.tsx @@ -8,6 +8,7 @@ import * as lib from "@clusterio/lib"; import App from "./components/App"; import InputRole from "./components/InputRole"; import InputModPack from "./components/InputModPack"; +import { InputInstanceTemplate } from "./components/InputInstance"; import { InputTargetVersion, InputPartialVersion, InputFullVersion } from "./components/InputVersion"; import { Control, ControlConnector } from "./util/websocket"; import { loadedPluginSetKey } from "./util/pluginSet"; @@ -148,6 +149,7 @@ async function inputComponentsFromHooks(control: Control) { "target_version": InputTargetVersion, "mod_pack": InputModPack, "role": InputRole, + "instance_template": InputInstanceTemplate, }]); const iterable = result.map( diff --git a/packages/web_ui/src/components/BaseConfigTree.tsx b/packages/web_ui/src/components/BaseConfigTree.tsx index 058487ce..489bdf7e 100644 --- a/packages/web_ui/src/components/BaseConfigTree.tsx +++ b/packages/web_ui/src/components/BaseConfigTree.tsx @@ -1,8 +1,11 @@ import React, { useContext, useEffect, useState } from "react"; import { - Button, Card, Checkbox, Form, FormInstance, Input, InputNumber, Select, Space, Spin, Tooltip, Tree, Typography, + Button, Card, Checkbox, Form, FormInstance, Input, InputNumber, Popconfirm, Select, Space, Spin, Tooltip, Tree, + Typography, } from "antd"; +import LockOutlined from "@ant-design/icons/LockOutlined"; import ReloadOutlined from "@ant-design/icons/ReloadOutlined"; +import UnlockOutlined from "@ant-design/icons/UnlockOutlined"; import * as lib from "@clusterio/lib"; @@ -65,8 +68,24 @@ function renderInput(inputComponents: Map, def: lib.Fiel return `Unknown type ${def.type}`; } +/** Lets fields follow the value of another config, inherited values are locked until unlocked */ +export type ConfigInheritance = { + canInherit: (config: lib.Config, field: string) => boolean; + isInherited: (config: lib.Config, field: string, prop?: string) => boolean; + inherit: (field: string, prop?: string) => Promise; + notice?: (config: lib.Config) => React.ReactNode; +}; + +type LockState = { + inheritance?: ConfigInheritance; + unlocked: Set; + setUnlocked: (unlocked: Set) => void; + inherit: (field: string, prop?: string) => void; +}; + type BaseConfigTreeProps = { ConfigClass: typeof lib.ControllerConfig | typeof lib.HostConfig | typeof lib.InstanceConfig; + inheritance?: ConfigInheritance; retrieveConfig: () => Promise; setConfig: (fields: Record>) => Promise; id?: number; @@ -79,6 +98,7 @@ export default function BaseConfigTree(props: BaseConfigTreeProps) { let [changedFields, setChangedFields] = useState>(new Set()); let [errorFields, setErrorFields] = useState(new Map()); let [applying, setApplying] = useState(false); + let [unlocked, setUnlocked] = useState>(new Set()); const available = props.available ?? true; const control = useContext(ControlContext); @@ -136,6 +156,17 @@ export default function BaseConfigTree(props: BaseConfigTreeProps) { } } + function inherit(field: string, prop?: string) { + props.inheritance!.inherit(field, prop).then(async () => { + const newConfig = await updateConfig(); + const values = getInitialValues(newConfig, props); + const keys = Object.keys(values).filter(key => key === field || key.startsWith(`${field}.`)); + form.resetFields(keys); + form.setFieldsValue(Object.fromEntries(keys.map(key => [key, values[key]]))); + setChangedFields(fields => new Set([...fields].filter(key => !keys.includes(key)))); + }).catch(err => notify(err.message, "error")); + } + const [propsMap, treeData] = computeTreeData( control, form, @@ -145,6 +176,7 @@ export default function BaseConfigTree(props: BaseConfigTreeProps) { setConfig, props.ConfigClass, onValuesChange, + { inheritance: props.inheritance, unlocked, setUnlocked, inherit }, ); return <> @@ -179,14 +211,21 @@ export default function BaseConfigTree(props: BaseConfigTreeProps) { } } + let applied = false; try { await props.setConfig(fields); setChangedFields(new Set()); + setUnlocked(new Set()); + applied = true; } catch (err: any) { notify(err.message, "error"); } - await updateConfig(); + const newConfig = await updateConfig(); + if (applied) { + // Linking a template can change other values + form.setFieldsValue(getInitialValues(newConfig, props)); + } setApplying(false); if (props.onApply) { @@ -202,6 +241,7 @@ export default function BaseConfigTree(props: BaseConfigTreeProps) { onClick={() => { form.resetFields(); setChangedFields(new Set()); + setUnlocked(new Set()); setErrorFields(new Map()); }} > @@ -210,6 +250,7 @@ export default function BaseConfigTree(props: BaseConfigTreeProps) { + {props.inheritance?.notice?.(config)}
) => void, ConfigClass: BaseConfigTreeProps["ConfigClass"], onValuesChange: (changedValues: object) => void, + lock: LockState, ) { let restartTip = + } + ; + } + return { account.hasAnyPermission(...InstanceControlButtonPermissions) @@ -190,6 +236,12 @@ function InstanceButtons(props: { instance: lib.InstanceDetails }) { Connect via Steam {account.hasPermission("core.instance.load_scenario") && } + {canCreate && } {account.hasAnyPermission( "core.instance.export_data", "core.instance.extract_players", @@ -224,11 +276,16 @@ export default function InstanceViewPage() { return } + status={instance.isTemplate + ? Template + : } extra={} /> - + {instance.isTemplate + ? + : } + {!instance.isTemplate && <> {account.hasPermission("core.user.list") && } @@ -275,6 +332,7 @@ export default function InstanceViewPage() { account.hasPermission("core.instance.send_rcon") && } + } {account.hasPermission("core.instance.get_config") && } diff --git a/packages/web_ui/src/components/InstancesPage.tsx b/packages/web_ui/src/components/InstancesPage.tsx index ed1498a2..9e6a7d54 100644 --- a/packages/web_ui/src/components/InstancesPage.tsx +++ b/packages/web_ui/src/components/InstancesPage.tsx @@ -14,12 +14,85 @@ import { useInstanceConfig, useInstances } from "../model/instance"; import { useSaves } from "../model/saves"; import InstanceList from "./InstanceList"; import { notifyErrorHandler } from "../util/notify"; +import type { Control } from "../util/websocket"; type SaveMode = "none" | "copy" | "save_and_copy"; const strcmp = new Intl.Collator(undefined, { numeric: true, sensitivity: "base" }).compare; -function CreateInstanceButton(props: { instances: ReturnType[0] }) { +function ConfigSourceFields(props: { + instances: ReturnType[0]; + templateId?: number | null; + source?: Readonly; + asTemplate: boolean; +}) { + const templates = [...props.instances.values()].filter(i => i.isTemplate); + const hasTemplate = typeof props.templateId === "number"; + return <> + {(templates.length > 0 || hasTemplate) && + + !i.isTemplate), + ].map(i => ({ value: i.id, label: i.name }))} + /> + + + {props.asTemplate && props.source && + Make {props.source.name} inherit from the new template + } + ; +} + +async function linkToTemplate(control: Control, source: lib.InstanceDetails | undefined, templateId: number) { + if (source) { + await control.send(new lib.InstanceConfigSetFieldRequest( + source.id, "instance.template_id", String(templateId), + )); + } +} + +function setTemplateFields( + config: lib.InstanceConfig, + fields: (keyof lib.InstanceConfigFields)[], + templateId: unknown, + asTemplate: boolean, +) { + if (typeof templateId === "number") { + config.set("instance.template_id", templateId); + fields.push("instance.template_id"); + } + if (asTemplate) { + config.set("instance.is_template", true); + fields.push("instance.is_template"); + } +} + +type CreateInstanceInitial = { + instanceClone?: number; + template?: number; + asTemplate?: boolean; + linkSource?: boolean; +}; + +export function CreateInstanceButton({ buttonText = "Create", buttonType = "primary", ...props }: { + instances: ReturnType[0]; + initial?: CreateInstanceInitial; + buttonText?: string; + buttonType?: "primary" | "default"; +}) { let control = useContext(ControlContext); let account = useAccount(); let navigate = useNavigate(); @@ -31,9 +104,11 @@ function CreateInstanceButton(props: { instances: ReturnType= 0 ? props.instances.get(cloneId) : undefined; const sourceRunning = source?.status === "running"; const activeSave = source ? [...saves.values()].find( @@ -42,6 +117,7 @@ function CreateInstanceButton(props: { instances: ReturnType | undefined; const sourceServerName = typeof sourceSettings?.name === "string" ? sourceSettings.name : undefined; + const copySettings = sourceSettings !== undefined && !asTemplate; useEffect(() => { if (open && sourceServerName !== undefined) { @@ -68,7 +144,8 @@ function CreateInstanceButton(props: { instances: ReturnType + >{buttonText} - {canAssign && + + + Create as template + + + {canAssign && !asTemplate && } - - - @@ -198,6 +285,8 @@ export default function InstancesPage() { let control = useContext(ControlContext); let account = useAccount(); let [instances] = useInstances(); + const plainInstances = new Map([...instances].filter(([, instance]) => !instance.isTemplate)); + const templates = new Map([...instances].filter(([, instance]) => instance.isTemplate)); return } /> - + + {templates.size > 0 && <> + Templates + + } ; } diff --git a/test/controller/ControlConnection.test.js b/test/controller/ControlConnection.test.js index e4d9efde..845c56fb 100644 --- a/test/controller/ControlConnection.test.js +++ b/test/controller/ControlConnection.test.js @@ -157,22 +157,22 @@ describe("controller/src/ControlConnection", function() { assert.equal(clone.config.get("instance.assigned_host"), null); }); - it("creates an instance inheriting from a base", async function() { - await create({ "instance.id": 4001, "instance.name": "base", "instance.is_base": true }); + it("creates an instance inheriting from a template", async function() { + await create({ "instance.id": 4001, "instance.name": "template", "instance.is_template": true }); mockController.instances.getMutable(4001).config.set("factorio.version", "2.0"); - await create({ "instance.id": 4002, "instance.name": "child", "instance.base_id": 4001 }); + await create({ "instance.id": 4002, "instance.name": "child", "instance.template_id": 4001 }); const child = mockController.instances.get(4002); assert.equal(child.config.get("factorio.version"), "2.0"); assert.equal(child.config.isInherited("factorio.version"), true); assert.equal(child.config.isInherited("instance.name"), false); }); - it("rejects a base that is not a base", async function() { + it("rejects a template that is not a template", async function() { await create({ "instance.id": 4001, "instance.name": "plain" }); await assert.rejects( - create({ "instance.id": 4002, "instance.name": "child", "instance.base_id": 4001 }), - new lib.ValidationError("Instance plain is not a base"), + create({ "instance.id": 4002, "instance.name": "child", "instance.template_id": 4001 }), + new lib.ValidationError("Instance plain is not a template"), ); assert.equal(mockController.instances.has(4002), false); }); @@ -192,14 +192,16 @@ describe("controller/src/ControlConnection", function() { checkStagedInheritance: ControlConnection.prototype.checkStagedInheritance, }; for (const [id, fields] of [ - [1, { "instance.is_base": true, "factorio.version": "2.0" }], - [2, { "instance.base_id": 1 }], + [1, { "instance.is_template": true, "factorio.version": "2.0" }], + [2, { "instance.template_id": 1 }], ]) { const config = new lib.InstanceConfig("controller", { "instance.id": id, "instance.name": `i${id}`, ...fields, }); - if (fields["instance.base_id"]) { - mockController.instances.applyBase(config, ["instance.id", "instance.name", "instance.base_id"]); + if (fields["instance.template_id"]) { + mockController.instances.applyTemplate( + config, ["instance.id", "instance.name", "instance.template_id"], + ); } await mockController.instances.createInstance(config); } @@ -208,6 +210,11 @@ describe("controller/src/ControlConnection", function() { function call(handler, request) { return ControlConnection.prototype[handler].call(connection, request); } + async function create(config, cloneFromId) { + await ControlConnection.prototype.handleInstanceCreateRequest.call( + connection, new lib.InstanceCreateRequest(config, cloneFromId), + ); + } const setField = (...args) => new lib.InstanceConfigSetFieldRequest(...args); const setFields = (...args) => new lib.InstanceConfigSetRequest(...args); @@ -221,28 +228,58 @@ describe("controller/src/ControlConnection", function() { assert.equal(mockController.instances.get(2).config.isInherited("factorio.version"), false); }); - it("stops inheriting a field changed through set-prop", async function() { + it("stops inheriting a property set through set-prop", async function() { await call( "handleInstanceConfigSetPropRequest", new lib.InstanceConfigSetPropRequest(2, "factorio.settings", "max_players", 0), ); - assert.equal(mockController.instances.get(2).config.isInherited("factorio.settings"), false); + const config = mockController.instances.get(2).config; + assert.equal(config.isInherited("factorio.settings", "max_players"), false); + assert.equal(config.isInherited("factorio.settings", "auto_pause"), true); }); - it("rejects base_id pointing at a non-base", async function() { + it("rejects template_id pointing at a non-template", async function() { await assert.rejects( - call("handleInstanceConfigSetFieldRequest", setField(1, "instance.base_id", "2")), - new lib.ValidationError("Instance i2 is not a base"), + call("handleInstanceConfigSetFieldRequest", setField(1, "instance.template_id", "2")), + new lib.ValidationError("Instance i2 is not a template"), ); - assert.equal(mockController.instances.get(1).config.get("instance.base_id"), null); + assert.equal(mockController.instances.get(1).config.get("instance.template_id"), null); }); - it("rejects unmarking a base with inheritors through set request", async function() { + it("rejects unmarking a template with inheritors through set request", async function() { await assert.rejects( - call("handleInstanceConfigSetRequest", setFields(1, { "instance.is_base": "false" })), - new lib.ConflictError("Instance is the base of 1 other instance(s)"), + call("handleInstanceConfigSetRequest", setFields(1, { "instance.is_template": "false" })), + new lib.ConflictError("Instance is the template of 1 other instance(s)"), ); - assert.equal(mockController.instances.get(1).config.get("instance.is_base"), true); + assert.equal(mockController.instances.get(1).config.get("instance.is_template"), true); + }); + + it("stops inheriting only the properties set through set request", async function() { + await call("handleInstanceConfigSetRequest", setFields(2, { "factorio.settings": { max_players: 3 } })); + const config = mockController.instances.get(2).config; + assert.equal(config.isInherited("factorio.settings", "max_players"), false); + assert.equal(config.isInherited("factorio.settings", "auto_pause"), true); + }); + + it("inherits a property again", async function() { + await call("handleInstanceConfigSetPropRequest", new lib.InstanceConfigSetPropRequest( + 2, "factorio.settings", "max_players", 3, + )); + await call( + "handleInstanceConfigInheritFieldRequest", + new lib.InstanceConfigInheritFieldRequest(2, "factorio.settings", "max_players"), + ); + const config = mockController.instances.get(2).config; + assert.equal(config.get("factorio.settings").max_players, 0); + assert.equal(config.isInherited("factorio.settings", "max_players"), true); + }); + + it("creates a template from an existing instance", async function() { + await create({ "instance.id": 3, "instance.name": "copy", "instance.is_template": true }, 2); + const config = mockController.instances.get(3).config; + assert.equal(config.get("instance.is_template"), true); + assert.equal(config.get("instance.template_id"), 1); + assert.equal(config.get("factorio.version"), "2.0"); }); it("inherits a field again", async function() { diff --git a/test/controller/InstanceManager.test.js b/test/controller/InstanceManager.test.js index fc6e262b..26e26385 100644 --- a/test/controller/InstanceManager.test.js +++ b/test/controller/InstanceManager.test.js @@ -396,67 +396,94 @@ describe("controller/InstanceManager", function () { describe("inheritance", function () { /** @type {InstanceRecord} */ - let base; + let template; /** @type {InstanceRecord} */ let child; let updated; - async function createInheritor(id, baseId, fields = {}) { + async function createInheritor(id, templateId, fields = {}) { const config = new lib.InstanceConfig("controller", { - "instance.id": id, "instance.name": `Instance ${id}`, "instance.base_id": baseId, ...fields, + "instance.id": id, "instance.name": `Instance ${id}`, "instance.template_id": templateId, ...fields, }); - instances.applyBase(config, ["instance.id", "instance.name", "instance.base_id", ...Object.keys(fields)]); + instances.applyTemplate( + config, ["instance.id", "instance.name", "instance.template_id", ...Object.keys(fields)] + ); return await instances.createInstance(config); } beforeEach(async function () { - base = await instances.createInstance(new lib.InstanceConfig("controller", { + template = await instances.createInstance(new lib.InstanceConfig("controller", { "instance.id": 10, - "instance.name": "Base", - "instance.is_base": true, + "instance.name": "Template", + "instance.is_template": true, "factorio.version": "2.0", - "factorio.settings": { name: "Base server" }, + "factorio.game_port": 34190, + "factorio.settings": { name: "Template server", max_players: 5 }, })); child = await createInheritor(11, 10, { "factorio.game_port": 34197 }); updated = []; controller.instanceConfigUpdated = async (inst) => { updated.push(inst.id); }; }); - describe(".applyBase()", function () { - it("should copy base values and mark them inherited", function () { + describe(".applyTemplate()", function () { + it("should copy template values and mark them inherited", function () { assert.equal(child.config.get("factorio.version"), "2.0"); assert.equal(child.config.isInherited("factorio.version"), true); assert.equal(child.config.get("factorio.game_port"), 34197); assert.equal(child.config.isInherited("factorio.game_port"), false); assert.equal(child.config.isInherited("instance.name"), false); }); - it("should keep inherited factorio.settings in createInstance", function () { - assert.deepEqual(child.config.get("factorio.settings"), base.config.get("factorio.settings")); - assert.notEqual(child.config.get("factorio.settings"), base.config.get("factorio.settings")); + it("should never inherit ports", function () { + const config = new lib.InstanceConfig("controller", { "instance.id": 12, "instance.template_id": 10 }); + instances.applyTemplate(config, ["instance.id", "instance.template_id"]); + assert.equal(config.get("factorio.game_port"), null); + assert.equal(config.isInherited("factorio.game_port"), false); + }); + it("should inherit factorio.settings per property with its own server name", function () { + const settings = child.config.get("factorio.settings"); + assert.equal(settings.max_players, 5); + assert.equal(settings.name, "TestController - Instance 11"); + assert.equal(child.config.isInherited("factorio.settings", "max_players"), true); + assert.equal(child.config.isInherited("factorio.settings", "name"), false); + }); + it("should keep properties given by the creator as overrides", async function () { + const other = await createInheritor(12, 10, { "factorio.settings": { max_players: 8 } }); + assert.equal(other.config.get("factorio.settings").max_players, 8); + assert.equal(other.config.isInherited("factorio.settings", "max_players"), false); + assert.equal(other.config.isInherited("factorio.settings", "auto_pause"), true); }); }); - describe("base changes", function () { + describe("template changes", function () { it("should propagate to inherited fields and send the config to the host", function () { - base.config.set("factorio.version", "2.1"); + template.config.set("factorio.version", "2.1"); assert.equal(child.config.get("factorio.version"), "2.1"); assert.equal(child.config.isInherited("factorio.version"), true); assert.deepEqual(updated, [11]); }); it("should mark the inheritor as requiring restart", function () { child.config.restartRequired = false; - base.config.set("factorio.version", "2.1"); + template.config.set("factorio.version", "2.1"); assert.equal(child.config.restartRequired, true); }); it("should not touch fields the inheritor has set", function () { - base.config.set("factorio.game_port", 1234); - assert.equal(child.config.get("factorio.game_port"), 34197); - assert.deepEqual(updated, []); + child.config.set("factorio.version", "2.1"); + template.config.set("factorio.version", "2.2"); + assert.equal(child.config.get("factorio.version"), "2.1"); + }); + it("should merge object properties and keep overrides", function () { + template.config.setProp("factorio.settings", "max_players", 10); + template.config.setProp("factorio.settings", "name", "Renamed"); + template.config.setProp("factorio.settings", "auto_pause", true); + const settings = child.config.get("factorio.settings"); + assert.equal(settings.max_players, 10); + assert.equal(settings.auto_pause, true); + assert.equal(settings.name, "TestController - Instance 11"); }); it("should propagate through several levels", async function () { - child.config.set("instance.is_base", true); + child.config.set("instance.is_template", true); const grandchild = await createInheritor(12, 11); - base.config.setProp("factorio.settings", "max_players", 10); + template.config.setProp("factorio.settings", "max_players", 10); assert.equal(grandchild.config.get("factorio.settings").max_players, 10); assert.deepEqual(updated.sort(), [11, 12]); }); @@ -466,26 +493,47 @@ describe("controller/InstanceManager", function () { it("should stop inheriting a field that is changed", function () { child.config.set("factorio.version", "2.1"); assert.equal(child.config.isInherited("factorio.version"), false); - base.config.set("factorio.version", "2.0.1"); - assert.equal(child.config.get("factorio.version"), "2.1"); }); - it("should stop inheriting with overrideFields() even if the value is unchanged", function () { - instances.overrideFields(child, ["factorio.version", "factorio.game_port"]); + it("should only override the properties changed in an object", function () { + child.config.setProp("factorio.settings", "max_players", 3); + assert.equal(child.config.isInherited("factorio.settings", "max_players"), false); + assert.equal(child.config.isInherited("factorio.settings", "auto_pause"), true); + }); + it("should restore the template value of a removed property", function () { + child.config.setProp("factorio.settings", "max_players", 3); + child.config.setProp("factorio.settings", "max_players"); + assert.equal(child.config.get("factorio.settings").max_players, 5); + assert.equal(child.config.isInherited("factorio.settings", "max_players"), true); + }); + it("should stop inheriting with overrideField() even if the value is unchanged", function () { + instances.overrideField(child, "factorio.version"); + instances.overrideField(child, "factorio.settings", ["auto_pause"]); assert.equal(child.config.isInherited("factorio.version"), false); + assert.equal(child.config.isInherited("factorio.settings", "auto_pause"), false); }); }); describe(".inheritField()", function () { - it("should restore the base value", function () { + it("should restore the template value", function () { child.config.set("factorio.version", "2.1"); instances.inheritField(11, "factorio.version"); assert.equal(child.config.get("factorio.version"), "2.0"); assert.equal(child.config.isInherited("factorio.version"), true); }); - it("should throw if the instance has no base", function () { + it("should restore a single property", function () { + child.config.setProp("factorio.settings", "max_players", 3); + instances.inheritField(11, "factorio.settings", "max_players"); + assert.equal(child.config.get("factorio.settings").max_players, 5); + assert.equal(child.config.get("factorio.settings").name, "TestController - Instance 11"); + }); + it("should restore all properties", function () { + instances.inheritField(11, "factorio.settings"); + assert.equal(child.config.get("factorio.settings").name, "Template server"); + }); + it("should throw if the instance has no template", function () { assert.throws( () => instances.inheritField(1, "factorio.version"), - new lib.ValidationError("Instance does not have a base"), + new lib.ValidationError("Instance does not have a template"), ); }); it("should throw for fields that cannot be inherited", function () { @@ -493,79 +541,102 @@ describe("controller/InstanceManager", function () { () => instances.inheritField(11, "instance.name"), new lib.ValidationError("Field instance.name cannot be inherited"), ); + assert.throws( + () => instances.inheritField(11, "factorio.rcon_port"), + new lib.ValidationError("Field factorio.rcon_port cannot be inherited"), + ); }); }); - describe("base_id changes", function () { - it("should inherit fields equal to the new base", function () { + describe("template_id changes", function () { + it("should inherit fields equal to the template or the default", function () { instance.config.set("factorio.version", "2.0"); - instance.config.set("instance.base_id", 10); + instance.config.set("factorio.verbose_logging", true); + template.config.set("instance.auto_start", true); + instance.config.set("instance.template_id", 10); assert.equal(instance.config.isInherited("factorio.version"), true); - assert.equal(instance.config.isInherited("factorio.settings"), false); + assert.equal(instance.config.isInherited("instance.auto_start"), true); + assert.equal(instance.config.get("instance.auto_start"), true); + assert.equal(instance.config.isInherited("factorio.verbose_logging"), false); }); - it("should clear inherited fields when the base is removed", function () { - child.config.set("instance.base_id", null); + it("should inherit object properties equal to the template or the default", function () { + instance.config.setProp("factorio.settings", "max_players", 0); + instance.config.setProp("factorio.settings", "game_password", "secret"); + instance.config.set("instance.template_id", 10); + const settings = instance.config.get("factorio.settings"); + assert.equal(settings.max_players, 5); + assert.equal(settings.game_password, "secret"); + assert.equal(instance.config.isInherited("factorio.settings", "game_password"), false); + assert.equal(instance.config.isInherited("factorio.settings", "max_players"), true); + }); + it("should keep the server name of the instance", function () { + instance.config.setProp("factorio.settings", "name", "Template server"); + instance.config.set("instance.template_id", 10); + assert.equal(instance.config.isInherited("factorio.settings", "name"), false); + }); + it("should clear inherited fields when the template is removed", function () { + child.config.set("instance.template_id", null); assert.deepEqual(child.config.get("instance.inherited_fields"), {}); assert.equal(child.config.get("factorio.version"), "2.0"); }); }); describe(".checkInheritance()", function () { - it("should accept a valid base", function () { + it("should accept a valid template", function () { instances.checkInheritance(instance.config, false, 10); }); - it("should reject a missing base", function () { + it("should reject a missing template", function () { assert.throws( () => instances.checkInheritance(instance.config, false, 99), new lib.NotFoundError("Instance with ID 99 does not exist"), ); }); - it("should reject an instance that is not a base", function () { + it("should reject an instance that is not a template", function () { assert.throws( () => instances.checkInheritance(instance.config, false, 11), - new lib.ValidationError("Instance Instance 11 is not a base"), + new lib.ValidationError("Instance Instance 11 is not a template"), ); }); it("should reject cycles", function () { - child.config.set("instance.is_base", true); + child.config.set("instance.is_template", true); assert.throws( - () => instances.checkInheritance(base.config, true, 11), - new lib.ValidationError("Base instances cannot inherit from themselves"), + () => instances.checkInheritance(template.config, true, 11), + new lib.ValidationError("Templates cannot inherit from themselves"), ); }); - it("should reject making an assigned instance a base", function () { + it("should reject making an assigned instance a template", function () { instance.config.set("instance.assigned_host", 5); assert.throws( () => instances.checkInheritance(instance.config, true, null), - new lib.ConflictError("Instance must be unassigned before it can be made a base"), + new lib.ConflictError("Instance must be unassigned before it can be made a template"), ); }); - it("should reject unmarking a base that has inheritors", function () { + it("should reject unmarking a template that has inheritors", function () { assert.throws( - () => instances.checkInheritance(base.config, false, null), - new lib.ConflictError("Instance is the base of 1 other instance(s)"), + () => instances.checkInheritance(template.config, false, null), + new lib.ConflictError("Instance is the template of 1 other instance(s)"), ); }); }); - it("should not assign a base to a host", async function () { + it("should not assign a template to a host", async function () { controller.wsServer.hostConnections.set(5, { send: async () => {}, connector: { closing: false } }); await assert.rejects( instances.assignInstance(10, 5), - new lib.ValidationError("Base instances cannot be assigned to a host"), + new lib.ValidationError("Templates cannot be assigned to a host"), ); }); - it("should not delete a base that has inheritors", async function () { + it("should not delete a template that has inheritors", async function () { await assert.rejects( instances.deleteInstance(10), - new lib.ConflictError("Instance is the base of 1 other instance(s)"), + new lib.ConflictError("Instance is the template of 1 other instance(s)"), ); }); - it("should report bases in instance details", function () { - assert.equal(base.toInstanceDetails().isBase, true); - assert.equal(child.toInstanceDetails().isBase, false); + it("should report templates in instance details", function () { + assert.equal(template.toInstanceDetails().isTemplate, true); + assert.equal(child.toInstanceDetails().isTemplate, false); }); }); }); diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index d25fc29b..3f0752df 100644 --- a/test/integration/clusterio.js +++ b/test/integration/clusterio.js @@ -971,42 +971,53 @@ describe("Integration of Clusterio", function() { }); }); - describe("instance base inheritance", function() { + describe("instance template inheritance", function() { after(async function() { - for (const id of [4402, 4401, 4400]) { + for (const id of [4403, 4402, 4401, 4400]) { await getControl().send(new lib.InstanceDeleteRequest(id)).catch(() => {}); } }); - it("creates a base and an instance inheriting from it", async function() { - await execCtl("instance create inheritBase --id 4400 --is-base"); - await execCtl("instance config set inheritBase factorio.version 2.0"); - await execCtl("instance create inheritChild --id 4401 --base inheritBase"); + it("creates a template and an instance inheriting from it", async function() { + await execCtl("instance create inheritTemplate --id 4400 --is-template"); + await execCtl("instance config set inheritTemplate factorio.version 2.0"); + await execCtl("instance create inheritChild --id 4401 --template inheritTemplate"); const config = await getControl().send(new lib.InstanceConfigGetRequest(4401)); - assert.equal(config["instance.base_id"], 4400); + assert.equal(config["instance.template_id"], 4400); assert.equal(config["factorio.version"], "2.0"); assert.equal(config["instance.inherited_fields"]["factorio.version"], true); - assert.equal((await getInstances()).get(4400).isBase, true); + assert.equal((await getInstances()).get(4400).isTemplate, true); }); - it("propagates base changes", async function() { - await execCtl("instance config set inheritBase factorio.version 2.1"); + it("propagates template changes per property", async function() { + await execCtl("instance config set inheritTemplate factorio.version 2.1"); + await execCtl("instance config set-prop inheritChild factorio.settings max_players 3"); + await execCtl("instance config set-prop inheritTemplate factorio.settings max_players 7"); + await execCtl("instance config set-prop inheritTemplate factorio.settings afk_autokick_interval 9"); const config = await getControl().send(new lib.InstanceConfigGetRequest(4401)); assert.equal(config["factorio.version"], "2.1"); + assert.equal(config["factorio.settings"].max_players, 3); + assert.equal(config["factorio.settings"].afk_autokick_interval, 9); }); - it("rejects inheriting from an instance that is not a base", async function() { + it("creates a template from an existing instance", async function() { + await execCtl("instance create inheritCopy --id 4403 --is-template --from 4401"); + const config = await getControl().send(new lib.InstanceConfigGetRequest(4403)); + assert.equal(config["instance.is_template"], true); + assert.equal(config["factorio.settings"].max_players, 3); + }); + it("rejects inheriting from an instance that is not a template", async function() { await assert.rejects( - execCtl("instance create inheritBad --id 4402 --base inheritChild"), - new lib.ValidationError("Instance inheritChild is not a base"), + execCtl("instance create inheritBad --id 4402 --template inheritChild"), + new lib.ValidationError("Instance inheritChild is not a template"), ); }); - it("rejects assigning a base to a host", async function() { + it("rejects assigning a template to a host", async function() { await assert.rejects( - execCtl("instance assign inheritBase 4"), - new lib.ValidationError("Base instances cannot be assigned to a host"), + execCtl("instance assign inheritTemplate 4"), + new lib.ValidationError("Templates cannot be assigned to a host"), ); }); - describe("instance list --bases", function() { + describe("instance list --templates", function() { it("runs", async function() { - await execCtl("instance list --bases"); + await execCtl("instance list --templates"); }); }); describe("instance config list", function() { @@ -1021,9 +1032,11 @@ describe("Integration of Clusterio", function() { assert.equal(config["instance.inherited_fields"]["factorio.version"], undefined); await execCtl("instance config inherit inheritChild factorio.version"); + await execCtl("instance config inherit inheritChild factorio.settings max_players"); config = await getControl().send(new lib.InstanceConfigGetRequest(4401)); assert.equal(config["factorio.version"], "2.1"); assert.equal(config["instance.inherited_fields"]["factorio.version"], true); + assert.equal(config["factorio.settings"].max_players, 7); }); }); }); diff --git a/test/lib/config/definitions.js b/test/lib/config/definitions.js index 33cc097e..632e10b1 100644 --- a/test/lib/config/definitions.js +++ b/test/lib/config/definitions.js @@ -12,28 +12,38 @@ describe("lib/config/definitions", function() { }); it("should reject identity, hidden, host only and unknown fields", function() { for (const name of [ - "instance.name", "instance.id", "instance.assigned_host", "instance.is_base", - "instance.base_id", "instance.inherited_fields", "factorio.host_assigned_game_port", "no.field", + "instance.name", "instance.id", "instance.assigned_host", "instance.is_template", + "instance.template_id", "instance.inherited_fields", "factorio.host_assigned_game_port", + "factorio.game_port", "factorio.rcon_port", "factorio.lua_udp_port", "no.field", ]) { assert.equal(lib.InstanceConfig.isInheritable(name), false, name); } }); }); describe(".isInherited()", function() { - it("should only report fields as inherited when a base is set", function() { + it("should only report fields as inherited when a template is set", function() { const config = new lib.InstanceConfig("controller", { "instance.inherited_fields": { "factorio.version": true }, }); assert.equal(config.isInherited("factorio.version"), false); - config.set("instance.base_id", 1); + config.set("instance.template_id", 1); assert.equal(config.isInherited("factorio.version"), true); assert.equal(config.isInherited("factorio.game_port"), false); }); + it("should report object properties not overridden as inherited", function() { + const config = new lib.InstanceConfig("controller", { + "instance.template_id": 1, + "instance.inherited_fields": { "factorio.settings": ["name"] }, + }); + assert.equal(config.isInherited("factorio.settings"), true); + assert.equal(config.isInherited("factorio.settings", "max_players"), true); + assert.equal(config.isInherited("factorio.settings", "name"), false); + }); }); it("should not send inheritance fields to the host", function() { - const config = new lib.InstanceConfig("controller", { "instance.base_id": 1 }); + const config = new lib.InstanceConfig("controller", { "instance.template_id": 1 }); const remote = config.toRemote("host"); - for (const name of ["instance.is_base", "instance.base_id", "instance.inherited_fields"]) { + for (const name of ["instance.is_template", "instance.template_id", "instance.inherited_fields"]) { assert(!Object.hasOwn(remote, name), name); } }); diff --git a/test/lib/data/InstanceDetails.test.js b/test/lib/data/InstanceDetails.test.js index 95058653..2cec180c 100644 --- a/test/lib/data/InstanceDetails.test.js +++ b/test/lib/data/InstanceDetails.test.js @@ -16,6 +16,8 @@ describe("lib/data/InstanceDetails", function () { [undefined, 1234], // startedAtMs [undefined, 9999], // updatedAtMs [undefined, true], // excludeFromStartAll + [undefined, true], // isTemplate + [undefined, 7], // templateId )); });