-
Notifications
You must be signed in to change notification settings - Fork 138
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
Improve migration to eosIDs #354
Merged
werewolfboy13
merged 10 commits into
Team-Silver-Sphere:master
from
Ulibos:fix/migrate-to-eosID
Jul 3, 2024
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
f68f8f3
ID parser implemented.
Ulibos 0a8332f
Cleanup player join logging.
Ulibos ad1f9e4
Rework admins features to be able to recieve any ID as an identifier.
Ulibos 8a2849e
Migrate core code to eosID and anyID.
Ulibos cc3fce9
Update plugins to eosID.
Ulibos b506205
Fix attemts to access non-existent oldPlayerInfo.
Ulibos 7b298a7
Refactor id-parser.
Ulibos a78036b
refactor: getAdminsWithPermission extended.
Ulibos cbe29e5
Merge branch 'master' into fix/migrate-to-eosID
Ulibos 07be06f
Add safeguerds for `INVALID` IDs.
Ulibos File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
const ID_MATCHER = /\s*(?<name>[^\s:]+)\s*:\s*(?<id>[^\s]+)/g; | ||
|
||
// COMMON CONSTANTS | ||
|
||
/** All possible IDs that a player can have. */ | ||
export const playerIdNames = ["steamID", "eosID"]; | ||
|
||
// PARSING AND ITERATION | ||
|
||
/** | ||
* Main function intended for parsing `Online IDs:` body. | ||
* @arg {string} idsStr - String with ids. Extra whitespace is allowed, | ||
* Number of {platform: ID} pairs can be arbitrary. String example: | ||
" platform1:id1 platform2: id2 platform3 : id3 " | ||
Keys and values are not allowed contain colons or whitespace | ||
characters. | ||
* @returns {IdsIterator} An iterator that yields {platform: ID} pairs. | ||
*/ | ||
export const iterateIDs = (idsStr) => { | ||
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 | ||
|
||
/** | ||
* Generates capitalized ID names. Examples: | ||
* steam -> SteamID | ||
* EOSID -> EOSID | ||
*/ | ||
export const capitalID = (str) => { | ||
return str.charAt(0).toUpperCase() + str.slice(1) + 'ID'; | ||
}; | ||
|
||
/** | ||
* Generates lowercase ID names. Examples: | ||
* steam -> steamID | ||
* EOSID -> eosID | ||
*/ | ||
export const lowerID = (str) => { | ||
return str.toLowerCase() + 'ID'; | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 ofController
.There was a problem hiding this comment.
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 togetAdminsWithPermission(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 inmaster
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.There was a problem hiding this comment.
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.