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.

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

Update admin list utility to recognise eosIDs #347

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

IgnisAlienus
Copy link
Contributor

So, I updated my admins.cfg to have EOSIDs for my Admins after that whole EOS Update, guess SquadJS hasn't been pulling them this whole time. Now it does.

Copy link
Contributor

@fantinodavide fantinodavide left a comment

Choose a reason for hiding this comment

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

Changes look good to me. Approved

@Thomas-Smyth
Copy link
Collaborator

How does this work when utilising the list? A lot of the Server class seems to search the list of admins by Steam IDs still, e.g.:

return this.admins[steamID];

@Ulibos
Copy link
Contributor

Ulibos commented Apr 4, 2024

@Thomas-Smyth I'm building a full migration to eosIDs. I'm planning to implement a mapping function that will convert steamIDs of online admins into eosIDs (non-matching IDs will be ignored instead of failing). This will allow us to support old steamID-based admin lists while people shift their stuff to eosIDs. My future PR builds upon this PR and my own: #352

@fantinodavide
Copy link
Contributor

How does this work when utilising the list? A lot of the Server class seems to search the list of admins by Steam IDs still, e.g.:

return this.admins[steamID];

it accepts both steamID and eosID

@Thomas-Smyth
Copy link
Collaborator

How does this work when utilising the list? A lot of the Server class seems to search the list of admins by Steam IDs still, e.g.:

return this.admins[steamID];

it accepts both steamID and eosID

How? If it's always looking up by Steam ID then it will never match on a EOSID.

@fantinodavide
Copy link
Contributor

How does this work when utilising the list? A lot of the Server class seems to search the list of admins by Steam IDs still, e.g.:

return this.admins[steamID];

it accepts both steamID and eosID

How? If it's always looking up by Steam ID then it will never match on a EOSID.

People will have to adjust their plugins in order to use eosID, but this PR improves how the admin lists are parsed and won't skip the lines with an eosID only, and doesn't make a specific discrimination between steamID and eosID

@Ulibos
Copy link
Contributor

Ulibos commented Apr 4, 2024

How? If it's always looking up by Steam ID then it will never match on a EOSID.

If an ID is used directly in rcon commands then it will work regardless. There are 2 plugins that will ignore eosIDs because they loop through players matching admin IDs against steamID:

  1. auto-kick-unassigned.js
  2. discord-admin-request.js

But I'm planning to address this in my next PR.

@werewolfboy13
Copy link
Collaborator

Pending further review.

@werewolfboy13 werewolfboy13 added core bug Bug related to the core SquadJS API minor Minor Change labels Jul 3, 2024
@werewolfboy13 werewolfboy13 merged commit e901403 into Team-Silver-Sphere:master Jul 3, 2024
@Thomas-Smyth Thomas-Smyth changed the title Update: Admin List Util to Recognize EOSIDs Update admin list utility to recognise eosIDs Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core bug Bug related to the core SquadJS API minor Minor Change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants