Address review on the role api

- Roles are objects and everything done to or with a role is a method on it:
  assign, unassign, has_player, has_permission, is_higher_than,
  is_lower_than, get_players, get_player_names, print. Assignment has one
  entry point, role:assign(player, options), with local_only as an option
  rather than a second function.
- Roles are looked up by clusterio id with get_role; get_role_by_name searches
  the list for the few places, such as configs, which only know a name. The
  name map and the ordered list are gone, get_roles sorts on demand and the
  index field is replaced by the comparison methods.
- Players are LuaPlayer objects only, with nil or index 0 for the server.
- get_higher_roles and get_lower_roles replace print_to_roles_higher and
  print_to_roles_lower, call sites loop over them with role:print.
- Permission groups are removed from roles again, exp_groups owns the mapping
  from roles to groups.
- Seeding is a SeedRolesRequest behind a button on the roles page rather than
  running on first start, and creates only the roles; the player assignments
  are dropped. The seed lists each permission once at the lowest role which
  has it and lets the parent chain carry it up.
- System commands unlock for core.admin rather than a permission of their own.
- Role metatables are registered with Storage.register_metatable so the
  methods survive save and load, which the role records in storage needed.
- The player list auth uses Roles.player_outranks directly, and the event
  carries role ids rather than names.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

exp_legacy/module/config/afk_kick.lua

@@ -8,7 +8,7 @@ return {
     trust_time = 3600 * 60 * 10, --- @setting trust_time The time in ticks that a player must be online for to count as trusted
     update_time = 3600 * 30, --- @setting update_time How often in ticks the script checks for active players
     custom_active_check = function(player)
-        local veteran = Roles.get_role("Veteran")
-        return veteran ~= nil and Roles.get_player_highest_role(player).index <= veteran.index
+        local veteran = Roles.get_role_by_name("Veteran")
+        return veteran ~= nil and not Roles.get_player_highest_role(player):is_lower_than(veteran)
     end,
 }

exp_legacy/module/config/gui/player_list_actions.lua

@@ -21,11 +21,6 @@ local function set_accessors(player_getter, action_setter)
     get_selected_player, set_selected_action = player_getter, action_setter
 end
 
--- auth that will only allow when on player's of lower roles
-local function auth_lower_role(player, selected_player_name)
-    return Roles.player_outranks(player, selected_player_name)
-end
-
 -- gets the action player and a coloured name for the action to be used on
 local function get_action_player(player)
     local selected_player = get_selected_player(player) --[[@as LuaPlayer]]
@@ -98,7 +93,10 @@ local function report_player_callback(player, reason)
     local selected_player, selected_player_color = get_action_player(player)
     local by_player_name_color = format_player_name(player)
     game.print{ "exp-commands_reports.response", selected_player_color, reason }
-    Roles.print_to_roles_higher("Trainee", { "exp-commands_reports.response-admin", selected_player_color, by_player_name_color, reason })
+    local trainee = Roles.get_role_by_name("Trainee")
+    for _, role in ipairs(trainee and Roles.get_higher_roles(trainee) or {}) do
+        role:print{ "exp-commands_reports.response-admin", selected_player_color, by_player_name_color, reason }
+    end
     Reports.report_player(selected_player.name, player.name, reason)
 end
 
@@ -180,22 +178,22 @@ return {
             report_player,
         },
         ["exp_scenario.command.create_warning"] = {
-            auth = auth_lower_role, -- warn a lower user, replaces report
+            auth = Roles.player_outranks, -- warn a lower user, replaces report
             reason_callback = warn_player_callback,
             warn_player,
         },
         ["exp_scenario.command.jail"] = {
-            auth = auth_lower_role,
+            auth = Roles.player_outranks,
             reason_callback = jail_player_callback,
             jail_player,
         },
         ["exp_scenario.gui.player_list.kick"] = {
-            auth = auth_lower_role,
+            auth = Roles.player_outranks,
             reason_callback = kick_player_callback,
             kick_player,
         },
         ["exp_scenario.gui.player_list.ban"] = {
-            auth = auth_lower_role,
+            auth = Roles.player_outranks,
             reason_callback = ban_player_callback,
             ban_player,
         },

exp_legacy/module/modules/control/jail.lua

@@ -20,8 +20,10 @@ local Roles = require("modules/exp_roles")
 
 local valid_player = function(p) return type(p) == "userdata" and p or game.get_player(p) end
 
---- The role which is given to jailed players, it has a higher priority than every other role
-local jail_role = "Jail"
+--- The role given to jailed players, it has a higher priority than every other role
+local function jail_role()
+    return assert(Roles.get_role_by_name("Jail"), "The Jail role does not exist")
+end
 
 local Jail = {
     events = {
@@ -62,7 +64,8 @@ end
 -- @tparam LuaPlayer player the player to check if they are in jail
 -- @treturn boolean whether the player is currently in jail
 function Jail.is_jailed(player)
-    return Roles.player_has_role(valid_player(player), jail_role)
+    local valid = valid_player(player)
+    return valid ~= nil and jail_role():has_player(valid)
 end
 
 --- Moves a player to jail, which suppresses all of their other roles
@@ -77,7 +80,8 @@ function Jail.jail_player(player, by_player_name, reason)
 
     reason = reason or "Non given."
 
-    if Roles.player_has_role(player, jail_role) then return end
+    local role = jail_role()
+    if role:has_player(player) then return end
 
     player.walking_state = { walking = false, direction = player.walking_state.direction }
     player.riding_state = { acceleration = defines.riding.acceleration.nothing, direction = player.riding_state.direction }
@@ -86,7 +90,7 @@ function Jail.jail_player(player, by_player_name, reason)
     player.picking_state = false
     player.repair_state = { repairing = false, position = player.repair_state.position }
 
-    Roles.assign_player(player, jail_role, by_player_name, true)
+    role:assign(player, { by_player_name = by_player_name, silent = true })
 
     event_emit(Jail.events.on_player_jailed, player, by_player_name, reason)
 
@@ -102,9 +106,10 @@ function Jail.unjail_player(player, by_player_name)
     if not player then return end
     if not by_player_name then return end
 
-    if not Roles.player_has_role(player, jail_role) then return end
+    local role = jail_role()
+    if not role:has_player(player) then return end
 
-    Roles.unassign_player(player, jail_role, by_player_name, true)
+    role:unassign(player, { by_player_name = by_player_name, silent = true })
 
     event_emit(Jail.events.on_player_unjailed, player, by_player_name)
 

exp_roles/controller.ts

@@ -1,7 +1,7 @@
 import { BaseControllerPlugin, InstanceRecord } from "@clusterio/controller";
 import * as lib from "@clusterio/lib";
 import * as messages from "./messages";
-import { SeedRole, seedRoles, seedAssignments, flattenSeedPermissions } from "./seed";
+import { SeedRole, seedRoles, flattenSeedPermissions } from "./seed";
 import * as path from "node:path";
 
 export class ControllerPlugin extends BaseControllerPlugin {
@@ -17,12 +17,6 @@ export class ControllerPlugin extends BaseControllerPlugin {
 			).bootstrap()
 		);
 
-		// An empty datastore means the plugin has not run before, so the roles
-		// the scenario used to define are created on the controller
-		if (this.roleMeta.size === 0) {
-			this.seed();
-		}
-
 		// The datastore can be out of step with the roles, either because the
 		// plugin was installed after they were created or because it was
 		// uninstalled while they were deleted
@@ -41,6 +35,7 @@ export class ControllerPlugin extends BaseControllerPlugin {
 
 		this.controller.handle(messages.RoleListRequest, this.handleRoleListRequest.bind(this));
 		this.controller.handle(messages.RoleMetaUpdateRequest, this.handleRoleMetaUpdateRequest.bind(this));
+		this.controller.handle(messages.SeedRolesRequest, this.handleSeedRolesRequest.bind(this));
 
 		this.controller.handle(messages.AssignmentListRequest, this.handleAssignmentListRequest.bind(this));
 		this.controller.handle(messages.AssignmentUpdateRequest, this.handleAssignmentUpdateRequest.bind(this));
@@ -55,18 +50,16 @@ export class ControllerPlugin extends BaseControllerPlugin {
 	*/
 
 	/**
-	 * Create the roles the scenario shipped with and give the players it listed
-	 * their roles. Roles which already exist by name are reused.
+	 * Create the roles the scenario shipped with. Roles which already exist
+	 * by name are reused and only gain the seed permissions.
 	 */
-	seed() {
-		const idsByName = new Map<string, number>();
+	async handleSeedRolesRequest() {
 		for (const [index, seedRole] of seedRoles.entries()) {
 			const role = this.seedRole(seedRole);
 			if (!role) {
 				continue;
 			}
 
-			idsByName.set(seedRole.name, role.id);
 			this.roleMeta.set(new messages.RoleMetaRecord(
 				role.id,
 				index + 1,
@@ -74,23 +67,12 @@ export class ControllerPlugin extends BaseControllerPlugin {
 				seedRole.shortHand,
 				"",
 				seedRole.color,
-				seedRole.permissionGroup,
 				seedRole.autoAssignHours === undefined ? null : seedRole.autoAssignHours * 3600000,
 				seedRole.blockAutoAssign ?? false,
 			));
 		}
 
-		for (const [name, roleNames] of Object.entries(seedAssignments)) {
-			const roleIds = roleNames.map(roleName => idsByName.get(roleName)).filter(id => id !== undefined);
-			if (roleIds.length !== roleNames.length) {
-				this.logger.warn(`Seed assignment for ${name} names a role which does not exist`);
-			}
-			const user = this.controller.users.getOrCreateUser(name);
-			user.set("roleIds", new Set([...user.roleIds, ...roleIds]));
-			this.controller.userPermissionsUpdated(user);
-		}
-
-		this.logger.info(`Seeded ${seedRoles.length} roles and ${Object.keys(seedAssignments).length} assignments`);
+		this.logger.info(`Seeded ${seedRoles.length} roles`);
 	}
 
 	/** Find or create the clusterio role for a seed role, returns undefined if it has no role to use. */

exp_roles/index.ts

@@ -27,6 +27,7 @@ export const plugin: lib.PluginDeclaration = {
 
 		messages.RoleListRequest,
 		messages.RoleMetaUpdateRequest,
+		messages.SeedRolesRequest,
 
 		messages.AssignmentListRequest,
 		messages.AssignmentUpdateRequest,

exp_roles/messages.ts

@@ -51,11 +51,6 @@ export class RoleMetaRecord {
 		public tag: string = "",
 		/** Colour used for the role in game, null uses the default colour. */
 		public color: RoleColor | null = null,
-		/**
-		 * Factorio permission group players are moved into while this is their
-		 * most privileged role which names one, empty leaves the group alone.
-		 */
-		public permissionGroup: string = "",
 		/** Online time after which this role is granted, null never grants it. */
 		public autoAssignOnlineTimeMs: number | null = null,
 		/** When true holders of this role are never granted roles automatically. */
@@ -71,7 +66,6 @@ export class RoleMetaRecord {
 		short_hand: Type.Optional(Type.String()),
 		tag: Type.Optional(Type.String()),
 		color: Type.Optional(Type.Union([RoleColor.jsonSchema, Type.Null()])),
-		permission_group: Type.Optional(Type.String()),
 		auto_assign_online_time_ms: Type.Optional(Type.Union([Type.Number(), Type.Null()])),
 		block_auto_assign: Type.Optional(Type.Boolean()),
 		updated_at_ms: Type.Optional(Type.Number()),
@@ -100,10 +94,6 @@ export class RoleMetaRecord {
 			json.color = this.color.toJSON();
 		}
 
-		if (this.permissionGroup) {
-			json.permission_group = this.permissionGroup;
-		}
-
 		if (this.autoAssignOnlineTimeMs !== null) {
 			json.auto_assign_online_time_ms = this.autoAssignOnlineTimeMs;
 		}
@@ -131,7 +121,6 @@ export class RoleMetaRecord {
 			json.short_hand ?? "",
 			json.tag ?? "",
 			json.color ? RoleColor.fromJSON(json.color) : null,
-			json.permission_group ?? "",
 			json.auto_assign_online_time_ms ?? null,
 			json.block_auto_assign ?? false,
 			json.updated_at_ms ?? 0,
@@ -376,6 +365,18 @@ export class RoleMetaUpdateRequest {
 	}
 }
 
+/** Create the roles the scenario shipped with, see seed.ts. */
+export class SeedRolesRequest {
+	declare ["constructor"]: typeof SeedRolesRequest;
+	static plugin = "exp_roles" as const;
+	static type = "request" as const;
+	static src = "control" as const;
+	static dst = "controller" as const;
+	static permission = "core.role.create" as const;
+
+	constructor() {}
+}
+
 /*
 	Assignment requests
 */

exp_roles/module/control.lua

@@ -6,19 +6,20 @@ the controller. This module keeps a copy of that state and answers permission
 checks from it, so a check such as `Roles.player_has_permission(player,
 "exp_scenario.command.kill")` is answered from the same data the web ui shows.
 
-Assignments made in game are applied locally first and then sent to the
-controller, which avoids callers having to deal with the round trip. A local
-assignment which is never sent, such as one earned from time on this map, can be
-made with `assign_player_local`.
+Roles are objects, looked up by their clusterio id, and everything done to or
+with a role is a method on it. Assignments made in game are applied locally
+first and then sent to the controller, which avoids callers having to deal with
+the round trip.
 
-Only the roles with the highest priority a player holds are considered. Jail
-sits above every other role, so holding it suppresses the rest, including the
-default role every player has.
+Only the roles with the highest priority a player holds apply. Jail sits above
+every other role, so holding it suppresses the rest, including the default role
+every player has.
 ]]
 
 local clusterio_api = require("modules/clusterio/api")
 local compat = require("modules/clusterio/compat") --[[@as LibCompat]]
 local Async = require("modules/exp_util/async")
+local Storage = require("modules/exp_util/storage")
 
 --- @class ExpRoles
 local ExpRoles = {
@@ -34,8 +35,8 @@ local ExpRoles = {
 --- @class EventData.ExpRoles.on_player_roles_changed : EventData
 --- @field player_index uint
 --- @field by_player_index uint 0 when the change did not come from a player
---- @field assigned string[] Names of the roles which were assigned
---- @field unassigned string[] Names of the roles which were unassigned
+--- @field assigned number[] Ids of the roles which were assigned
+--- @field unassigned number[] Ids of the roles which were unassigned
 
 --- Methods shared by every role, kept apart from the fields so that defining
 --- them does not count as injecting fields into the role itself
@@ -43,19 +44,25 @@ local ExpRoles = {
 local Role = {}
 ExpRoles._prototype = Role
 
+--- Registered so roles keep their methods across save and load
+local role_metatable = Storage.register_metatable("Role", { __index = Role })
+
 --- @class ExpRoles.Role : ExpRoles.RolePrototype
 --- @field id number Clusterio role id
 --- @field name string
 --- @field short_hand string
---- @field order number Order given by the controller, which can have gaps
---- @field index number Position within the role order, with no gaps
+--- @field order number Position given by the controller, compare roles with the methods rather than this
 --- @field priority number Only the highest priority roles a player holds apply
 --- @field tag string
 --- @field color Color?
 --- @field permissions table<string, true> Permission names granted by this role
---- @field permission_group string? Factorio permission group holders are moved to
 --- @field block_auto_assign boolean
 
+--- @class ExpRoles.AssignOptions
+--- @field by_player_name string? Shown in the game message, defaults to the current player or the server
+--- @field silent boolean? When true no game message is printed
+--- @field local_only boolean? When true the controller is not told, for roles earned on this map only
+
 --- @class ExpRoles.ScriptData
 --- @field roles table<number, ExpRoles.Role> Roles indexed by clusterio id
 --- @field synced_players table<string, number[]> Role ids from the controller
@@ -65,49 +72,13 @@ ExpRoles._prototype = Role
 --- @field emit_updates boolean
 local script_data = {}
 
---- Roles in order, the most privileged first, rebuilt from script data
-local ordered_roles = {} --- @type ExpRoles.Role[]
---- Roles indexed by name, rebuilt from script data
-local roles_by_name = {} --- @type table<string, ExpRoles.Role>
 --- Async handlers run when the state of a permission may have changed
 local permission_triggers = {} --- @type table<string, Async.AsyncFunction>
 
---- Move a player into a permission group, done async because the game does not
---- allow it from within every event
-local set_permission_group_async = Async.register(function(player, group)
-    --- @cast player LuaPlayer
-    --- @cast group LuaPermissionGroup
-    if player.valid and group.valid then
-        group.add_player(player)
-    end
-end)
-
 --[[
     Role state
 ]]
 
---- Rebuild the ordered and by name views of the roles
-local function rebuild_role_views()
-    local roles = {}
-    for _, role in pairs(script_data.roles) do
-        roles[#roles + 1] = role
-    end
-
-    -- Sorted on the order given by the controller, which can have gaps, the
-    -- index each role is given is its position within that order
-    table.sort(roles, function(a, b)
-        if a.order == b.order then return a.id < b.id end
-        return a.order < b.order
-    end)
-
-    ordered_roles, roles_by_name = {}, {}
-    for index, role in ipairs(roles) do
-        role.index = index
-        ordered_roles[index] = role
-        roles_by_name[role.name] = role
-    end
-end
-
 --- Build a role from the record sent by the controller
 --- @param record table
 --- @param names string[]? When given, record.permissions holds indexes into it
@@ -126,33 +97,37 @@ local function decode_role(record, names)
         end
     end
 
-    local group = meta.permission_group
-    if group == "" then group = nil end
-
     --- @type ExpRoles.Role
     return setmetatable({
         id = record.id,
         name = record.name,
         short_hand = meta.short_hand or record.name,
         order = meta.order or record.id,
-        index = meta.order or record.id,
         priority = meta.priority or 0,
         tag = meta.tag or "",
         color = meta.color,
         permissions = permissions,
-        permission_group = group,
         block_auto_assign = meta.block_auto_assign or false,
-    }, { __index = ExpRoles._prototype })
+    }, role_metatable)
+end
+
+--- Sort roles in place, the most privileged first
+--- @param roles ExpRoles.Role[]
+--- @return ExpRoles.Role[]
+local function sort_roles(roles)
+    table.sort(roles, function(a, b)
+        if a.order == b.order then return a.id < b.id end
+        return a.order < b.order
+    end)
+    return roles
 end
 
---- Get the name of a player from a player or a name, nil for the server
 --- The server is represented by nil, or by a player object with index 0
---- @param player LuaPlayer | string | nil
---- @return string?
-local function player_name_of(player)
-    if type(player) == "string" then return player end
-    if player and player.index ~= 0 then return player.name end
-    return nil
+--- @param player LuaPlayer?
+--- @return LuaPlayer? # The player when it is not the server
+local function not_server(player)
+    if player == nil or player.index == 0 then return nil end
+    return player
 end
 
 --- Role ids a player has been given, without the default role or priority applied
@@ -171,59 +146,75 @@ local function get_held_role_ids(player_name)
     return role_ids
 end
 
---- Role ids which apply to a player, with the default role and priority applied
+--- Role ids a player has been given, as a set
 --- @param player_name string
 --- @return table<number, true>
-local function get_effective_role_ids(player_name)
+local function get_held_role_set(player_name)
+    local rtn = {}
+    for _, role_id in pairs(get_held_role_ids(player_name)) do
+        rtn[role_id] = true
+    end
+    return rtn
+end
+
+--- Roles which apply to a player, with the default role and priority applied
+--- @param player_name string
+--- @return ExpRoles.Role[]
+local function get_effective_roles(player_name)
     local role_ids = get_held_role_ids(player_name)
     if script_data.default_role_id then
         role_ids[#role_ids + 1] = script_data.default_role_id
     end
 
-    local highest_priority
+    local roles, highest_priority = {}, nil
     for _, role_id in pairs(role_ids) do
         local role = script_data.roles[role_id]
-        if role and (highest_priority == nil or role.priority > highest_priority) then
-            highest_priority = role.priority
+        if role then
+            roles[#roles + 1] = role
+            if highest_priority == nil or role.priority > highest_priority then
+                highest_priority = role.priority
+            end
         end
     end
 
     local rtn = {}
-    for _, role_id in pairs(role_ids) do
-        local role = script_data.roles[role_id]
-        if role and role.priority == highest_priority then
-            rtn[role_id] = true
+    for _, role in pairs(roles) do
+        if role.priority == highest_priority then
+            rtn[#rtn + 1] = role
         end
     end
-    return rtn
+    return sort_roles(rtn)
 end
 
 --[[
     Role lookup
 ]]
 
---- Get a role from its name, its clusterio id, or a role
---- @param any string | number | ExpRoles.Role
+--- Get a role from its clusterio id
+--- @param role_id number
 --- @return ExpRoles.Role?
-function ExpRoles.get_role(any)
-    local t_any = type(any)
-    if t_any == "string" then
-        return roles_by_name[any]
-    elseif t_any == "number" then
-        return script_data.roles[any]
-    elseif t_any == "table" then
-        return script_data.roles[any.id]
-    end
+function ExpRoles.get_role(role_id)
+    return script_data.roles[role_id]
 end
 
---- Get every role in order, the most privileged first
+--- Get a role from its name, roles should be referred to by id where possible
+--- @param name string
+--- @return ExpRoles.Role?
+function ExpRoles.get_role_by_name(name)
+    for _, role in pairs(script_data.roles) do
+        if role.name == name then return role end
+    end
+    return nil
+end
+
+--- Get every role, the most privileged first
 --- @return ExpRoles.Role[]
 function ExpRoles.get_roles()
     local rtn = {}
-    for index, role in ipairs(ordered_roles) do
-        rtn[index] = role
+    for _, role in pairs(script_data.roles) do
+        rtn[#rtn + 1] = role
     end
-    return rtn
+    return sort_roles(rtn)
 end
 
 --- Get the role every player holds
@@ -232,43 +223,60 @@ function ExpRoles.get_default_role()
     return script_data.default_role_id and script_data.roles[script_data.default_role_id] or nil
 end
 
+--- Get a role and every role more privileged than it, the default role excluded
+--- @param role ExpRoles.Role
+--- @return ExpRoles.Role[]
+function ExpRoles.get_higher_roles(role)
+    local rtn = {}
+    for _, other in pairs(script_data.roles) do
+        if not other:is_lower_than(role) and other.id ~= script_data.default_role_id then
+            rtn[#rtn + 1] = other
+        end
+    end
+    return sort_roles(rtn)
+end
+
+--- Get a role and every role less privileged than it, the default role excluded
+--- @param role ExpRoles.Role
+--- @return ExpRoles.Role[]
+function ExpRoles.get_lower_roles(role)
+    local rtn = {}
+    for _, other in pairs(script_data.roles) do
+        if not other:is_higher_than(role) and other.id ~= script_data.default_role_id then
+            rtn[#rtn + 1] = other
+        end
+    end
+    return sort_roles(rtn)
+end
+
 --- Role used when there is no player, such as for commands run by the server
 --- It has core.admin so it passes every permission check
 local server_role = setmetatable({
     id = -1,
     name = "<server>",
     short_hand = "SRV",
-    order = 0,
-    index = 0,
+    order = -math.huge,
     priority = 0,
     tag = "",
     color = nil,
     permissions = { ["core.admin"] = true },
-    permission_group = nil,
     block_auto_assign = true,
-}, { __index = ExpRoles._prototype })
+}, role_metatable)
 
 --- Get the roles which apply to a player, including the default role
 --- Only the roles with the highest priority are returned, which lets a role
 --- such as Jail suppress every other role a player holds
---- @param player LuaPlayer | string | nil
+--- @param player LuaPlayer? nil for the server
 --- @return ExpRoles.Role[]
 function ExpRoles.get_player_roles(player)
-    local player_name = player_name_of(player)
     -- The server is not a player and is allowed to do anything
-    if player_name == nil then return { server_role } end
-
-    local rtn = {}
-    for role_id in pairs(get_effective_role_ids(player_name)) do
-        rtn[#rtn + 1] = script_data.roles[role_id]
-    end
-
-    table.sort(rtn, function(a, b) return a.index < b.index end)
-    return rtn
+    local valid = not_server(player)
+    if not valid then return { server_role } end
+    return get_effective_roles(valid.name)
 end
 
 --- Get the most privileged role which applies to a player
---- @param player LuaPlayer | string | nil
+--- @param player LuaPlayer? nil for the server
 --- @return ExpRoles.Role
 function ExpRoles.get_player_highest_role(player)
     local role = ExpRoles.get_player_roles(player)[1]
@@ -280,7 +288,7 @@ end
 ]]
 
 --- Check if a player has a permission through any of their roles
---- @param player LuaPlayer | string | nil
+--- @param player LuaPlayer? nil for the server
 --- @param permission string A clusterio permission such as `exp_scenario.command.kill`
 --- @return boolean
 function ExpRoles.player_has_permission(player, permission)
@@ -294,49 +302,21 @@ function ExpRoles.player_has_permission(player, permission)
     return false
 end
 
---- Check if a role applies to a player
---- @param player LuaPlayer | string | nil
---- @param search_role string | number | ExpRoles.Role
---- @return boolean
-function ExpRoles.player_has_role(player, search_role)
-    local role = ExpRoles.get_role(search_role)
-    if not role then return false end
-
-    for _, player_role in pairs(ExpRoles.get_player_roles(player)) do
-        if player_role.id == role.id then return true end
-    end
-
-    return false
-end
-
---- Check if a player is more privileged than a role
---- A player with core.admin, which includes the server, outranks every role
---- @param player LuaPlayer | string | nil
---- @param role string | number | ExpRoles.Role
---- @return boolean
-function ExpRoles.player_outranks_role(player, role)
-    local resolved = ExpRoles.get_role(role)
-    if not resolved then return false end
-    if ExpRoles.player_has_permission(player, "core.admin") then return true end
-    local highest = ExpRoles.get_player_roles(player)[1]
-    return highest ~= nil and highest.index < resolved.index
-end
-
 --- Check if a player is more privileged than another player
 --- A player with core.admin, which includes the server, outranks every player
---- @param player LuaPlayer | string | nil
---- @param other LuaPlayer | string | nil
+--- @param player LuaPlayer? nil for the server
+--- @param other LuaPlayer? nil for the server
 --- @return boolean
 function ExpRoles.player_outranks(player, other)
     if ExpRoles.player_has_permission(player, "core.admin") then return true end
     local highest = ExpRoles.get_player_roles(player)[1]
     local other_highest = ExpRoles.get_player_roles(other)[1]
     if highest == nil then return false end
-    return other_highest == nil or highest.index < other_highest.index
+    return other_highest == nil or highest:is_higher_than(other_highest)
 end
 
 --[[
-    Role prototype
+    Role methods
 ]]
 
 --- Check if this role grants a permission
@@ -348,6 +328,35 @@ function Role.has_permission(self, permission)
     return permissions["core.admin"] or permissions[permission] or false
 end
 
+--- Check if this role is more privileged than another
+--- @param self ExpRoles.Role
+--- @param other ExpRoles.Role
+--- @return boolean
+function Role.is_higher_than(self, other)
+    if self.order == other.order then return self.id < other.id end
+    return self.order < other.order
+end
+
+--- Check if this role is less privileged than another
+--- @param self ExpRoles.Role
+--- @param other ExpRoles.Role
+--- @return boolean
+function Role.is_lower_than(self, other)
+    return other:is_higher_than(self)
+end
+
+--- Check if a player has been given this role, or it is the default role
+--- A role a player holds does not always apply, see ExpRoles.get_player_roles
+--- @param self ExpRoles.Role
+--- @param player LuaPlayer? nil for the server
+--- @return boolean
+function Role.has_player(self, player)
+    local valid = not_server(player)
+    if not valid then return self == server_role end
+    if self.id == script_data.default_role_id then return true end
+    return get_held_role_set(valid.name)[self.id] == true
+end
+
 --- Get the names of every player who has been given this role
 --- This includes players who have never joined this map, and is not affected
 --- by priority, so a jailed moderator is still listed under moderator
@@ -401,56 +410,6 @@ function Role.print(self, message)
     return #players
 end
 
---[[
-    Printing to roles
-]]
-
---- Print a message to every player holding one of the given roles
---- @param roles (string | number | ExpRoles.Role)[]
---- @param message LocalisedString
-function ExpRoles.print_to_roles(roles, message)
-    for _, role in pairs(roles) do
-        local resolved = ExpRoles.get_role(role)
-        if resolved then resolved:print(message) end
-    end
-end
-
---- Print a message to every player holding the given role or a more privileged one
---- The default role is never included
---- @param role string | number | ExpRoles.Role
---- @param message LocalisedString
-function ExpRoles.print_to_roles_higher(role, message)
-    local resolved = ExpRoles.get_role(role)
-    if not resolved then return end
-
-    local roles = {}
-    for _, other in ipairs(ordered_roles) do
-        if other.index <= resolved.index and other.id ~= script_data.default_role_id then
-            roles[#roles + 1] = other
-        end
-    end
-
-    ExpRoles.print_to_roles(roles, message)
-end
-
---- Print a message to every player holding the given role or a less privileged one
---- The default role is never included
---- @param role string | number | ExpRoles.Role
---- @param message LocalisedString
-function ExpRoles.print_to_roles_lower(role, message)
-    local resolved = ExpRoles.get_role(role)
-    if not resolved then return end
-
-    local roles = {}
-    for _, other in ipairs(ordered_roles) do
-        if other.index >= resolved.index and other.id ~= script_data.default_role_id then
-            roles[#roles + 1] = other
-        end
-    end
-
-    ExpRoles.print_to_roles(roles, message)
-end
-
 --[[
     Permission triggers
 ]]
@@ -463,23 +422,12 @@ function ExpRoles.define_permission_trigger(permission, callback)
     permission_triggers[permission] = Async.register(callback)
 end
 
---- Run every permission trigger for a player and move them into the permission
---- group of their most privileged role which names one
+--- Run every permission trigger for a player
 --- @param player LuaPlayer
-local function apply_player_state(player)
+local function apply_permission_triggers(player)
     for permission, async_function in pairs(permission_triggers) do
         async_function(player, ExpRoles.player_has_permission(player, permission))
     end
-
-    for _, role in ipairs(ExpRoles.get_player_roles(player)) do
-        if role.permission_group then
-            local group = game.permissions.get_group(role.permission_group)
-            if group and (not player.permission_group or player.permission_group.name ~= group.name) then
-                set_permission_group_async(player, group)
-            end
-            break
-        end
-    end
 end
 
 --[[
@@ -496,22 +444,27 @@ local function emit_player_roles_changed(player, assigned, unassigned, by_player
     by_player_name = by_player_name or (game.player and game.player.name) or "<server>"
     local by_player = game.get_player(by_player_name)
 
+    local assigned_ids, unassigned_ids = {}, {}
     local assigned_names, unassigned_names = {}, {}
-    for index, role in ipairs(assigned) do assigned_names[index] = role.name end
-    for index, role in ipairs(unassigned) do unassigned_names[index] = role.name end
+    for index, role in ipairs(assigned) do
+        assigned_ids[index], assigned_names[index] = role.id, role.name
+    end
+    for index, role in ipairs(unassigned) do
+        unassigned_ids[index], unassigned_names[index] = role.id, role.name
+    end
 
     if not silent then
-        if #assigned_names > 0 then
+        if #assigned > 0 then
             game.print{ "exp-roles.game-message-assign", player.name, table.concat(assigned_names, ", "), by_player_name }
         end
-        if #unassigned_names > 0 then
+        if #unassigned > 0 then
             game.print{ "exp-roles.game-message-unassign", player.name, table.concat(unassigned_names, ", "), by_player_name }
         end
     end
 
-    if #assigned_names > 0 then
+    if #assigned > 0 then
         player.play_sound{ path = "utility/achievement_unlocked" }
-    elseif #unassigned_names > 0 then
+    elseif #unassigned > 0 then
         player.play_sound{ path = "utility/game_lost" }
     end
 
@@ -520,22 +473,11 @@ local function emit_player_roles_changed(player, assigned, unassigned, by_player
         tick = game.tick,
         player_index = player.index,
         by_player_index = by_player and by_player.index or 0,
-        assigned = assigned_names,
-        unassigned = unassigned_names,
+        assigned = assigned_ids,
+        unassigned = unassigned_ids,
     })
 
-    apply_player_state(player)
-end
-
---- Role ids a player has been given, as a set
---- @param player_name string
---- @return table<number, true>
-local function get_held_role_set(player_name)
-    local rtn = {}
-    for _, role_id in pairs(get_held_role_ids(player_name)) do
-        rtn[role_id] = true
-    end
-    return rtn
+    apply_permission_triggers(player)
 end
 
 --- Compare the roles a player had been given before and after a change and
@@ -558,9 +500,7 @@ local function emit_held_diff(player_name, before, by_player_name, silent)
     end
 
     if #assigned > 0 or #unassigned > 0 then
-        table.sort(assigned, function(a, b) return a.index < b.index end)
-        table.sort(unassigned, function(a, b) return a.index < b.index end)
-        emit_player_roles_changed(player, assigned, unassigned, by_player_name, silent)
+        emit_player_roles_changed(player, sort_roles(assigned), sort_roles(unassigned), by_player_name, silent)
     end
 end
 
@@ -591,56 +531,32 @@ local function add_role_id(list, role_id)
     return true
 end
 
---- Convert a role, role name, or array of either into an array of roles
---- @param roles any
---- @return ExpRoles.Role[]
-local function resolve_roles(roles)
-    if type(roles) ~= "table" or roles.name then
-        roles = { roles }
-    end
-
-    local rtn = {}
-    for _, role in pairs(roles) do
-        local resolved = ExpRoles.get_role(role)
-        if resolved then rtn[#rtn + 1] = resolved end
-    end
-
-    return rtn
-end
-
---- Apply a role change to the local state
+--- Apply a role change to the local state, returns true when something changed
 --- @param player_name string
---- @param roles ExpRoles.Role[]
---- @param change_type "assign" | "unassign"
+--- @param role ExpRoles.Role
+--- @param assign boolean
 --- @param sync boolean
---- @return ExpRoles.Role[] # The roles which actually changed
-local function apply_local_change(player_name, roles, change_type, sync)
+--- @return boolean
+local function apply_local_change(player_name, role, assign, sync)
     local local_roles = script_data.local_players[player_name] or {}
     local pending = script_data.pending[player_name] or {}
-    local changed = {}
+    local changed
 
-    for _, role in pairs(roles) do
-        if change_type == "assign" then
-            local synced = script_data.synced_players[player_name] or {}
-            local already_synced = false
-            for _, role_id in pairs(synced) do
-                if role_id == role.id then already_synced = true break end
-            end
-            if not already_synced and add_role_id(local_roles, role.id) then
-                changed[#changed + 1] = role
-                if sync then add_role_id(pending, role.id) end
-            end
-        else
-            local removed = remove_role_id(local_roles, role.id)
-            remove_role_id(pending, role.id)
-            -- A synced role is only taken away here when the controller is
-            -- being told as well, otherwise the next update would restore it
-            if sync then
-                removed = remove_role_id(script_data.synced_players[player_name], role.id) or removed
-            end
-            if removed then
-                changed[#changed + 1] = role
-            end
+    if assign then
+        local synced = script_data.synced_players[player_name] or {}
+        local already_synced = false
+        for _, role_id in pairs(synced) do
+            if role_id == role.id then already_synced = true break end
+        end
+        changed = not already_synced and add_role_id(local_roles, role.id)
+        if changed and sync then add_role_id(pending, role.id) end
+    else
+        changed = remove_role_id(local_roles, role.id)
+        remove_role_id(pending, role.id)
+        -- A synced role is only taken away here when the controller is
+        -- being told as well, otherwise the next update would restore it
+        if sync then
+            changed = remove_role_id(script_data.synced_players[player_name], role.id) or changed
         end
     end
 
@@ -652,83 +568,53 @@ end
 
 --- Send a role change to the controller
 --- @param player_name string
---- @param roles ExpRoles.Role[]
---- @param change_type "assign" | "unassign"
-local function emit_assignment_update(player_name, roles, change_type)
+--- @param role ExpRoles.Role
+--- @param assign boolean
+local function emit_assignment_update(player_name, role, assign)
     if not script_data.emit_updates then return end
 
-    local role_ids = {}
-    for index, role in ipairs(roles) do
-        role_ids[index] = role.id
-    end
-
     clusterio_api.send_json("exp_roles:assignment_update", {
         name = player_name,
-        assign = change_type == "assign" and role_ids or nil,
-        unassign = change_type == "unassign" and role_ids or nil,
+        assign = assign and { role.id } or nil,
+        unassign = not assign and { role.id } or nil,
     })
 end
 
---- Change the roles a player holds
---- @param player LuaPlayer | string
---- @param roles any A role, role name, or array of either
---- @param change_type "assign" | "unassign"
---- @param by_player_name string?
---- @param silent boolean?
---- @param sync boolean
-local function change_player_roles(player, roles, change_type, by_player_name, silent, sync)
-    local player_name = player_name_of(player)
-    if not player_name then return end
-
-    local role_objects = resolve_roles(roles)
-    if #role_objects == 0 then return end
+--- Change whether a player holds a role
+--- @param role ExpRoles.Role
+--- @param player LuaPlayer
+--- @param assign boolean
+--- @param options ExpRoles.AssignOptions?
+local function change_player_role(role, player, assign, options)
+    local valid = not_server(player)
+    if not valid then return end
+    options = options or {}
 
+    local player_name = valid.name
     local before = get_held_role_set(player_name)
-    local changed = apply_local_change(player_name, role_objects, change_type, sync)
-    if #changed == 0 then return end
+    if not apply_local_change(player_name, role, assign, not options.local_only) then return end
 
-    if sync then
-        emit_assignment_update(player_name, changed, change_type)
+    if not options.local_only then
+        emit_assignment_update(player_name, role, assign)
     end
 
-    emit_held_diff(player_name, before, by_player_name, silent)
+    emit_held_diff(player_name, before, options.by_player_name, options.silent)
 end
 
---- Give a player one or more roles, the change is sent to the controller
---- @param player LuaPlayer | string
---- @param roles any A role, role name, or array of either
---- @param by_player_name string? Shown in the game message, defaults to the current player or the server
---- @param silent boolean? When true no game message is printed
-function ExpRoles.assign_player(player, roles, by_player_name, silent)
-    change_player_roles(player, roles, "assign", by_player_name, silent, true)
+--- Give a player this role, the change is sent to the controller unless local_only is set
+--- @param self ExpRoles.Role
+--- @param player LuaPlayer
+--- @param options ExpRoles.AssignOptions?
+function Role.assign(self, player, options)
+    change_player_role(self, player, true, options)
 end
 
---- Take one or more roles from a player, the change is sent to the controller
---- @param player LuaPlayer | string
---- @param roles any A role, role name, or array of either
---- @param by_player_name string? Shown in the game message, defaults to the current player or the server
---- @param silent boolean? When true no game message is printed
-function ExpRoles.unassign_player(player, roles, by_player_name, silent)
-    change_player_roles(player, roles, "unassign", by_player_name, silent, true)
-end
-
---- Give a player one or more roles on this map only, the controller is not told
---- Use this for roles earned from progress which does not leave this map
---- @param player LuaPlayer | string
---- @param roles any A role, role name, or array of either
---- @param by_player_name string? Shown in the game message, defaults to the current player or the server
---- @param silent boolean? When true no game message is printed
-function ExpRoles.assign_player_local(player, roles, by_player_name, silent)
-    change_player_roles(player, roles, "assign", by_player_name, silent, false)
-end
-
---- Take one or more local roles from a player, the controller is not told
---- @param player LuaPlayer | string
---- @param roles any A role, role name, or array of either
---- @param by_player_name string? Shown in the game message, defaults to the current player or the server
---- @param silent boolean? When true no game message is printed
-function ExpRoles.unassign_player_local(player, roles, by_player_name, silent)
-    change_player_roles(player, roles, "unassign", by_player_name, silent, false)
+--- Take this role from a player, the change is sent to the controller unless local_only is set
+--- @param self ExpRoles.Role
+--- @param player LuaPlayer
+--- @param options ExpRoles.AssignOptions?
+function Role.unassign(self, player, options)
+    change_player_role(self, player, false, options)
 end
 
 --[[
@@ -736,7 +622,7 @@ end
 ]]
 
 --- Stop holding a pending role locally once the controller has confirmed it
---- Roles assigned with assign_player_local are never pending so are untouched
+--- Roles assigned with local_only are never pending so are untouched
 --- @param player_name string
 local function drop_confirmed_local(player_name)
     local pending = script_data.pending[player_name]
@@ -758,9 +644,6 @@ end
 --- Restore local references to persistent script data after load
 function ExpRoles.on_load()
     script_data = compat.script_data["exp_roles"]
-    if script_data then
-        rebuild_role_views()
-    end
 end
 
 --- Enable or disable sending role changes back to the controller
@@ -769,7 +652,7 @@ function ExpRoles.set_emit_events(enabled)
     script_data.emit_updates = enabled ~= false
 end
 
---- Apply the state of every connected player, and raise the event so guis
+--- Apply the triggers of every connected player, and raise the event so guis
 --- can refresh, after the roles themselves have changed
 local function roles_changed()
     for _, player in pairs(game.connected_players) do
@@ -818,9 +701,7 @@ function ExpRoles.initialise(payload)
     end
     script_data.pending = {}
 
-    rebuild_role_views()
     roles_changed()
-
     script_data.emit_updates = emit_updates
 end
 
@@ -843,7 +724,6 @@ function ExpRoles.receive_role_updates(records)
         end
     end
 
-    rebuild_role_views()
     roles_changed()
 end
 
@@ -868,17 +748,15 @@ end
 --- Roll back a local assignment which the controller rejected
 --- @param payload { name: string, role_ids: number[] }
 function ExpRoles.reject_assignment(payload)
-    local player_name = payload.name
-    local roles = {}
+    local player = game.get_player(payload.name)
     for _, role_id in pairs(payload.role_ids) do
         local role = script_data.roles[role_id]
-        if role then roles[#roles + 1] = role end
+        if role and player then
+            role:unassign(player, { by_player_name = "<server>", silent = true, local_only = true })
+        elseif role then
+            apply_local_change(payload.name, role, false, false)
+        end
     end
-
-    if #roles == 0 then return end
-
-    -- Sync is false, so this is never sent back to the controller
-    change_player_roles(player_name, roles, "unassign", "<server>", true, false)
 end
 
 --- Get the current script data for debugging purposes
@@ -908,11 +786,11 @@ function ExpRoles.on_server_startup()
     ExpRoles.on_load()
 end
 
---- Apply the permission triggers and group for a player who just joined
+--- Apply the permission triggers for a player who just joined
 --- @param event EventData.on_player_joined_game
 function ExpRoles.on_player_joined_game(event)
     local player = game.get_player(event.player_index)
-    if player then apply_player_state(player) end
+    if player then apply_permission_triggers(player) end
 end
 
 return ExpRoles

exp_roles/seed.ts

@@ -1,8 +1,8 @@
 import { RoleColor } from "./messages";
 
 /**
- * A role created the first time the plugin runs, as the scenario defined it
- * before roles moved to the controller. `parent` stands in for the inheritance
+ * A role created by the seed, as the scenario defined it before roles moved to
+ * the controller. `parent` stands in for the inheritance
  * the old config had and is flattened by the seed. The default and admin roles
  * already exist, so those entries only provide the in game properties.
  */
@@ -10,8 +10,6 @@ export interface SeedRole {
 	name: string;
 	shortHand: string;
 	color: RoleColor | null;
-	/** Factorio permission group, see exp_legacy/module/config/expcore/permission_groups.lua */
-	permissionGroup: string;
 	priority?: number;
 	autoAssignHours?: number;
 	blockAutoAssign?: boolean;
@@ -24,16 +22,11 @@ export interface SeedRole {
 	permissions: string[];
 }
 
-const admin = ["exp_scenario.player.admin", "exp_scenario.player.spectator", "exp_scenario.bypass.reports"];
-const instantRespawn = ["exp_scenario.player.instant_respawn"];
-const trusted = ["exp_scenario.player.spectator", "exp_scenario.bypass.reports", ...instantRespawn];
-
 export const seedRoles: SeedRole[] = [
 	{
 		name: "System",
 		shortHand: "SYS",
 		color: null,
-		permissionGroup: "Default",
 		isAdmin: true,
 		permissions: [],
 	},
@@ -41,11 +34,8 @@ export const seedRoles: SeedRole[] = [
 		name: "Senior Administrator",
 		shortHand: "SAdmin",
 		color: new RoleColor(233, 63, 233),
-		permissionGroup: "Admin",
 		parent: "Administrator",
 		permissions: [
-			...admin, ...instantRespawn,
-			"exp_scenario.player.system_commands",
 			"exp_scenario.command._rcon",
 			"exp_scenario.command.debug",
 			"exp_scenario.command.set_cheat_mode",
@@ -56,10 +46,8 @@ export const seedRoles: SeedRole[] = [
 		name: "Administrator",
 		shortHand: "Admin",
 		color: new RoleColor(233, 63, 233),
-		permissionGroup: "Admin",
 		parent: "Moderator",
 		permissions: [
-			...admin, ...instantRespawn,
 			"exp_scenario.gui.warp_list.bypass_proximity",
 			"exp_scenario.gui.warp_list.bypass_cooldown",
 			"exp_scenario.command.connect_all",
@@ -69,10 +57,9 @@ export const seedRoles: SeedRole[] = [
 		name: "Moderator",
 		shortHand: "Mod",
 		color: new RoleColor(0, 170, 0),
-		permissionGroup: "Admin",
 		parent: "Trainee",
 		permissions: [
-			...admin, ...instantRespawn,
+			"exp_scenario.player.instant_respawn",
 			"exp_scenario.command.assign_role",
 			"exp_scenario.command.unassign_role",
 			"exp_scenario.command.repair",
@@ -107,10 +94,11 @@ export const seedRoles: SeedRole[] = [
 		name: "Trainee",
 		shortHand: "TrMod",
 		color: new RoleColor(0, 170, 0),
-		permissionGroup: "Admin",
 		parent: "Veteran",
 		permissions: [
-			...admin,
+			"exp_scenario.player.admin",
+			"exp_scenario.player.spectator",
+			"exp_scenario.bypass.reports",
 			"exp_scenario.command.admin_chat",
 			"exp_scenario.command.goto",
 			"exp_scenario.command.teleport",
@@ -139,10 +127,8 @@ export const seedRoles: SeedRole[] = [
 		name: "Board Member",
 		shortHand: "Board",
 		color: new RoleColor(247, 246, 54),
-		permissionGroup: "Trusted",
 		parent: "Sponsor",
 		permissions: [
-			...trusted,
 			"exp_scenario.command.goto",
 			"exp_scenario.command.repair",
 			"exp_scenario.command.spectate",
@@ -154,20 +140,17 @@ export const seedRoles: SeedRole[] = [
 		name: "Senior Backer",
 		shortHand: "Backer",
 		color: new RoleColor(238, 172, 44),
-		permissionGroup: "Trusted",
 		parent: "Sponsor",
-		permissions: [
-			...trusted,
-		],
+		permissions: [],
 	},
 	{
 		name: "Sponsor",
 		shortHand: "Spon",
 		color: new RoleColor(238, 172, 44),
-		permissionGroup: "Trusted",
 		parent: "Supporter",
 		permissions: [
-			...trusted,
+			"exp_scenario.bypass.reports",
+			"exp_scenario.player.instant_respawn",
 			"exp_scenario.gui.rocket_info.toggle_active",
 			"exp_scenario.gui.rocket_info.remote_launch",
 			"exp_scenario.gui.bonus",
@@ -182,7 +165,6 @@ export const seedRoles: SeedRole[] = [
 		name: "Supporter",
 		shortHand: "Sup",
 		color: new RoleColor(230, 99, 34),
-		permissionGroup: "Trusted",
 		parent: "Veteran",
 		permissions: [
 			"exp_scenario.player.spectator",
@@ -197,7 +179,6 @@ export const seedRoles: SeedRole[] = [
 		name: "Partner",
 		shortHand: "Part",
 		color: new RoleColor(140, 120, 200),
-		permissionGroup: "Trusted",
 		parent: "Veteran",
 		permissions: [
 			"exp_scenario.player.spectator",
@@ -209,7 +190,6 @@ export const seedRoles: SeedRole[] = [
 		name: "Veteran",
 		shortHand: "Vet",
 		color: new RoleColor(140, 120, 200),
-		permissionGroup: "Trusted",
 		parent: "Member",
 		autoAssignHours: 10,
 		permissions: [
@@ -223,7 +203,6 @@ export const seedRoles: SeedRole[] = [
 		name: "Member",
 		shortHand: "Mem",
 		color: new RoleColor(24, 172, 188),
-		permissionGroup: "Standard",
 		parent: "Regular",
 		permissions: [
 			"exp_scenario.bypass.deconstruction_log",
@@ -245,7 +224,6 @@ export const seedRoles: SeedRole[] = [
 		name: "Regular",
 		shortHand: "Reg",
 		color: new RoleColor(79, 155, 163),
-		permissionGroup: "Standard",
 		autoAssignHours: 3,
 		permissions: [
 			"exp_scenario.command.kill",
@@ -261,7 +239,6 @@ export const seedRoles: SeedRole[] = [
 		name: "Jail",
 		shortHand: "Jail",
 		color: new RoleColor(50, 50, 50),
-		permissionGroup: "Restricted",
 		priority: 1,
 		blockAutoAssign: true,
 		permissions: [],
@@ -270,54 +247,11 @@ export const seedRoles: SeedRole[] = [
 		name: "Guest",
 		shortHand: "",
 		color: new RoleColor(185, 187, 160),
-		permissionGroup: "Guest",
 		isDefault: true,
 		permissions: [],
 	},
 ];
 
-/** Players given roles the first time the plugin runs, by role name. */
-export const seedAssignments: Record<string, string[]> = {
-	"PHIDIAS0303": ["Moderator", "Board Member", "Member"],
-	"aldldl": ["Administrator", "Moderator", "Member"],
-	"arty714": ["Senior Administrator", "Moderator", "Member"],
-	"Cooldude2606": ["Senior Administrator", "Moderator", "Member"],
-	"Drahc_pro": ["Administrator", "Moderator", "Member"],
-	"mark9064": ["Administrator", "Moderator", "Member"],
-	"7h3w1z4rd": ["Moderator", "Member"],
-	"FlipHalfling90": ["Moderator", "Member"],
-	"hamsterbryan": ["Moderator", "Member"],
-	"HunterOfGames": ["Moderator", "Member"],
-	"NextIdea": ["Moderator", "Member"],
-	"TheKernel32": ["Moderator", "Member"],
-	"TheKernel64": ["Moderator", "Member"],
-	"tovernaar123": ["Moderator", "Member"],
-	"UUBlueFire": ["Moderator", "Member"],
-	"AssemblyStorm": ["Moderator", "Member"],
-	"banakeg": ["Moderator", "Member"],
-	"connormkii": ["Moderator", "Member"],
-	"cydes": ["Moderator", "Member"],
-	"darklich14": ["Moderator", "Member"],
-	"facere": ["Moderator", "Member"],
-	"freek18": ["Moderator", "Member"],
-	"Gizan": ["Moderator", "Member"],
-	"LoicB": ["Moderator", "Member"],
-	"M74132": ["Moderator", "Member"],
-	"mafisch3": ["Moderator", "Member"],
-	"maplesyrup01": ["Moderator", "Member"],
-	"ookl": ["Moderator", "Member"],
-	"Phoenix27833": ["Moderator", "Member"],
-	"porelos": ["Moderator", "Member"],
-	"Ruuyji": ["Moderator", "Member"],
-	"samy115": ["Moderator", "Member"],
-	"SilentLog": ["Moderator", "Member"],
-	"Tcheko": ["Moderator", "Member"],
-	"thadius856": ["Moderator", "Member"],
-	"whoami32": ["Moderator", "Member"],
-	"Windbomb": ["Moderator", "Member"],
-	"XenoCyber": ["Moderator", "Member"],
-};
-
 /** The permissions a seed role grants, including those of its parents. */
 export function flattenSeedPermissions(role: SeedRole, roles = seedRoles) {
 	const permissions = new Set<string>();

exp_roles/web/components/RoleProperties.tsx

@@ -15,7 +15,6 @@ type RoleFormValues = {
 	shortHand: string;
 	tag: string;
 	color: string | { toRgb(): { r: number, g: number, b: number } } | null;
-	permissionGroup: string;
 	autoAssignHours: number | null;
 	blockAutoAssign: boolean;
 };
@@ -47,7 +46,6 @@ export default function RoleProperties(props: { plugin: WebPlugin, role?: lib.Ro
 			shortHand: meta.shortHand,
 			tag: meta.tag,
 			color: meta.color ? `rgb(${meta.color.r}, ${meta.color.g}, ${meta.color.b})` : null,
-			permissionGroup: meta.permissionGroup,
 			autoAssignHours: meta.autoAssignOnlineTimeMs === null
 				? null
 				: meta.autoAssignOnlineTimeMs / MS_PER_HOUR,
@@ -80,7 +78,6 @@ export default function RoleProperties(props: { plugin: WebPlugin, role?: lib.Ro
 			values.shortHand ?? "",
 			values.tag ?? "",
 			color,
-			values.permissionGroup ?? "",
 			values.autoAssignHours === null || values.autoAssignHours === undefined
 				? null
 				: values.autoAssignHours * MS_PER_HOUR,
@@ -161,19 +158,6 @@ export default function RoleProperties(props: { plugin: WebPlugin, role?: lib.Ro
 					</Form.Item>
 				</Col>
 
-				<Col xs={24} sm={12} xl={6}>
-					<Form.Item
-						name="permissionGroup"
-						label={labelled(
-							"Permission group",
-							"Factorio permission group players are moved into while this is their "
-							+ "most privileged role which names one. Leave empty to not change their group"
-						)}
-					>
-						<Input />
-					</Form.Item>
-				</Col>
-
 				<Col xs={24} sm={12} xl={6}>
 					<Form.Item
 						name="autoAssignHours"

exp_roles/web/components/SeedRoles.tsx

@@ -0,0 +1,33 @@
+import React, { useContext, useState } from "react";
+import { Button, Popconfirm } from "antd";
+
+import { ControlContext, SectionHeader, useAccount, notifyErrorHandler } from "@clusterio/web_ui";
+
+import { SeedRolesRequest } from "../../messages";
+
+/** Button on the roles page which creates the roles the scenario shipped with. */
+export default function SeedRoles() {
+	const control = useContext(ControlContext);
+	const account = useAccount();
+	const [seeding, setSeeding] = useState(false);
+
+	if (!account.hasPermission("core.role.create")) {
+		return null;
+	}
+
+	return <SectionHeader
+		title="ExpGaming Roles"
+		extra={<Popconfirm
+			title="Create the ExpGaming roles?"
+			description="Roles which already exist by name are kept and only gain permissions."
+			onConfirm={() => {
+				setSeeding(true);
+				control.send(new SeedRolesRequest())
+					.catch(notifyErrorHandler("Error seeding roles"))
+					.finally(() => setSeeding(false));
+			}}
+		>
+			<Button loading={seeding}>Seed roles</Button>
+		</Popconfirm>}
+	/>;
+}

exp_roles/web/index.tsx

@@ -5,6 +5,7 @@ import * as lib from "@clusterio/lib";
 import * as messages from "../messages";
 
 import RoleProperties from "./components/RoleProperties";
+import SeedRoles from "./components/SeedRoles";
 
 export class WebPlugin extends BaseWebPlugin {
 	roles = new lib.MapSubscriber(messages.RoleUpdatedEvent, this.control);
@@ -14,6 +15,7 @@ export class WebPlugin extends BaseWebPlugin {
 		// does not carry in its type
 		this.componentExtra = {
 			RoleViewPage: RoleProperties as React.ComponentType,
+			RolesPage: SeedRoles,
 		};
 	}
 

exp_scenario/module/commands/_authorities.lua

@@ -22,7 +22,7 @@ authorities.exp_permission =
         end
     end)
 
-Roles.define_permission_trigger("exp_scenario.player.system_commands", function(player, state)
+Roles.define_permission_trigger("core.admin", function(player, state)
     if state then
         Commands.unlock_system_commands(player.name)
     else

exp_scenario/module/commands/_types.lua

@@ -19,6 +19,7 @@ local valid, invalid = Commands.status.success, Commands.status.invalid_input
 
 local Roles = require("modules/exp_roles")
 local player_outranks = Roles.player_outranks
+local player_has_permission = Roles.player_has_permission
 
 local types = {} --- @class (partial) Commands.types
 
@@ -34,7 +35,7 @@ types.role =
         if name == nil then
             return invalid{ "exp-commands-parse.string-options", table.concat(names, ", ") }
         else
-            return valid(Roles.get_role(name))
+            return valid(Roles.get_role_by_name(name))
         end
     end)
 
@@ -45,7 +46,7 @@ types.lower_role =
         if not success then return status, result end
         --- @cast result ExpRoles.Role
 
-        if not Roles.player_outranks_role(player, result) then
+        if not player_has_permission(player, "core.admin") and not Roles.get_player_highest_role(player):is_higher_than(result) then
             return invalid{ "exp-commands-parse_role.lower-role" }
         else
             return valid(result)

exp_scenario/module/commands/protected_entities.lua

@@ -218,13 +218,16 @@ SelectEntities:on_stop(on_player_selection_stop)
 
 --- When there is a repeat offence print it in chat
 local function on_repeat_violation(event)
-    Roles.print_to_roles_higher("Regular", {
-        "exp-commands_entity-protection.repeat-offence",
-        format_player_name(event.player_index),
-        event.entity.localised_name,
-        event.entity.position.x,
-        event.entity.position.y
-    })
+    local regular = Roles.get_role_by_name("Regular")
+    for _, role in ipairs(regular and Roles.get_higher_roles(regular) or {}) do
+        role:print{
+            "exp-commands_entity-protection.repeat-offence",
+            format_player_name(event.player_index),
+            event.entity.localised_name,
+            event.entity.position.x,
+            event.entity.position.y
+        }
+    end
 end
 
 return {

exp_scenario/module/commands/roles.lua

@@ -19,7 +19,7 @@ Commands.new("assign-role", { "exp-commands_roles.description-assign" })
     :register(function(player, other_player, role)
         --- @cast other_player LuaPlayer
         --- @cast role ExpRoles.Role
-        Roles.assign_player(other_player, role, player.name)
+        role:assign(other_player, { by_player_name = player.name })
     end)
 
 --- Unassigns a role to a player
@@ -31,7 +31,7 @@ Commands.new("unassign-role", { "exp-commands_roles.description-unassign" })
     :register(function(player, other_player, role)
         --- @cast other_player LuaPlayer
         --- @cast role ExpRoles.Role
-        Roles.unassign_player(other_player, role, player.name)
+        role:unassign(other_player, { by_player_name = player.name })
     end)
 
 --- Lists all roles in they correct order

exp_scenario/module/gui/player_bonus.lua

@@ -68,8 +68,11 @@ do local _points_limit = {} --- @type table<number, number>
     --- @param player LuaPlayer
     --- @return number
     function Elements.bonus_used._calculate_points_limit(player)
-        local role = assert(Roles.get_role(config.points.role_name), "Bonus points role does not exist")
-        local role_diff = role.index - Roles.get_player_highest_role(player).index
+        local roles = Roles.get_roles()
+        local positions = {}
+        for index, role in ipairs(roles) do positions[role.id] = index end
+        local role = assert(Roles.get_role_by_name(config.points.role_name), "Bonus points role does not exist")
+        local role_diff = positions[role.id] - positions[Roles.get_player_highest_role(player).id]
         local points_limit = math.floor(config.points.base * (1 + config.points.increase_percentage_per_role_level * role_diff))
         _points_limit[player.index] = points_limit
         return points_limit

exp_scenario/module/gui/readme.lua

@@ -396,7 +396,7 @@ define_tab(
         for _, group in ipairs(groups) do
             local seen = {}
             for _, role_name in ipairs(group.roles) do
-                local role = Roles.get_role(role_name)
+                local role = Roles.get_role_by_name(role_name)
                 for _, player_name in ipairs(role and role:get_player_names() or {}) do
                     if not seen[player_name] then
                         seen[player_name] = true

exp_scenario/permissions.ts

@@ -123,7 +123,6 @@ const definitions: Definition[] = [
 	["exp_scenario.player.admin", "Factorio admin", "Be promoted to Factorio admin while holding a role with this permission."],
 	["exp_scenario.player.instant_respawn", "Instant respawn", "Respawn after two seconds instead of the default delay."],
 	["exp_scenario.player.spectator", "Spectator", "Remove the zoom to world noise effect, as Factorio does for spectators."],
-	["exp_scenario.player.system_commands", "System commands", "Unlock commands flagged system only, such as /_rcon and /_sudo."],
 ];
 
 for (const [name, title, description, grantByDefault] of definitions) {