diff --git a/server/db/schema.sql b/server/db/schema.sql index 7d56898..4ce751b 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -241,11 +241,29 @@ CREATE TABLE IF NOT EXISTS shard_guild_members ( acct VARCHAR(120) NULL, -- absent for a mobile with no account web_id INT NULL, -- set only when the account is linked is_player TINYINT(1) NOT NULL DEFAULT 1, + -- Guild rank, 0-4, with 4 being Leader (ServUO RankDefinition.Ranks). NULL means + -- "not known", which is a real state and not a demotion: the shard omits the rank + -- for a staff account, because PlayerMobile.GuildRank reports Leader for anyone at + -- GameMaster or above whatever their actual rank, and publishing that would put a + -- staff member on a public roster as a guild leader. + -- Backticked, like `int` on shard_online: RANK is a reserved word in MySQL 8 and + -- a non-reserved keyword in MariaDB, so it parses here bare but must not be + -- written that way anywhere it might not. + `rank` TINYINT NULL, + -- The rank's NAME, as the game states it: a cliloc id for the five standard ranks + -- (1062959-1062963, which ship with no text), or a literal string when a shard has + -- replaced the rank table with custom definitions. Resolving one to a label is this + -- module's job -- it owns the cliloc table and the game vocabulary. + rank_cliloc INT NULL, + rank_name VARCHAR(64) NULL, t BIGINT NULL, -- roster event time, epoch ms updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, PRIMARY KEY (guild_id, serial), INDEX idx_shard_guild_members_acct (acct), - INDEX idx_shard_guild_members_web (web_id) + INDEX idx_shard_guild_members_web (web_id), + -- Leadership is "rank >= 4", asked per guild, which is the query the Team provider + -- runs on every reconcile. + INDEX idx_shard_guild_members_rank (guild_id, rank) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; -- Town-governor board (Protocol 2.0, City Loyalty). One row per city, upserted on @@ -669,4 +687,14 @@ INSERT IGNORE INTO settings (`key`, value) VALUES ('uo_link_protocol_3_migrated' -- only a database that has never seen the key gets the default. Nothing in core -- reads either one; `game_account_signup` is read through ctx.settings by -- server/utils/gameSignup.js, which owns the policy. -INSERT IGNORE INTO settings (`key`, value) VALUES ('game_account_signup', 'disabled'); \ No newline at end of file +INSERT IGNORE INTO settings (`key`, value) VALUES ('game_account_signup', 'disabled'); +-- Protocol 4 guild rank, added to databases that already have shard_guild_members. +-- +-- The table itself is new in Protocol 4 and unreleased, so no production install has +-- it — but `edge` deployments do, from the roster work that landed before the rank +-- amendment, and CREATE TABLE IF NOT EXISTS adds a table and never a column. This is +-- the same gap the sidecar's own store hit when `guilds.members` was added. +ALTER TABLE shard_guild_members ADD COLUMN IF NOT EXISTS `rank` TINYINT NULL; +ALTER TABLE shard_guild_members ADD COLUMN IF NOT EXISTS rank_cliloc INT NULL; +ALTER TABLE shard_guild_members ADD COLUMN IF NOT EXISTS rank_name VARCHAR(64) NULL; +ALTER TABLE shard_guild_members ADD INDEX IF NOT EXISTS idx_shard_guild_members_rank (guild_id, `rank`); diff --git a/server/model/shardState/shardState.db.js b/server/model/shardState/shardState.db.js index 3c0233c..4dbc569 100644 --- a/server/model/shardState/shardState.db.js +++ b/server/model/shardState/shardState.db.js @@ -172,19 +172,32 @@ const clearGuilds = () => query('DELETE FROM shard_guilds') const listGuilds = () => query(`SELECT ${GUILD_COLS} FROM shard_guilds ORDER BY name ASC`) // ── Guild membership (Protocol 4) ────────────────────────────────────────── -const MEMBER_COLS = 'guild_id, serial, name, acct, web_id, is_player, t' +// `rank` is backticked wherever it is written, like `int` on shard_online: it is a +// reserved word in MySQL 8 and merely a keyword in MariaDB, so it parses bare here +// and must not be relied on to. +const MEMBER_COLS = 'guild_id, serial, name, acct, web_id, is_player, `rank`, rank_cliloc, rank_name, t' // Upsert rather than plain insert: a roster frame can be redelivered (the /history // backfill replays stored frames on every reconnect), and a redelivery must be a // no-op rather than a duplicate-key error. +// +// The rank columns are assigned unconditionally, NULL included. A member whose rank +// the shard withheld — a staff account, whose GuildRank getter reports Leader +// regardless of the truth — must go back to "not known" rather than keeping a rank +// from before they were promoted. const upsertGuildMembers = (rows) => { if (!rows.length) return Promise.resolve() - const values = rows.map(() => '(?, ?, ?, ?, ?, ?, ?)').join(', ') - const params = rows.flatMap((r) => [r.guild_id, r.serial, r.name, r.acct, r.web_id, r.is_player, r.t]) + const values = rows.map(() => '(?, ?, ?, ?, ?, ?, ?, ?, ?, ?)').join(', ') + const params = rows.flatMap((r) => [ + r.guild_id, r.serial, r.name, r.acct, r.web_id, r.is_player, + r.rank, r.rank_cliloc, r.rank_name, r.t, + ]) return query( `INSERT INTO shard_guild_members (${MEMBER_COLS}) VALUES ${values} ON DUPLICATE KEY UPDATE name = VALUES(name), acct = VALUES(acct), - web_id = VALUES(web_id), is_player = VALUES(is_player), t = VALUES(t)`, + web_id = VALUES(web_id), is_player = VALUES(is_player), + \`rank\` = VALUES(\`rank\`), rank_cliloc = VALUES(rank_cliloc), + rank_name = VALUES(rank_name), t = VALUES(t)`, params, ) } diff --git a/server/model/shardState/shardState.model.js b/server/model/shardState/shardState.model.js index afde59e..7c25254 100644 --- a/server/model/shardState/shardState.model.js +++ b/server/model/shardState/shardState.model.js @@ -386,6 +386,14 @@ async function upsertGuildRoster(ev) { acct: m.acct ?? null, web_id: Number.isFinite(m.webId) ? m.webId : null, is_player: m.player ? 1 : 0, + // Guild rank (Protocol 4). ABSENT is a real state and is stored as NULL: the + // shard withholds the rank for a staff account, because ServUO's GuildRank + // getter reports Leader for anyone at GameMaster or above whatever their + // actual rank. Defaulting a missing rank to 0 here would turn "we were not + // told" into "rank 0", which is a demotion invented by this line. + rank: Number.isInteger(m.rank) ? m.rank : null, + rank_cliloc: Number.isInteger(m.rankCliloc) ? m.rankCliloc : null, + rank_name: typeof m.rankName === 'string' && m.rankName ? m.rankName.slice(0, 64) : null, t: Number.isFinite(ev.t) ? ev.t : null, })) diff --git a/server/model/teamProvider/teamProvider.db.js b/server/model/teamProvider/teamProvider.db.js index 3608c26..88980ff 100644 --- a/server/model/teamProvider/teamProvider.db.js +++ b/server/model/teamProvider/teamProvider.db.js @@ -10,6 +10,9 @@ // though `ctx` does not exist yet when this file is first required. const { query } = require('../../core') +/** ServUO's `RankDefinition.Ranks[4]` is Leader, and 4 is the top of the ladder. */ +const LEADER_RANK = 4 + /** * The guild board — one row per guild the shard has told us about. * @@ -53,8 +56,8 @@ const findGuild = (id) => */ const listGuildMembers = (guildId) => query( - `SELECT m.serial, m.name, m.acct, m.web_id, m.is_player, - l.user_id AS linked_user_id, + "SELECT m.serial, m.name, m.acct, m.web_id, m.is_player, m.`rank`, m.rank_cliloc, m.rank_name, " + + ` l.user_id AS linked_user_id, (o.serial IS NOT NULL) AS is_online FROM shard_guild_members m LEFT JOIN shard_account_links l ON l.account = m.acct @@ -64,4 +67,21 @@ const listGuildMembers = (guildId) => [guildId], ) -module.exports = { listGuilds, findGuild, listGuildMembers } +/** + * Every member at leader rank — rank 4, the top of ServUO's `RankDefinition.Ranks`. + * + * A set, not a single row, and that is the whole reason Protocol 4 grew a per-member + * rank: the guild board carries one `leader_serial`, so before this the website could + * only ever be told about one leader, while a UO guild routinely has several. + * + * A NULL rank is excluded by the comparison, which is correct — the shard withholds + * the rank for a staff account rather than publishing the Leader its getter falsely + * reports, and "not known" must not be read as "leads this guild". + */ +const listGuildLeaders = (guildId) => + query( + 'SELECT serial FROM shard_guild_members WHERE guild_id = ? AND `rank` >= ? ORDER BY name ASC', + [guildId, LEADER_RANK], + ) + +module.exports = { listGuilds, findGuild, listGuildMembers, listGuildLeaders, LEADER_RANK } diff --git a/server/model/teamProvider/teamProvider.model.js b/server/model/teamProvider/teamProvider.model.js index a18888f..41f5d2b 100644 --- a/server/model/teamProvider/teamProvider.model.js +++ b/server/model/teamProvider/teamProvider.model.js @@ -24,9 +24,27 @@ const core = require('../../core') const db = require('./teamProvider.db') const uoLinkConfig = require('../uoLinkConfig/uoLinkConfig.model') const uoLinkSocket = require('../../utils/uoLinkSocket') +const clilocs = require('../shardClilocs/shardClilocs.model') const log = core.logger('teams') +/** + * ServUO's five stock rank names, by the cliloc id the game names them with. + * + * A fallback, not the source of truth: the operator's own cliloc table is consulted + * first, and a shard with custom rank definitions sends a literal string that beats + * both. This exists because the cliloc table is populated only if someone ran the + * client-file extraction, and a roster on a shard that has not should still say + * "Warlord" rather than nothing. + */ +const STANDARD_RANK_NAMES = { + 1062959: 'Leader', + 1062960: 'Warlord', + 1062961: 'Emissary', + 1062962: 'Member', + 1062963: 'Ronin', +} + /** A refusal, in the shape core reads (§2.3). */ const refuse = (reason) => ({ ok: false, reason }) @@ -116,17 +134,19 @@ async function getTeamMembers(externalId) { return refuse(`roster for guild ${externalId} has not arrived yet (board says ${guild.members} members)`) } - const leaderSerial = guild.leader_serial || null + const labels = await rankLabels(rows) return { ok: true, complete: true, members: rows.map((row) => ({ memberKey: row.serial, displayName: row.name || null, - // Not on the wire. The roster member is the standard actor object, which - // carries no guild rank — see the note at the bottom of this file. - rankLabel: null, - leader: Boolean(leaderSerial && row.serial === leaderSerial), + rankLabel: labels.get(row.serial) || null, + // Rank 4 is Leader, and several members can hold it. A NULL rank is not a + // leader: the shard withholds the rank for a staff account rather than + // publishing the Leader its getter falsely reports, and "not known" must + // never be read as "leads this guild". + leader: Number.isInteger(row.rank) && row.rank >= db.LEADER_RANK, online: Boolean(row.is_online), userId: resolveUserId(row), })), @@ -138,17 +158,17 @@ async function getTeamMembers(externalId) { } /** - * `getTeamLeaders(externalId)` — who leads the guild. + * `getTeamLeaders(externalId)` — everyone at leader rank. * - * **One leader, because that is all the wire carries.** TEAMS.md §2.5 expects - * multiple leaders to be the normal case, from `PlayerMobile.GuildRank.Rank >= 4`, - * and core supports them — but Protocol 4's roster member is the standard actor - * object with no rank field, so the only leadership this module can see is the - * board's single `leader_serial` from `guild.update`. Reporting a guessed second - * leader would be worse than reporting one honestly. + * **All of them, which is why Protocol 4 grew a per-member rank.** The guild board + * carries one `leader_serial`, so before the rank amendment this could only ever + * name a single member, while a UO guild routinely has several at rank 4 and + * TEAMS.md §2.5 treats multiple leaders as the normal case. * - * Raising this to the full set is a protocol change (rank on the actor object), - * not something this file can fix. + * The board's own `leader_serial` is folded in as a floor. It is the guild's + * founder-leader and it comes from a different frame (`guild.update`), so on a + * shard whose roster has not been re-emitted since the amendment it is the only + * leadership signal there is — and it should never be *lost* by moving to ranks. */ async function getTeamLeaders(externalId) { const ready = await boardIsCurrent() @@ -157,13 +177,65 @@ async function getTeamLeaders(externalId) { try { const [guild] = await db.findGuild(externalId) if (!guild) return refuse(`guild ${externalId} is not on the board`) - return { ok: true, leaders: guild.leader_serial ? [guild.leader_serial] : [] } + + const rows = await db.listGuildLeaders(externalId) + const leaders = rows.map((r) => r.serial) + + if (guild.leader_serial && !leaders.includes(guild.leader_serial)) { + leaders.push(guild.leader_serial) + } + return { ok: true, leaders } } catch (err) { log.warn('getTeamLeaders failed', { externalId, message: err.message }) return refuse(`leadership unreadable: ${err.message}`) } } +/** + * Resolve each member's rank to a display label, keyed by serial. + * + * The shard sends the rank's NAME as the game states it — a cliloc id for the five + * standard ranks, or a literal string for a custom rank definition — and never a + * resolved label, because ServUO ships no text for those clilocs. This module does + * have a cliloc table, which is why the resolution belongs here. + * + * Three sources, in order: a custom string wins, then the operator's cliloc table, + * then the five standard names. The last exists because the cliloc table is + * populated only if someone ran the client extraction, and a shard that has not + * should still read "Warlord" rather than nothing. + * + * Never throws: a rank label is decoration on a roster, and a lookup failure must + * not turn a good roster into a refusal. + */ +async function rankLabels(rows) { + const out = new Map() + const wanted = [] + + for (const row of rows) { + if (row.rank_name) { + out.set(row.serial, row.rank_name) + } else if (Number.isInteger(row.rank_cliloc)) { + wanted.push(row.rank_cliloc) + } + } + + let resolved = new Map() + if (wanted.length) { + try { + resolved = await clilocs.resolveMany(wanted) + } catch (err) { + log.warn('rank cliloc lookup failed; falling back to the standard names', { message: err.message }) + } + } + + for (const row of rows) { + if (out.has(row.serial) || !Number.isInteger(row.rank_cliloc)) continue + const label = resolved.get(row.rank_cliloc) || STANDARD_RANK_NAMES[row.rank_cliloc] || null + if (label) out.set(row.serial, label) + } + return out +} + /** * The site account behind a character, or null. * diff --git a/server/test/teamProvider.test.js b/server/test/teamProvider.test.js index 4780b15..3305fb3 100644 --- a/server/test/teamProvider.test.js +++ b/server/test/teamProvider.test.js @@ -26,6 +26,7 @@ core.init({ const db = require('../model/teamProvider/teamProvider.db') const uoLinkConfig = require('../model/uoLinkConfig/uoLinkConfig.model') const uoLinkSocket = require('../utils/uoLinkSocket') +const clilocs = require('../model/shardClilocs/shardClilocs.model') const provider = require('../model/teamProvider/teamProvider.model') const saved = [] @@ -39,6 +40,10 @@ function patch(mod, name, fn) { function healthy() { patch(uoLinkConfig, 'getSafe', async () => ({ baseUrl: 'http://127.0.0.1:7787', enabled: true })) patch(uoLinkSocket, 'getState', () => ({ connected: true, running: true })) + // An operator who has never run the client extraction — the default. The standard + // rank names must still resolve from the fallback table. + patch(clilocs, 'resolveMany', async () => new Map()) + patch(db, 'listGuildLeaders', async () => []) } const guild = (extra = {}) => ({ @@ -48,6 +53,7 @@ const guild = (extra = {}) => ({ const member = (extra = {}) => ({ serial: '0x1', name: 'Aldric', acct: 'aldric', web_id: null, is_player: 1, + rank: 1, rank_cliloc: 1062962, rank_name: null, linked_user_id: null, is_online: 0, ...extra, }) @@ -167,8 +173,8 @@ test('the external id is a string, so core never compares a number to one', asyn test('a roster maps to the member shape core expects', async () => { patch(db, 'findGuild', async () => [guild()]) patch(db, 'listGuildMembers', async () => [ - member({ serial: '0x1', name: 'Aldric', is_online: 1 }), - member({ serial: '0x2', name: 'Bree', acct: null, is_online: 0 }), + member({ serial: '0x1', name: 'Aldric', rank: 4, rank_cliloc: 1062959, is_online: 1 }), + member({ serial: '0x2', name: 'Bree', acct: null, rank: 1, is_online: 0 }), ]) const { members } = await provider.getTeamMembers('1') @@ -176,21 +182,113 @@ test('a roster maps to the member shape core expects', async () => { assert.equal(members[0].memberKey, '0x1') assert.equal(members[0].displayName, 'Aldric') assert.equal(members[0].online, true) - assert.equal(members[0].leader, true, 'matches the board’s leader_serial') + assert.equal(members[0].leader, true, 'rank 4 is Leader') assert.equal(members[1].leader, false) assert.equal(members[1].online, false) }) -test('rankLabel is null, honestly — the wire carries no guild rank', async () => { - // The roster member is the standard actor object (serial, name, player, acct?, - // webId?). Inventing a rank from the leader flag would be core displaying a - // label this module made up. +// ── Rank (the Protocol 4 amendment) ──────────────────────────────────────── + +test('several members can be leaders at once', async () => { + // The whole reason the wire grew a per-member rank: the board carries one + // leader_serial, so before this only a single leader could ever be reported. patch(db, 'findGuild', async () => [guild()]) - patch(db, 'listGuildMembers', async () => [member()]) + patch(db, 'listGuildMembers', async () => [ + member({ serial: '0x1', rank: 4 }), + member({ serial: '0x2', rank: 4 }), + member({ serial: '0x3', rank: 3 }), + ]) + const { members } = await provider.getTeamMembers('1') + assert.deepEqual(members.filter((m) => m.leader).map((m) => m.memberKey), ['0x1', '0x2']) +}) + +test('getTeamLeaders returns everyone at rank 4, not just the board’s one', async () => { + patch(db, 'findGuild', async () => [guild({ leader_serial: '0x1' })]) + patch(db, 'listGuildLeaders', async () => [{ serial: '0x1' }, { serial: '0x2' }]) + assert.deepEqual((await provider.getTeamLeaders('1')).leaders, ['0x1', '0x2']) +}) + +test('the board’s leader is kept even when no roster row has rank yet', async () => { + // A shard whose roster has not been re-emitted since the amendment has no ranks + // stored. The founder-leader comes from a different frame and must not be lost + // by moving to ranks. + patch(db, 'findGuild', async () => [guild({ leader_serial: '0x9' })]) + patch(db, 'listGuildLeaders', async () => []) + assert.deepEqual((await provider.getTeamLeaders('1')).leaders, ['0x9']) +}) + +test('the board’s leader is not duplicated when they also hold rank 4', async () => { + patch(db, 'findGuild', async () => [guild({ leader_serial: '0x1' })]) + patch(db, 'listGuildLeaders', async () => [{ serial: '0x1' }, { serial: '0x2' }]) + const { leaders } = await provider.getTeamLeaders('1') + assert.equal(new Set(leaders).size, leaders.length) +}) + +test('a NULL rank is not a leader — "not known" is not "leads this guild"', async () => { + // The shard withholds the rank for a staff account, because ServUO's GuildRank + // getter reports Leader for anyone at GameMaster or above whatever their real + // rank. Reading the absence as leadership would republish exactly that lie. + patch(db, 'findGuild', async () => [guild()]) + patch(db, 'listGuildMembers', async () => [member({ serial: '0x1', rank: null, rank_cliloc: null })]) + + const { members } = await provider.getTeamMembers('1') + assert.equal(members[0].leader, false) assert.equal(members[0].rankLabel, null) }) +test('a standard rank resolves to its name without a cliloc table', async () => { + // The operator may never have run the client extraction, and a roster should + // still read "Warlord" rather than nothing. + patch(db, 'findGuild', async () => [guild()]) + patch(db, 'listGuildMembers', async () => [ + member({ serial: '0x1', rank: 4, rank_cliloc: 1062959 }), + member({ serial: '0x2', rank: 3, rank_cliloc: 1062960 }), + member({ serial: '0x3', rank: 0, rank_cliloc: 1062963 }), + ]) + + const { members } = await provider.getTeamMembers('1') + assert.deepEqual(members.map((m) => m.rankLabel), ['Leader', 'Warlord', 'Ronin']) +}) + +test('the operator’s cliloc table wins over the built-in names', async () => { + // A localised or edited client should name the ranks, not this module's English + // fallback. + patch(clilocs, 'resolveMany', async () => new Map([[1062960, 'Kriegsherr']])) + patch(db, 'findGuild', async () => [guild()]) + patch(db, 'listGuildMembers', async () => [member({ serial: '0x1', rank: 3, rank_cliloc: 1062960 })]) + + assert.equal((await provider.getTeamMembers('1')).members[0].rankLabel, 'Kriegsherr') +}) + +test('a custom rank’s literal name beats both', async () => { + // A shard that replaced RankDefinition.Ranks sends a string instead of a cliloc, + // and its own naming has to survive. + patch(clilocs, 'resolveMany', async () => new Map([[1062960, 'Warlord']])) + patch(db, 'findGuild', async () => [guild()]) + patch(db, 'listGuildMembers', async () => [ + member({ serial: '0x1', rank: 3, rank_cliloc: 1062960, rank_name: 'Sword-Captain' }), + ]) + + assert.equal((await provider.getTeamMembers('1')).members[0].rankLabel, 'Sword-Captain') +}) + +test('a failing cliloc lookup falls back rather than failing the roster', async () => { + patch(clilocs, 'resolveMany', async () => { throw new Error('cliloc table missing') }) + patch(db, 'findGuild', async () => [guild()]) + patch(db, 'listGuildMembers', async () => [member({ serial: '0x1', rank: 3, rank_cliloc: 1062960 })]) + + const answer = await provider.getTeamMembers('1') + assert.equal(answer.ok, true, 'a label is decoration; losing it must not lose the roster') + assert.equal(answer.members[0].rankLabel, 'Warlord') +}) + +test('an unknown cliloc leaves the label null rather than inventing one', async () => { + patch(db, 'findGuild', async () => [guild()]) + patch(db, 'listGuildMembers', async () => [member({ serial: '0x1', rank: 2, rank_cliloc: 9999999 })]) + assert.equal((await provider.getTeamMembers('1')).members[0].rankLabel, null) +}) + test('a member with no account at all is fine and unlinked', async () => { // §2.3 of the protocol spec: acct is genuinely optional — a PlayerMobile can // have no Account, and the local test world contains such mobiles. @@ -221,16 +319,9 @@ test('web_id arrives as a string from the wire and is coerced', async () => { assert.equal(typeof members[0].userId, 'number') }) -test('leadership is the board’s single leader — one, and honestly one', async () => { - // TEAMS.md §2.5 expects multiple leaders (GuildRank.Rank >= 4) and core - // supports them, but Protocol 4 does not put rank on the wire. Raising this to - // the full set is a protocol change, not something this file can fix. - patch(db, 'findGuild', async () => [guild({ leader_serial: '0x1' })]) - assert.deepEqual((await provider.getTeamLeaders('1')).leaders, ['0x1']) -}) - -test('a guild with no leader on the board reports none rather than guessing', async () => { +test('a guild with no leader anywhere reports none rather than guessing', async () => { patch(db, 'findGuild', async () => [guild({ leader_serial: null })]) + patch(db, 'listGuildLeaders', async () => []) const answer = await provider.getTeamLeaders('1') assert.equal(answer.ok, true) assert.deepEqual(answer.leaders, [])