Multiauth / SimpleStation RT#1696
Multiauth / SimpleStation RT#1696DEATHB4DEFEAT wants to merge 20 commits intoSimple-Station:masterfrom
Conversation
github-actions
bot
added
Changes: C#
DEATHB4DEFEAT
added
Priority: 2-High
|
This will require a test of the launcher and all associated systems before merging so that we don't accidentally break everything. |
…-Engines; branch 'master' of https://github.com/Simple-Station/Einstein-Engines into multiauth
Yeah I'll probably run a Parkstation event. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Undrafting since it seems to work, and for tests, still DNM though. |
|
Caution Review failedAn error occurred during the review process. Please try again later. WalkthroughThis update modifies configuration behavior and reference details across several components. The user data directory becomes dynamically determined via an environment variable in the replay module, while the authentication server selection in player lookup now processes a list to retrieve the first server’s URL. Additionally, the engine name in the documentation has been updated, and the subproject commit for RobustToolbox has been advanced to a new identifier. Changes
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
Content.Server/Administration/PlayerLocator.cs (1)
105-129: 🧹 Nitpick (assertive)Tactical recommendation: Consider extracting auth server logic
To maintain colonial code quality and reduce duplication, consider extracting the authentication server retrieval logic into a separate method:
+private string? GetAuthServerUrl() +{ + var servers = AuthServer.FromStringList(_configurationManager.GetCVar(CVars.AuthServers)); + if (!servers.Any()) + { + _sawmill.Error("No authentication servers configured!"); + return null; + } + return servers.First().AuthUrl; +}Then update both locations:
-var authServer = AuthServer.FromStringList(_configurationManager.GetCVar(CVars.AuthServers)).First().AuthUrl; +var authServer = GetAuthServerUrl(); +if (authServer == null) + return null;This refactoring will enhance code maintainability and enforce consistent error handling - a strategic advantage for the Biesel Republic!
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
Resources/Textures/WhiteDream/BloodCult/Entities/Items/shadowkin_shackles.rsi/icon.pngis excluded by!**/*.pngResources/Textures/WhiteDream/BloodCult/Entities/Items/shadowkin_shackles.rsi/inhand-left.pngis excluded by!**/*.pngResources/Textures/WhiteDream/BloodCult/Entities/Items/shadowkin_shackles.rsi/inhand-right.pngis excluded by!**/*.png
📒 Files selected for processing (4)
Content.Replay/Program.cs(1 hunks)Content.Server/Administration/PlayerLocator.cs(3 hunks)README.md(1 hunks)RobustToolbox(1 hunks)
🔇 Additional comments (3)
RobustToolbox (1)
1-1: New Submodule Commit Update – Steady as She Goes, Soldier!The subproject commit has been updated to
7d81e5d66e4755df42b7cb49ac3c585597d0769b, ensuring our vessel is powered by the latest version of RobustToolbox. This change is crucial for our multi-auth modernization efforts. Please verify that all dependent modules, especially those related to authentication and configuration, are fully compatible with this new commit.Content.Replay/Program.cs (1)
15-16: Mission Report: Environmental Variable Integration CompleteThe dynamic retrieval of the user data directory through environment variables represents a tactical improvement to our configuration systems. This change properly supports our multi-authentication initiative by allowing flexible data storage locations based on deployment environment.
The fallback to "SimpleStation14" provides appropriate redundancy protocols should the environment variable be unavailable. The code formatting is clean and follows proper Republic coding standards.
Glory to Biesel!
Content.Server/Administration/PlayerLocator.cs (1)
2-2: LGTM: System.Linq import is necessaryThe addition of the Linq namespace is a required modification to support the new authentication server selection mechanism. Strategic import!
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: DEATHB4DEFEAT <77995199+DEATHB4DEFEAT@users.noreply.github.com>
github-actions
bot
added
Changes: Localization
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Content.Server/Chat/Managers/ChatManager.cs (1)
252-256: Optimized string formatting needs documentation.This new message formatting logic implements our multi-authentication protocol - excellent tactical strategy! However, future engineers need to understand the reasoning behind this change.
Add a brief comment explaining why we're including the auth server in OOC messages:
+// Include authentication server in OOC messages to differentiate between players with the same name on different auth servers var wrappedMessage = Loc.GetString( "chat-manager-send-ooc-wrap-message", ("playerName", $"{player.Name}"), ("authServer", authServer), ("message", FormattedMessage.EscapeText(message)));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Content.Client/Administration/UI/CustomControls/PlayerListControl.xaml.cs(4 hunks)Content.Client/Administration/UI/CustomControls/PlayerListEntry.xaml.cs(2 hunks)Content.Server/Chat/Managers/ChatManager.cs(1 hunks)Resources/Locale/en-US/chat/managers/chat-manager.ftl(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
Content.Client/Administration/UI/CustomControls/PlayerListEntry.xaml.cs (1)
Content.Client/Administration/UI/CustomControls/PlayerListControl.xaml.cs (1)
GenerateTypedNameReferences(20-175)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Label
🔇 Additional comments (7)
Content.Server/Chat/Managers/ChatManager.cs (1)
264-269: Consistent logic applied for patron messages. Carry on, officer.The auth server information has been correctly implemented for patron message formatting, matching the standard message formatting. This maintains consistency across our communications protocols.
Content.Client/Administration/UI/CustomControls/PlayerListControl.xaml.cs (2)
27-28: Properly added new dependencies. Regulation-compliant.Clean addition of dependency fields for the player manager and configuration manager. Your code organization is ship-shape, engineer!
42-43: Dependencies correctly initialized.Dependencies properly resolved through the IoCManager. Standard naval procedure followed.
Content.Client/Administration/UI/CustomControls/PlayerListEntry.xaml.cs (2)
17-18: Class dependencies added correctly.These dependency fields follow the standard Biesel Republic naval coding pattern. Good work, recruit!
23-24: Dependencies properly initialized.IoCManager resolution follows standard protocol. Cleared for duty!
Resources/Locale/en-US/chat/managers/chat-manager.ftl (2)
42-45: OOC message formatting handles authentication elegantly.The localization pattern using FTL's selective branching for handling different auth server cases is tactically sound. This maintains clean formatting whether auth info is present or not.
46-49: Consistent patron message formatting. Well done, officer.The multi-authentication formatting is consistently applied to patron messages as well. This attention to detail honors the Biesel Republic's standards!
Content.Client/Administration/UI/CustomControls/PlayerListControl.xaml.cs
Show resolved
Hide resolved
* better abductors * apparently the component handles the use of teleportation console and the agent should have it until we implement a better system like in ss13 (cherry picked from commit c5fea20e4f71c7685a62d6846724d94def2a66d7)
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
* add supermatter * Delete RobustToolbox * change robusttoolbox to supermatter * chore: Automatically update REUSE headers * chore: Automatically update REUSE headers * Revert "change robusttoolbox to supermatter" This reverts commit 36f0524. * renaming of engine I'm not fucking with renaming EVERYTHING in repo, related to hardcoded engine name * Update .gitmodules * Update .gitmodules * Update RobustToolbox * changes taken from Simple-Station/Einstein-Engines#1696 * chore: Automatically update REUSE headers * chore: Automatically update REUSE headers * change from GetSessionById to safer TryGetSessionById * Add null checks for player session data in admin UI * Fix NRE in player list when session data is incomplete i hate and love integration tests * another try to fix NRE * and another one * and AGAIN * sawmill fix * delete stupid reuse * test rollback * PlayerListControl.xaml.cs rollback Will (or not) update later * delete stupid reuse --------- Co-authored-by: ReserveBot <211949879+ReserveBot@users.noreply.github.com>
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
good luck |
4 participants
Description
Requires Simple-Station/SupermatterEngine#3
TODO
Media
Changelog
🆑