Move the scenario onto exp_roles and remove the legacy role system

[bbassie]

Part two of #448. Moves every call site off expcore.roles, deletes the legacy module and config, simplifies jail, and seeds the roles on first run. Taking "improve the role api with no restraint, no external consumers other than the mono repo" at its word, the lua api is redesigned rather than kept compatible, and the permission names are tidied up while nothing holds them yet.

Lua api

The legacy action strings and the transform which mapped them onto permission names are gone. Call sites check the clusterio permission directly, so grep exp_scenario.command.kill finds the definition, the check, and the seed, and there is no second transform which silently breaks every check if it drifts.

Before	After
player_allowed(p, "command/kill")	player_has_permission(p, "exp_scenario.command.kill")
player_has_flag(p, "report-immune")	player_has_permission(p, "exp_scenario.bypass.reports")
define_flag_trigger(flag, fn)	define_permission_trigger(permission, fn) — any permission can have one
events.on_role_assigned / on_role_unassigned	events.on_player_roles_changed with assigned and unassigned names
get_player_highest_role(a).index < get_player_highest_role(b).index	player_outranks(a, b), plus player_outranks_role(a, role)
get_role_from_any / get_role_by_name / get_roles_ordered	get_role(name | id | role) / get_roles()
config.order / config.roles / config.players	removed; role:get_player_names() covers the one use
assign_player(p, roles, by, skip_checks, silent)	assign_player(p, roles, by, silent)

Flags were only permissions with a change trigger, so the separate concept goes. Every consumer registered both role events for the same handler, so they become one, and it is now also raised when the roles a player holds change on the controller, or when a role they hold is edited there. The old events were never raised for either, which left guis stale after a web ui change. player_outranks applies the core.admin bypass consistently; two of the six index comparisons did not.

A role now carries a permission group, which the legacy system mapped roles to and #448 dropped. A player is moved into the group of their most privileged role which names one, which is how Jail actually restricts a player. It sits with the other in game properties on the role page.

Permission names

Explicit list in exp_scenario/permissions.ts, no transform. The action and flag buckets only reflected how the old config was written, so: exp_scenario.bypass.* (entity protection, nuke protection, deconstruction log, reports), exp_scenario.decon.* (standard, fast trees — with descriptions that say what they gate), exp_scenario.player.* (admin, spectator, instant respawn, system commands), exp_scenario.chat.commands, and exp_scenario.gui.player_list.kick / .ban for the player list buttons which were never commands. command.* and gui.* are unchanged apart from clear_tag.always → tag_clear.always to match the command it belongs to.

Commands derive exp_scenario.command.<name> in the authority, so assign-role, unassign-role and get-roles get scenario permissions rather than the core ones #448 mapped them to. The in game command is bounded by the lower role check, core.user.update_roles is not — granting it to Moderator would have let any mod with a web account change anyone's roles.

Dropped: defer_role_changes (priority replaced it), command/give-warning (no role held it; the player list now keys the warn button on create_warning, the permission the command already needed) and command/report (never defined; now create_report). Both buttons were only ever visible to root. _ipc and _sudo are added so every command has a definition.

Jail

jail_player gives the Jail role, unjail_player takes it away. Jail.old_roles and the stash/restore are gone, since priority suppresses the other roles for as long as the role is held.

Seeding

On the first run (empty role properties datastore) the controller creates the roles the scenario shipped with and gives the listed players their roles. The seed keeps the parent relationships from the old config and flattens them, since clusterio roles do not inherit; the flattened sets were diffed against the legacy config and match for every role. Cluster Admin and the default role already exist, so those entries only set properties — the default role gets its permissions through grantByDefault, which is checked to equal the old Guest list. Roles which already exist by name are reused and only gain permissions, so seeding a cluster which already has some is safe.

Verification

• 118 behavioural assertions over the real module under a stubbed environment: lookup, priority suppression, ranking, holders, every sync entry point, pending confirmation, rejection rollback, local only roles, events and messages for both local and controller originated changes, triggers and permission groups, jail.
• End to end in a dev cluster: seed on controller start (15 roles, 38 users), instance on Factorio 2.1.14 starts with the full scenario, permission checks over rcon resolve as expected, in game assignment confirmed by the controller, assignment for an unknown user rejected and rolled back, ctl user set-roles reaching the game.
• emmylua lint as ci runs it: 0 findings. tsc and the web bundle clean.

Notes

• exp_scenario.command.kill.always and spawn.always stay defined; /kill never checked kill.always before either, its lower role parser already bounds it. Left for a later tidy.
• The seed data is the old config, staff list included, so nothing new is in the repo.
• Web ui screenshots are not included — the controller run --dev web build is not set up here, the form only gains one text field next to the existing ones.

🤖 Generated with Claude Code

[Cooldude2606]

Along with all the changes highlighted in this review; can you also weigh the pros and cons of using oop for the roles vs only using module level methods. At the moment there are a few places where we have mutliple ways to do things or where things can be cumbersome, so we should align on a strong dx api interface for working with roles and covering the use cases we have or would expect.

[Cooldude2606]

Roles should not interact with permission groups anymore. This is the responsibility of exp_groups. Therefore this can be removed.

[Cooldude2606]

We should have scripts use the role id not the name. So we dont need this mapping. When we need to get a role by name (which will be rare) we can search the list of roles.

[Cooldude2606]

Why do we need to maintain this order? The only time we check index is when doing direct comparisons, so we dont need this list.

[Cooldude2606]

We should keep "index" internal and instead expose a method for doing comparisons.

[Cooldude2606]

It is bad practice to have mutliple ways to specify a role. We should only use role objects only, and have them be methods instead.

[Cooldude2606]

We can use this directory rather than as a proxy local function.

[Cooldude2606]

As previous comment. We should not have permission groups as part of roles.

[Cooldude2606]

This should be a manual button trigger and not automatic. Only the roles and permissions should be seeded, not the player assignments.

[Cooldude2606]

Can we inline these at the lowest place in the hierarchy. Including these individually is unneeded repition.

[Cooldude2606]

We can change this to be based on core.admin

[bbassie]

Pushed bea8d46 covering every point. The api question first, since it decided the shape of the rest.

Object methods vs module functions

I weighed it as: module functions are easiest when the role is incidental to the call — a permission check where the caller has a player and a permission name and never thinks about roles, or anything that spans roles such as ordering. Methods are easiest when the caller already holds a role, because there is only one way to say it and the role cannot be given three ways (name, id, object) as it could before. The old api mixed both, which is where the duplication came from: assign_player and assign_player_local, player_has_role next to role:get_players, get_role_from_any accepting anything.

So the split is now by what the caller has in hand:

• Module functions take a player or nothing and answer questions about players or the role list: get_role(id), get_role_by_name(name), get_roles(), get_default_role(), get_higher_roles(role), get_lower_roles(role), get_player_roles(player), get_player_highest_role(player), player_has_permission(player, permission), player_outranks(player, other), define_permission_trigger(permission, fn).
• Everything done to or with a role is a method: role:assign(player, options), role:unassign(player, options), role:has_player(player), role:has_permission(permission), role:is_higher_than(other), role:is_lower_than(other), role:get_players(online), role:get_player_names(), role:print(message).

There is one assignment entry point, role:assign(player, { by_player_name, silent, local_only }), with local only as an option rather than a second function. Players are LuaPlayer only, nil or index 0 for the server. Roles are looked up by id; get_role_by_name is a search over the list and is used by the few places which only know a name — the configs, jail, and the role parser — which I think is the right trade rather than maps to keep in step. The name map, the ordered list and the index field are gone; get_roles() sorts on demand and comparisons go through the methods, order stays on the object as data but is documented as not for comparing.

The one cost of methods worth naming: a call site that wants the roles of a player and then something about each of them now does two steps, get_player_roles then a method, where a flat player_has_role(player, "Jail") was one. Jail is the only place that pattern existed and it reads fine as jail_role():has_player(player).

The rest

• Permission groups removed from roles, the web form and the seed; exp_groups owns that mapping. Lua no longer touches game.permissions.
• Seed is a SeedRolesRequest (core.role.create) behind a "Seed roles" button in a section on the roles page, roles and permissions only, no assignments. Existing roles are matched by name and only gain permissions, so it is safe to press again. Verified against the dev cluster by deleting a role and seeding: it came back, the rest were untouched. The seed lists each permission once at the lowest role which has it (player.admin, player.spectator and bypass.reports on Trainee, instant_respawn on Moderator and Sponsor) and the parent chain carries it up; the flattened sets still match the legacy config for every role.
• System commands unlock on core.admin, and exp_scenario.player.system_commands is gone. Note this means Senior Administrator can no longer use /_rcon unless they also hold Cluster Admin, which the old is_system flag allowed — I take that as intended.
• Player list auth uses Roles.player_outranks directly. The get_higher_roles callers (reports, protected entities) loop with role:print.
• The event now carries role ids in assigned / unassigned rather than names, to match.
• While there I registered the role metatable with Storage.register_metatable: the role records live in storage and would have lost their methods on load.

Tests updated for the new api, 108 assertions passing; emmylua still 0 findings; tsc and the web bundle clean; the instance starts with the full scenario and the methods answer as expected over rcon.