aperx/factorio-clusterio
1
0 Fork 0
mirror of https://github.com/clusterio/clusterio.git synced 2026-11-11 15:04:52 +09:00
Code Issues Packages Projects Releases Wiki Activity

Address review of template inheritance

Browse Source 
- Unlinking a field enables Apply on its own and sends the current value.
- The link button no longer toggles the checkbox of its label.
- Link and disconnect icons for all four states, pending ones highlighted.
- instance.is_template is hidden.
- ctl create takes --as-template.
- Clearer names for the InstanceManager inheritance helpers.

Part of #200

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
bbassieandClaude Opus 5.5 committed 2026-11-02 11:57:01 +00:00
1 parent 943619a657
commit f14bfc3222
8 files changed
+68 -59
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

docs/configuration.md
+2 -1
View File
@@ -527,6 +527,7 @@ If enabled this instance is a template other instances can inherit config values
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.
Hidden in the web interface, set when creating the template.
Defaults to false.
@@ -540,7 +541,7 @@ When set on an existing instance fields with the same value as the template or t
`instance.name`, `instance.is_template`, `instance.template_id`, the port fields, `factorio.rcon_password` 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.
In the web interface inherited values are linked and read only until they are unlinked.
Defaults to null meaning no template.
docs/managing-a-cluster.md
+2 -2
View File
@@ -86,9 +86,9 @@ Templates are only listed if `--templates` is passed.
### Create instance
ctl> instance create <name> [--id <instance id>] [--from <instance id>] [--template <instance>] [--is-template]
ctl> instance create <name> [--id <instance id>] [--from <instance id>] [--template <instance>] [--as-template]
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 <instance>` to make a template out of an existing instance.
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. `--as-template` creates the instance as a template, see `instance.is_template` in the [configuration docs](configuration.md). Use `--as-template --from <instance>` to make a template out of an existing instance.
### Kill instance
packages/controller/src/InstanceManager.ts
+15 -15
View File
@@ -115,13 +115,13 @@ export default class InstanceManager {
addInstanceHooks(instance: InstanceRecord): void {
instance.config.on("fieldChanged", (field: string, curr: unknown, prev: unknown) => {
if (field === "instance.template_id") {
this._link(instance);
this._linkMatchingFields(instance);
} else if (!this._inheriting && instance.config.isInherited(field)) {
this._override(instance, field, curr, prev);
this._unlinkEditedField(instance, field, curr, prev);
}
this.records.set(instance);
this._notifyConfigFieldChanged(instance, field, curr, prev);
this._propagate(instance, field, curr);
this._updateInheritors(instance, field, curr);
});
}
@@ -203,7 +203,7 @@ export default class InstanceManager {
}
}
config.set("instance.inherited_fields", inherited);
config.update(InstanceManager._resolve(config, template.config, Object.keys(inherited)), false);
config.update(InstanceManager._inheritedValues(config, template.config, Object.keys(inherited)), false);
}
/**
@@ -234,7 +234,7 @@ export default class InstanceManager {
inherited[field] = overrides.filter(p => p !== prop);
}
instance.config.set("instance.inherited_fields", inherited);
this._resolveInto(instance, [field]);
this._copyInheritedValues(instance, [field]);
}
/**
@@ -279,7 +279,7 @@ export default class InstanceManager {
}
/** Values the given inherited fields take from the template, with object overrides applied */
private static _resolve(config: lib.InstanceConfig, template: lib.InstanceConfig, fields: string[]) {
private static _inheritedValues(config: lib.InstanceConfig, template: lib.InstanceConfig, fields: string[]) {
const inherited = config.get("instance.inherited_fields");
const values: Record<string, any> = {};
for (const field of fields) {
@@ -303,12 +303,12 @@ export default class InstanceManager {
}
/** Update the given inherited fields of a live instance from its template */
private _resolveInto(instance: InstanceRecord, fields: string[]): void {
private _copyInheritedValues(instance: InstanceRecord, fields: string[]): void {
const template = this._templateOf(instance.config);
if (!template) {
return;
}
const values = InstanceManager._resolve(instance.config, template.config, fields);
const values = InstanceManager._inheritedValues(instance.config, template.config, fields);
this._inheriting += 1;
try {
instance.config.update(values, true);
@@ -317,8 +317,8 @@ export default class InstanceManager {
}
}
/** Track a change to an inherited field made outside of inheritance */
private _override(instance: InstanceRecord, field: string, curr: unknown, prev: unknown): void {
/** Unlink a field, or the changed properties of an object field, edited on the instance */
private _unlinkEditedField(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);
@@ -333,12 +333,12 @@ export default class InstanceManager {
instance.config.set("instance.inherited_fields", inherited);
if (removed.length) {
// Removed properties fall back to the template value
this._resolveInto(instance, [field]);
this._copyInheritedValues(instance, [field]);
}
}
/** Recompute inherited fields after template_id changed, values equal to the template or default are inherited */
private _link(instance: InstanceRecord): void {
private _linkMatchingFields(instance: InstanceRecord): void {
const template = this._templateOf(instance.config);
const inherited: InheritedFields = {};
if (template) {
@@ -360,11 +360,11 @@ export default class InstanceManager {
}
}
instance.config.set("instance.inherited_fields", inherited);
this._resolveInto(instance, Object.keys(inherited));
this._copyInheritedValues(instance, Object.keys(inherited));
}
/** Copy a changed value from a template to the instances inheriting it */
private _propagate(template: InstanceRecord, field: string, value: unknown): void {
private _updateInheritors(template: InstanceRecord, field: string, value: unknown): void {
if (!lib.InstanceConfig.isInheritable(field)) {
return;
}
@@ -372,7 +372,7 @@ export default class InstanceManager {
if (!instance.config.isInherited(field)) {
continue;
}
this._resolveInto(instance, [field]);
this._copyInheritedValues(instance, [field]);
this._controller.instanceConfigUpdated(instance).catch(err => {
logger.error(`Failed to send config of instance ${instance.id} to its host:\n${err.stack}`);
});
packages/ctl/src/commands_instance.ts
+3 -3
View File
@@ -39,11 +39,11 @@ instanceCommands.add(new lib.Command({
"id": { type: "number", nargs: 1, describe: "Instance id" },
"from": { type: "number", nargs: 1, describe: "Clone config from instance id" },
"template": { type: "string", nargs: 1, describe: "Template to inherit config from" },
"is-template": { type: "boolean", nargs: 0, describe: "Create the instance as a template" },
"as-template": { type: "boolean", nargs: 0, describe: "Create the instance as a template" },
});
}],
handler: async function(
args: { name: string, id?: number, from?: number, template?: string, isTemplate?: boolean },
args: { name: string, id?: number, from?: number, template?: string, asTemplate?: boolean },
ctl: Ctl,
) {
let instanceConfig = new lib.InstanceConfig("control");
@@ -56,7 +56,7 @@ instanceCommands.add(new lib.Command({
instanceConfig.set("instance.template_id", await lib.resolveInstance(ctl, args.template));
fields.push("instance.template_id");
}
if (args.isTemplate) {
if (args.asTemplate) {
instanceConfig.set("instance.is_template", true);
fields.push("instance.is_template");
}
packages/lib/src/config/definitions.ts
+1
View File
@@ -608,6 +608,7 @@ export class InstanceConfig extends classes.Config<InstanceConfigFields> {
access: ["controller", "control"],
type: "boolean",
initialValue: false,
hidden: true,
},
"instance.template_id": {
title: "Template",
packages/web_ui/src/components/BaseConfigTree.tsx
+40 -33
View File
@@ -4,9 +4,7 @@ import {
} from "antd";
import DisconnectOutlined from "@ant-design/icons/DisconnectOutlined";
import LinkOutlined from "@ant-design/icons/LinkOutlined";
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";
@@ -69,7 +67,7 @@ function renderInput(inputComponents: Map<string, InputComponent>, def: lib.Fiel
return `Unknown type ${def.type}`;
}
/** Lets fields follow the value of another config, inherited values are locked until unlocked */
/** Lets fields follow the value of another config, inherited values are locked until unlinking */
export type ConfigInheritance = {
canInherit: (config: lib.Config<any>, field: string) => boolean;
isInherited: (config: lib.Config<any>, field: string, prop?: string) => boolean;
@@ -80,10 +78,10 @@ export type ConfigInheritance = {
type Linking = Map<string, [field: string, prop?: string]>;
type LockState = {
type LinkState = {
inheritance?: ConfigInheritance;
unlocked: Set<string>;
setUnlocked: (unlocked: Set<string>) => void;
unlinking: Set<string>;
setUnlinking: (unlinking: Set<string>) => void;
linking: Linking;
setLinking: (linking: Linking) => void;
};
@@ -103,7 +101,7 @@ export default function BaseConfigTree(props: BaseConfigTreeProps) {
let [changedFields, setChangedFields] = useState<Set<string>>(new Set());
let [errorFields, setErrorFields] = useState(new Map<string, string>());
let [applying, setApplying] = useState(false);
let [unlocked, setUnlocked] = useState<Set<string>>(new Set());
let [unlinking, setUnlinking] = useState<Set<string>>(new Set());
let [linking, setLinking] = useState<Linking>(new Map());
const available = props.available ?? true;
const control = useContext(ControlContext);
@@ -170,7 +168,7 @@ export default function BaseConfigTree(props: BaseConfigTreeProps) {
await props.inheritance!.inherit(field, prop);
}
setChangedFields(new Set());
setUnlocked(new Set());
setUnlinking(new Set());
setLinking(new Map());
}
@@ -183,7 +181,7 @@ export default function BaseConfigTree(props: BaseConfigTreeProps) {
setConfig,
props.ConfigClass,
onValuesChange,
{ inheritance: props.inheritance, unlocked, setUnlocked, linking, setLinking },
{ inheritance: props.inheritance, unlinking, setUnlinking, linking, setLinking },
);
return <>
@@ -197,7 +195,8 @@ export default function BaseConfigTree(props: BaseConfigTreeProps) {
onClick={async () => {
setApplying(true);
const fields: Record<string, string | Record<string, string>> = {};
for (const field of changedFields) {
// Unlinked fields are sent even when unchanged so they stop following the template
for (const field of new Set([...changedFields, ...unlinking])) {
const value = form.getFieldValue(field);
if (propsMap.has(field)) {
const [fieldName, prop] = propsMap.get(field)!;
@@ -237,7 +236,7 @@ export default function BaseConfigTree(props: BaseConfigTreeProps) {
props.onApply();
}
}}
disabled={changedFields.size === 0 && linking.size === 0}
disabled={changedFields.size === 0 && linking.size === 0 && unlinking.size === 0}
>
Apply
</Button>
@@ -246,7 +245,7 @@ export default function BaseConfigTree(props: BaseConfigTreeProps) {
onClick={() => {
form.resetFields();
setChangedFields(new Set());
setUnlocked(new Set());
setUnlinking(new Set());
setLinking(new Map());
setErrorFields(new Map());
}}
@@ -284,7 +283,7 @@ function computeTreeData(
setConfig: (newConfig: lib.Config<any>) => void,
ConfigClass: BaseConfigTreeProps["ConfigClass"],
onValuesChange: (changedValues: object) => void,
lock: LockState,
link: LinkState,
) {
let restartTip = <Tooltip
className="ant-form-item-tooltip"
@@ -302,36 +301,43 @@ function computeTreeData(
children: ChildNode[];
};
function lockButton(key: string, fieldName: string, prop?: string) {
const { inheritance, unlocked, setUnlocked, linking, setLinking } = lock;
function linkButton(key: string, fieldName: string, prop?: string) {
const { inheritance, unlinking, setUnlinking, linking, setLinking } = link;
if (!inheritance?.canInherit(config, fieldName) || !config.canAccess(fieldName, lib.ConfigAccess.write)) {
return null;
}
const button = (title: string, icon: React.ReactNode, label: string, onClick: () => void) => <Tooltip
title={title}
>
<Button type="text" size="small" icon={icon} aria-label={`${label} ${key}`} onClick={onClick} />
const button = (
title: string, icon: React.ReactNode, label: string, pending: boolean, onClick: () => void,
) => <Tooltip title={title}>
<Button
type={pending ? "link" : "text"} size="small" icon={icon} aria-label={`${label} ${key}`}
onClick={e => {
// The button sits in the label, which would toggle checkboxes
e.preventDefault();
onClick();
}}
/>
</Tooltip>;
const revert = () => {
form.setFieldValue(key, initialValues[key]);
onValuesChange({ [key]: initialValues[key] });
};
if (inheritance.isInherited(config, fieldName, prop)) {
if (!unlocked.has(key)) {
if (!unlinking.has(key)) {
return button(
"Linked to the template. Click to unlock and override it.", <LockOutlined />, "Unlock",
() => setUnlocked(new Set([...unlocked, key])),
"Linked to the template. Click to unlink it and set your own value.", <LinkOutlined />, "Unlink",
false, () => setUnlinking(new Set([...unlinking, key])),
);
}
return button(
"Unlocked, applying a change unlinks it from the template. Click to lock it again.",
<UnlockOutlined />, "Lock",
() => { setUnlocked(new Set([...unlocked].filter(k => k !== key))); revert(); },
"Unlinked from the template when applied. Click to keep it linked.", <DisconnectOutlined />,
"Keep linked", true,
() => { setUnlinking(new Set([...unlinking].filter(k => k !== key))); revert(); },
);
}
if (!linking.has(key)) {
return button(
"Not linked to the template. Click to link it again when applied.", <DisconnectOutlined />, "Link",
"Not linked to the template. Click to link it again.", <DisconnectOutlined />, "Link", false,
() => {
setLinking(new Map([...linking, [key, [fieldName, prop]]]));
revert();
@@ -342,14 +348,15 @@ function computeTreeData(
);
}
return button(
"Linked to the template when applied. Click to keep this value instead.", <LinkOutlined />, "Unlink",
"Linked to the template when applied. Click to keep this value instead.", <LinkOutlined />,
"Keep unlinked", true,
() => { setLinking(new Map([...linking].filter(([k]) => k !== key))); revert(); },
);
}
function isReadonly(canWrite: boolean, key: string, fieldName: string, prop?: string) {
return !canWrite || lock.linking.has(key) || Boolean(lock.inheritance?.canInherit(config, fieldName))
&& lock.inheritance!.isInherited(config, fieldName, prop)
&& !lock.unlocked.has(key);
return !canWrite || link.linking.has(key) || Boolean(link.inheritance?.canInherit(config, fieldName))
&& link.inheritance!.isInherited(config, fieldName, prop)
&& !link.unlinking.has(key);
}
let treeData = [];
@@ -410,7 +417,7 @@ function computeTreeData(
label={<>
{`"${prop}"`}
{restart && restartTip}
{lockButton(propPath, fieldName, prop)}
{linkButton(propPath, fieldName, prop)}
</>}
dependencies={def.dependsOn}
rules={[({getFieldValue}) => ({ validator: async (_, fieldValue) => {
@@ -481,7 +488,7 @@ function computeTreeData(
}
let propPath = `${groupName}.${field}.${propName}`;
lock.setUnlocked(new Set([...lock.unlocked, propPath]));
link.setUnlinking(new Set([...link.unlinking, propPath]));
form.setFieldsValue({
[`${newPropPath}.name`]: "",
[`${newPropPath}.value`]: "",
@@ -507,7 +514,7 @@ function computeTreeData(
label={<>
{def.title || field}
{def.restartRequired && restartTip}
{lockButton(fieldName, fieldName)}
{linkButton(fieldName, fieldName)}
</>}
validateStatus={errorFields.has(fieldName) ? "error" : undefined}
help={errorFields.get(fieldName)}
packages/web_ui/src/components/InstanceConfigTree.tsx
+2 -2
View File
@@ -20,9 +20,9 @@ function TemplateNotice(props: { templateId: number }) {
showIcon
style={{ marginBottom: 8 }}
message={<>
Locked values are inherited from the template{" "}
Linked values are inherited from the template{" "}
<Link to={`/instances/${props.templateId}/view`}>{template?.name ?? props.templateId}</Link>.
Unlock a value to override it on this instance.
Unlink a value to set your own on this instance.
</>}
/>;
}
test/integration/clusterio.js
+3 -3
View File
@@ -978,7 +978,7 @@ describe("Integration of Clusterio", function() {
}
});
it("creates a template and an instance inheriting from it", async function() {
await execCtl("instance create inheritTemplate --id 4400 --is-template");
await execCtl("instance create inheritTemplate --id 4400 --as-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));
@@ -998,7 +998,7 @@ describe("Integration of Clusterio", function() {
assert.equal(config["factorio.settings"].afk_autokick_interval, 9);
});
it("creates a template from an existing instance", async function() {
await execCtl("instance create inheritCopy --id 4403 --is-template --from 4401");
await execCtl("instance create inheritCopy --id 4403 --as-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);
@@ -1011,7 +1011,7 @@ describe("Integration of Clusterio", function() {
});
it("rejects assigning a template to a host", async function() {
await assert.rejects(
execCtl("instance assign inheritTemplate 4"),
execCtl(`instance assign inheritTemplate ${hostId}`),
new lib.ValidationError("Templates cannot be assigned to a host"),
);
});