Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve migration to eosIDs #354

Merged
merged 10 commits into from
Jul 3, 2024
Merged

Improve migration to eosIDs #354

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f68f8f3
ID parser implemented.
Ulibos Mar 26, 2024
0a8332f
Cleanup player join logging.
Ulibos Apr 13, 2024
ad1f9e4
Rework admins features to be able to recieve any ID as an identifier.
Ulibos Apr 13, 2024
8a2849e
Migrate core code to eosID and anyID.
Ulibos Apr 14, 2024
cc3fce9
Update plugins to eosID.
Ulibos Apr 14, 2024
b506205
Fix attemts to access non-existent oldPlayerInfo.
Ulibos Apr 14, 2024
7b298a7
Refactor id-parser.
Ulibos Apr 15, 2024
a78036b
refactor: getAdminsWithPermission extended.
Ulibos May 17, 2024
cbe29e5
Merge branch 'master' into fix/migrate-to-eosID
Ulibos Jun 18, 2024
07be06f
Add safeguerds for `INVALID` IDs.
Ulibos Jun 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions core/id-parser.js
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally don't like the approach of having a "unified" regex to match the IDs because it depends only on the hope of OWI not breaking things with typos or non-standard formats, which has already happened, an example is a log line which has Contoller instead of Controller.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All suggestions are implemented apart from one: changing getAdminsWithPermission(perm) to return players instead of IDs is not yet implemented. I'm all for implementing it, just want to make sure it is greenlit. Or, as a compromise, we could extend it to getAdminsWithPermission(perm, returnPlayers=false), that would allow for the new feature without potentially breaking custom plugins.
As for unified regex, I kindly disagree. master implementation is prone to: changing ordering, extra spacing, lack of steamID (some regexes in master were fixed to have steam as an optional but not all), renaming a key. This implementation will break only if delimiters are broken. Swapping, cloning, removing arbitrary k:v pairs, introducing new IDs, surrounding colons with spaces won't break it. Renaming a key will break things down the line but not the parser itself. And when OWI eventually breaks some output again - we can patch that specific item within it's parser, while everything else follows the same logic and is easy to remember.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with the new changes after my first review, and with keeping your implementation of the ID parser since you proved to have went through an analysis of all the side effects of both your implementation and the one currently on the master branch.

Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
const ID_MATCHER = /\s*(?<name>[^:]+): (?<id>[^\s]+)/g;

// PARSING AND ITERATION

// main function intended for parsing `Online IDs:` body. Returns an
// iterator that yields `{platform: id}` pairs.
export const iterate = (idsStr) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be renamed to a more descriptive name such as iterateIDs

return new IdsIterator(idsStr.matchAll(ID_MATCHER));
};

class IdsIterator {
constructor(matchIterator) {
this.inner = matchIterator;
}

[Symbol.iterator]() {
return this;
}

next() {
const match = this.inner.next();
if (match.done) return { value: undefined, done: true };
return { value: { key: match.value[1], value: match.value[2] }, done: false };
}

forEach(callbackFn) {
for (const { key, value } of this) callbackFn(key, value);
}
}

// FORMATTING

// `steam => SteamID`
export const capitalID = (str) => {
return str.charAt(0).toUpperCase() + str.slice(1) + 'ID';
};

// `EOS => eosID`
export const lowerID = (str) => {
return str.toLowerCase() + 'ID';
};
14 changes: 5 additions & 9 deletions core/log-parser/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,8 @@ export default class LogParser extends EventEmitter {

this.eventStore = {
disconnected: {}, // holding area, cleared on map change.
players: [], // persistent data, steamid, controller, suffix.
playersEOS: [], // proxies from EOSID to persistent data, steamid, controller, suffix.
connectionIdToSteamID: new Map(),
players: {}, // persistent data, steamid, controller, suffix.
session: {}, // old eventstore, nonpersistent data
clients: {}, // used in the connection chain before we resolve a player.
lastConnection: {}, // used to store the last client connection data to then associate a steamid
joinRequests: []
};

Expand Down Expand Up @@ -74,10 +70,10 @@ export default class LogParser extends EventEmitter {
clearEventStore() {
Logger.verbose('LogParser', 2, 'Cleaning Eventstore');
for (const player of Object.values(this.eventStore.players)) {
if (this.eventStore.disconnected[player.steamID] === true) {
Logger.verbose('LogParser', 2, `Removing ${player.steamID} from eventStore`);
delete this.eventStore.players[player.steamID];
delete this.eventStore.disconnected[player.steamID];
if (this.eventStore.disconnected[player.eosID] === true) {
Logger.verbose('LogParser', 2, `Removing ${player.eosID} from eventStore`);
delete this.eventStore.players[player.eosID];
delete this.eventStore.disconnected[player.eosID];
}
}
this.eventStore.session = {};
Expand Down
1 change: 1 addition & 0 deletions core/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"exports": {
"./log-parser": "./log-parser/index.js",
"./constants": "./constants.js",
"./id-parser": "./id-parser.js",
"./logger": "./logger.js",
"./rcon": "./rcon.js"
},
Expand Down
12 changes: 6 additions & 6 deletions core/rcon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,15 +414,15 @@ export default class Rcon extends EventEmitter {
return util.inspect(decodedPacket, { breakLength: Infinity });
}

async warn(steamID, message) {
await this.execute(`AdminWarn "${steamID}" ${message}`);
async warn(anyID, message) {
await this.execute(`AdminWarn "${anyID}" ${message}`);
}

async kick(steamID, reason) {
await this.execute(`AdminKick "${steamID}" ${reason}`);
async kick(anyID, reason) {
await this.execute(`AdminKick "${anyID}" ${reason}`);
}

async forceTeamChange(steamID) {
await this.execute(`AdminForceTeamChange "${steamID}"`);
async forceTeamChange(anyID) {
await this.execute(`AdminForceTeamChange "${anyID}"`);
}
}
97 changes: 50 additions & 47 deletions squad-server/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import Rcon from './rcon.js';
import { SQUADJS_VERSION } from './utils/constants.js';

import fetchAdminLists from './utils/admin-lists.js';
import { anyIDToPlayer, anyIDsToPlayers } from './utils/any-id.js';

export default class SquadServer extends EventEmitter {
constructor(options = {}) {
Expand Down Expand Up @@ -98,7 +99,7 @@ export default class SquadServer extends EventEmitter {
});

this.rcon.on('CHAT_MESSAGE', async (data) => {
data.player = await this.getPlayerBySteamID(data.steamID);
data.player = await this.getPlayerByEOSID(data.eosID);
this.emit('CHAT_MESSAGE', data);

const command = data.message.match(/!([^ ]+) ?(.*)/);
Expand All @@ -110,22 +111,22 @@ export default class SquadServer extends EventEmitter {
});

this.rcon.on('POSSESSED_ADMIN_CAMERA', async (data) => {
data.player = await this.getPlayerBySteamID(data.steamID);
data.player = await this.getPlayerByEOSID(data.eosID);

this.adminsInAdminCam[data.steamID] = data.time;
this.adminsInAdminCam[data.eosID] = data.time;

this.emit('POSSESSED_ADMIN_CAMERA', data);
});

this.rcon.on('UNPOSSESSED_ADMIN_CAMERA', async (data) => {
data.player = await this.getPlayerBySteamID(data.steamID);
if (this.adminsInAdminCam[data.steamID]) {
data.duration = data.time.getTime() - this.adminsInAdminCam[data.steamID].getTime();
data.player = await this.getPlayerByEOSID(data.eosID);
if (this.adminsInAdminCam[data.eosID]) {
data.duration = data.time.getTime() - this.adminsInAdminCam[data.eosID].getTime();
} else {
data.duration = 0;
}

delete this.adminsInAdminCam[data.steamID];
delete this.adminsInAdminCam[data.eosID];

this.emit('UNPOSSESSED_ADMIN_CAMERA', data);
});
Expand All @@ -141,13 +142,13 @@ export default class SquadServer extends EventEmitter {
});

this.rcon.on('PLAYER_KICKED', async (data) => {
data.player = await this.getPlayerBySteamID(data.steamID);
data.player = await this.getPlayerByEOSID(data.eosID);

this.emit('PLAYER_KICKED', data);
});

this.rcon.on('PLAYER_BANNED', async (data) => {
data.player = await this.getPlayerBySteamID(data.steamID);
data.player = await this.getPlayerByEOSID(data.eosID);

this.emit('PLAYER_BANNED', data);
});
Expand All @@ -157,8 +158,7 @@ export default class SquadServer extends EventEmitter {
data.player.squadID = data.squadID;

delete data.playerName;
delete data.playerSteamID;
delete data.playerEOSID;
for (const k in data) if (k.startsWith('player') && k.endsWith('ID')) delete data[k];

this.emit('SQUAD_CREATED', data);
});
Expand Down Expand Up @@ -208,7 +208,7 @@ export default class SquadServer extends EventEmitter {
this.emit('NEW_GAME', data);
});

this.logParser.on('PLAYER_CONNECTED', async (data) => {
this.logParser.on('JOIN_SUCCEEDED', async (data) => {
Logger.verbose(
'SquadServer',
1,
Expand All @@ -218,7 +218,7 @@ export default class SquadServer extends EventEmitter {
data.player = await this.getPlayerByEOSID(data.eosID);
if (data.player) data.player.suffix = data.playerSuffix;

delete data.steamID;
for (const k in data) if (k.endsWith('ID')) delete data[k];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be more specific:

for (const k in data) if (["steamID", "eosID"].includes(k)) delete data[k];

delete data.playerSuffix;

this.emit('PLAYER_CONNECTED', data);
Expand All @@ -227,7 +227,7 @@ export default class SquadServer extends EventEmitter {
this.logParser.on('PLAYER_DISCONNECTED', async (data) => {
data.player = await this.getPlayerByEOSID(data.eosID);

delete data.steamID;
for (const k in data) if (k.endsWith('ID')) delete data[k];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be more specific:

for (const k in data) if (["steamID", "eosID"].includes(k)) delete data[k];


this.emit('PLAYER_DISCONNECTED', data);
});
Expand All @@ -242,7 +242,7 @@ export default class SquadServer extends EventEmitter {
if (data.victim && data.attacker) {
data.teamkill =
data.victim.teamID === data.attacker.teamID &&
data.victim.steamID !== data.attacker.steamID;
data.victim.eosID !== data.attacker.eosID;
}

delete data.victimName;
Expand All @@ -260,7 +260,7 @@ export default class SquadServer extends EventEmitter {
if (data.victim && data.attacker)
data.teamkill =
data.victim.teamID === data.attacker.teamID &&
data.victim.steamID !== data.attacker.steamID;
data.victim.eosID !== data.attacker.eosID;

delete data.victimName;
delete data.attackerName;
Expand All @@ -278,7 +278,7 @@ export default class SquadServer extends EventEmitter {
if (data.victim && data.attacker)
data.teamkill =
data.victim.teamID === data.attacker.teamID &&
data.victim.steamID !== data.attacker.steamID;
data.victim.eosID !== data.attacker.eosID;

delete data.victimName;
delete data.attackerName;
Expand Down Expand Up @@ -322,23 +322,6 @@ export default class SquadServer extends EventEmitter {
this.logParser.on('TICK_RATE', (data) => {
this.emit('TICK_RATE', data);
});

this.logParser.on('CLIENT_EXTERNAL_ACCOUNT_INFO', (data) => {
this.rcon.addIds(data.steamID, data.eosID);
});
// this.logParser.on('CLIENT_CONNECTED', (data) => {
// Logger.verbose("SquadServer", 1, `Client connected. Connection: ${data.connection} - SteamID: ${data.steamID}`)
// })
// this.logParser.on('CLIENT_LOGIN_REQUEST', (data) => {
// Logger.verbose("SquadServer", 1, `Login request. ChainID: ${data.chainID} - Suffix: ${data.suffix} - EOSID: ${data.eosID}`)

// })
// this.logParser.on('RESOLVED_EOS_ID', (data) => {
// Logger.verbose("SquadServer", 1, `Resolved EOSID. ChainID: ${data.chainID} - Suffix: ${data.suffix} - EOSID: ${data.eosID}`)
// })
// this.logParser.on('ADDING_CLIENT_CONNECTION', (data) => {
// Logger.verbose("SquadServer", 1, `Adding client connection`, data)
// })
}

async restartLogParser() {
Expand All @@ -353,16 +336,26 @@ export default class SquadServer extends EventEmitter {
await this.logParser.watch();
}

// DEPRECATED FUNCTION
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be replaced with a more suitable jsdoc

/**
 * @deprecated <optional message here>
 */

getAdminPermsBySteamID(steamID) {
return this.admins[steamID];
return this.getAdminPermsByAnyID(steamID);
}

getAdminPermsByAnyID(anyID) {
// using this.players directly to keep the function sync
const player = anyIDToPlayer(anyID, this.players);
if (player === undefined) return;
for (const parm in player)
if (parm.endsWith("ID") && player[parm] in this.admins)
return this.admins[player[parm]];
}

getAdminsWithPermission(perm) {
const ret = [];
for (const [steamID, perms] of Object.entries(this.admins)) {
if (perm in perms) ret.push(steamID);
for (const [anyID, perms] of Object.entries(this.admins)) {
if (perm in perms) ret.push(anyID);
}
return ret;
return anyIDsToPlayers(ret, this.players).map(player => player.eosID);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

considering that this is a breaking change, and it also limits the functionality from getting ALL the admins to ONLY THE ONLINE admins, I would prefer this to be converted to returning an array of objects with both eosID and steamID optional parameters.
this should be further discussed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed on Discord with @Ulibos, the best choice appears to be a method that accepts an extra parameter to tune the output based on the needs.

A jsdoc would be a good fit for defining all the possible values for the extra parameter that I will just refer to as "output format parameter".

The output format parameter could be defined with the following jsdoc: @param {'steamID'|'eosID'|'any'|'player'} and will return the output formatted as showed in the following example:

  • steamID: The output stays the same as the current implementation. Example: [ '01234567891234567', '01234567891234567' ]
  • eosID: Example: [ 'a1b2c3d4e5f6a7b8c9d1e2f3a4b5c6d7e8f9', 'a1b2c3d4e5f6a7b8c9d1e2f3a4b5c6d7e8f9' ]
  • any: The eosID should be the preferred ID but include a fallback to steamID in case of missing eosID. Example [ 'a1b2c3d4e5f6a7b8c9d1e2f3a4b5c6d7e8f9', '01234567891234567' ]
  • player: [ { name: 'example_name', eosID: 'a1b2c3d4e5f6a7b8c9d1e2f3a4b5c6d7e8f9', steamID: '01234567891234567', ... }, { name: 'example_name', eosID: 'a1b2c3d4e5f6a7b8c9d1e2f3a4b5c6d7e8f9', steamID: null|undefined, ... } ]

}

async updateAdmins() {
Expand All @@ -377,34 +370,35 @@ export default class SquadServer extends EventEmitter {
try {
const oldPlayerInfo = {};
for (const player of this.players) {
oldPlayerInfo[player.steamID] = player;
oldPlayerInfo[player.eosID] = player;
}

const players = [];
for (const player of await this.rcon.getListPlayers(this))
players.push({
...oldPlayerInfo[player.steamID],
...oldPlayerInfo[player.eosID],
...player,
playercontroller: this.logParser.eventStore.players[player.steamID]
? this.logParser.eventStore.players[player.steamID].controller
playercontroller: this.logParser.eventStore.players[player.eosID]
? this.logParser.eventStore.players[player.eosID].controller
: null,
squad: await this.getSquadByID(player.teamID, player.squadID)
});

this.players = players;

for (const player of this.players) {
if (typeof oldPlayerInfo[player.steamID] === 'undefined') continue;
if (player.teamID !== oldPlayerInfo[player.steamID].teamID)
const oldInfo = oldPlayerInfo[player.eosID];
if (oldInfo === undefined) continue;
if (player.teamID !== oldInfo.teamID)
this.emit('PLAYER_TEAM_CHANGE', {
player: player,
oldTeamID: oldPlayerInfo[player.steamID].teamID,
oldTeamID: oldInfo.teamID,
newTeamID: player.teamID
});
if (player.squadID !== oldPlayerInfo[player.steamID].squadID)
if (player.squadID !== oldInfo.squadID)
this.emit('PLAYER_SQUAD_CHANGE', {
player: player,
oldSquadID: oldPlayerInfo[player.steamID].squadID,
oldSquadID: oldInfo.squadID,
newSquadID: player.squadID
});
}
Expand Down Expand Up @@ -591,6 +585,7 @@ export default class SquadServer extends EventEmitter {
);
}

// DEPRECATED
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be replaced with a more suitable jsdoc

/**
 * @deprecated <optional message here>
 */

async getPlayerBySteamID(steamID, forceUpdate) {
return this.getPlayerByCondition((player) => player.steamID === steamID, forceUpdate);
}
Expand All @@ -599,6 +594,14 @@ export default class SquadServer extends EventEmitter {
return this.getPlayerByCondition((player) => player.eosID === eosID, forceUpdate);
}

async getPlayerByAnyID(anyID, forceUpdate) {
return this.getPlayerByCondition(
(player) =>
Object.entries(player).filter(([parm, val]) => parm.endsWith('ID') && val === anyID).length,
forceUpdate
);
}

async getPlayerByName(name, forceUpdate) {
return this.getPlayerByCondition((player) => player.name === name, forceUpdate);
}
Expand Down
20 changes: 0 additions & 20 deletions squad-server/log-parser/adding-client-connection.js
View file
Open in desktop

This file was deleted.

15 changes: 0 additions & 15 deletions squad-server/log-parser/check-permission-resolve-eosid.js
View file
Open in desktop

This file was deleted.

Loading