Basic xbox support #126
Basic xbox support #126
Conversation
ShiralynDev
left a comment
ShiralynDev
left a comment
There was a problem hiding this comment.
I don't think I ended up using userID and platform since the data types use the ControlledBy[XBOX/Steam]ID and therefor I think I could remove it but I will have to look at it again just to make sure.
There was a problem hiding this comment.
Pull request overview
This PR adds basic Xbox platform support to prevent crashes on Xbox servers and displays Xbox server flags in the menu. Xbox users are shown as "Unknown [XBOX]" in the UI until further integration is completed.
Changes:
- Updated @simrail/types dependency to version 0.0.5 for Xbox type support
- Added Xbox server configurations (XBX1, XBX2, XBX3) with flag mappings
- Modified user handling logic to support both Steam and Xbox IDs across components
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated @simrail/types dependency to support Xbox types |
| packages/map/components/serverTimes.json | Added server time configurations for three Xbox servers |
| packages/map/pages/index.tsx | Added logic to handle Xbox server codes and map them to appropriate flag icons |
| packages/map/components/SpotlightSearch.tsx | Extended user class and search functionality to handle both Steam and Xbox user IDs |
| packages/map/components/SelectedTrainPopup.tsx | Added TODO comment for Xbox player support |
| packages/map/components/Markers/StationMarker.tsx | Added Xbox user handling for dispatchers with string comparison checks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| if (stations[i].DispatchedBy[0] && stations[i].DispatchedBy[0].XboxId != null) { | ||
| userIDs.push(stations[i].DispatchedBy[0].XboxId); | ||
| platforms.push("xbox") |
There was a problem hiding this comment.
Missing semicolon at the end of the statement. This is inconsistent with the code style used elsewhere in the file (e.g., line 68).
| } | ||
|
|
||
| if (train.TrainData.ControlledByXboxID) { | ||
| username = "Unknown [XBOX]" // make function for getting xbox usernames |
There was a problem hiding this comment.
Missing semicolon at the end of the statement. This is inconsistent with the code style used elsewhere in the file.
| } | ||
|
|
||
| if (station.DispatchedBy[0] && station.DispatchedBy[0].XboxId) { | ||
| username = "Unknown [XBOX]" // make function for getting xbox usernames https://panel.simrail.eu:8084/users-open/ |
There was a problem hiding this comment.
Missing semicolon at the end of the statement. This is inconsistent with the code style used elsewhere in the file.
| (() => { | ||
| const letters = | ||
| server.ServerCode.match(/[A-Za-z]+/)?.at(0) ?? | ||
| server.ServerCode.slice(0, 2); | ||
| if (letters.toUpperCase() === "XBX") { | ||
| const numMatch = server.ServerCode.match(/^XBX(\d+)/i); | ||
| return numMatch ? `XBX${numMatch[1]}` : "XBX"; | ||
| } | ||
| return letters; | ||
| })() |
There was a problem hiding this comment.
This IIFE logic for handling Xbox server codes adds complexity inline. Consider extracting this into a separate function like getServerFlagCode(serverCode: string) to improve readability and testability.
There was a problem hiding this comment.
it would add complexity by having to make sure the serverID always sends the number with it
| if (lang === "XBX1") lang = "PL"; | ||
| if (lang === "XBX2") lang = "INT"; | ||
| if (lang === "XBX3") lang = "INT"; | ||
| if (lang === "EN") lang = "GB"; |
There was a problem hiding this comment.
These individual if statements for mapping Xbox codes could be simplified using a switch statement or an object lookup for better maintainability as more Xbox servers are added.
| if (lang === "XBX1") lang = "PL"; | |
| if (lang === "XBX2") lang = "INT"; | |
| if (lang === "XBX3") lang = "INT"; | |
| if (lang === "EN") lang = "GB"; | |
| const langOverrides: Record<string, string> = { | |
| XBX1: "PL", | |
| XBX2: "INT", | |
| XBX3: "INT", | |
| EN: "GB", | |
| }; | |
| if (langOverrides[lang]) { | |
| lang = langOverrides[lang]; | |
| } |
There was a problem hiding this comment.
Im following the style of the current code
Co-authored-by: Copilot <[email protected]>
📝 WalkthroughWalkthroughAdded Xbox platform support across UI lookup and rendering: dependency bump, platform-aware user/profile lookup (Steam vs Xbox), Xbox server entries, and flag/locale mapping for Xbox servers. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/map/components/SelectedTrainPopup.tsx (1)
29-35: Xbox players are not actually handled here.The comment states "support xbox players" but this code only calls
getSteamProfileOrBot, which returns "BOT" whenControlledBySteamIDis null/undefined. For Xbox players, this will incorrectly show "BOT" instead of "Unknown [XBOX]" as displayed inStationMarker.tsxandSpotlightSearch.tsx.To maintain consistency across the codebase, consider adding Xbox ID handling similar to the other components.
Suggested fix
useEffect(() => { - if (selectedTrain) - getSteamProfileOrBot(selectedTrain.TrainData.ControlledBySteamID).then( // support xbox players - // @ts-ignore - setData, - ); + if (selectedTrain) { + if (selectedTrain.TrainData.ControlledByXboxID) { + setData([null, "Unknown [XBOX]"]); + } else { + getSteamProfileOrBot(selectedTrain.TrainData.ControlledBySteamID).then( + // @ts-ignore + setData, + ); + } + } }, [selectedTrain]);
🤖 Fix all issues with AI agents
In @packages/map/components/Markers/StationMarker.tsx:
- Around line 24-31: The conditional in StationMarker.tsx uses non-strict
inequality (station.DispatchedBy[0].SteamId != "null"); replace it with strict
inequality (!==) to avoid type-coercion issues — update the if check around the
fetch/profile logic (the block that calls
fetch(...steam/${station.DispatchedBy[0].SteamId}), awaits avatarRequest.json(),
and calls setAvatar/setUsername) to use !== "null".
- Around line 32-35: The code uses a non-strict inequality and may
unintentionally override Steam data: replace the `!=` check on
station.DispatchedBy[0].XboxId with strict `!==`, and change the conditional
structure so Xbox handling does not unconditionally override Steam handling (use
`else if` or explicitly document precedence) modifying the logic around
setAvatar and setUsername in the StationMarker component to preserve mutual
exclusivity or clarify intent.
In @packages/map/components/SpotlightSearch.tsx:
- Around line 60-70: In the for-loop inside SpotlightSearch
(SpotlightSearch.tsx) that iterates over trains, a train can currently push both
ControlledBySteamID and ControlledByXboxID which creates duplicate/misaligned
entries in the parallel arrays userIDs and platforms; update the logic in that
loop (the block checking trains[i].Type === "user") to treat the two checks as
mutually exclusive (use an else if or equivalent) so only one ID/platform is
pushed per train, keeping the null checks (trains[i] != null and TrainData.* !=
null) intact to preserve alignment between userIDs and platforms.
- Around line 72-81: In SpotlightSearch.tsx inside the loop over stations, the
logic pushing IDs pushes both Steam and Xbox for the same station; change the
second conditional to an else if so only one ID is pushed per station (e.g.,
keep the existing check for stations[i].DispatchedBy[0] &&
stations[i].DispatchedBy[0].SteamId != null then push Steam, and use else if
(stations[i].DispatchedBy[0] && stations[i].DispatchedBy[0].XboxId != null) to
push Xbox), updating the userIDs and platforms arrays accordingly to enforce
mutual exclusivity.
- Around line 120-124: The code is accessing station.DispatchedBy[0].SteamId
without optional chaining; replace that block to safely handle missing
arrays/entries by first extracting steamId with optional chaining (e.g., const
steamId = station.DispatchedBy?.[0]?.SteamId) and then set username using
usernamesCache.current.get(steamId) ?? "Unknown", or if steamId is undefined
just assign "Unknown"; update the logic around the username variable in
SpotlightSearch (the station.DispatchedBy/SteamId access and
usernamesCache.current.get call) to use optional chaining and nullish coalescing
to satisfy the linter.
- Around line 126-128: Replace the manual null-checks with optional chaining:
update the conditional that currently checks station.DispatchedBy[0] and
station.DispatchedBy[0].XboxId to use station.DispatchedBy?.[0]?.XboxId, leaving
the username assignment to "Unknown [XBOX]" in the same block; reference the
station variable and the username assignment in SpotlightSearch.tsx when making
this change.
🧹 Nitpick comments (2)
packages/map/components/Markers/StationMarker.tsx (1)
25-30: Missing error handling for fetch.If the Steam profile fetch fails (network error, non-2xx response, or invalid JSON), the promise rejection will be unhandled and may cause silent failures. Consider wrapping in try-catch.
Suggested fix
if (station.DispatchedBy[0].SteamId !== "null") { - const avatarRequest = await fetch( - `https://simrail-edr.emeraldnetwork.xyz/steam/${station.DispatchedBy[0].SteamId}`, - ); - const profile: ProfileResponse = await avatarRequest.json(); - setAvatar(profile.avatar); - setUsername(profile.personaname); + try { + const avatarRequest = await fetch( + `https://simrail-edr.emeraldnetwork.xyz/steam/${station.DispatchedBy[0].SteamId}`, + ); + const profile: ProfileResponse = await avatarRequest.json(); + setAvatar(profile.avatar); + setUsername(profile.personaname); + } catch (error) { + console.error("Failed to fetch Steam profile:", error); + setUsername("Unknown"); + } }packages/map/pages/index.tsx (1)
181-184: Consider consolidating flag mappings into an object.The sequential
ifstatements work but could be more maintainable as an object lookup, especially if more Xbox servers are added in the future.Optional refactor
let lang = code.toUpperCase(); - - if (lang === "XBX1") lang = "PL"; - if (lang === "XBX2") lang = "INT"; - if (lang === "XBX3") lang = "INT"; + const flagOverrides: Record<string, string> = { + XBX1: "PL", + XBX2: "INT", + XBX3: "INT", + EN: "GB", + }; + lang = flagOverrides[lang] ?? lang; if (lang === "EN") lang = "GB";
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
package.jsonpackages/map/components/Markers/StationMarker.tsxpackages/map/components/SelectedTrainPopup.tsxpackages/map/components/SpotlightSearch.tsxpackages/map/components/serverTimes.jsonpackages/map/pages/index.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/map/components/SelectedTrainPopup.tsx (1)
packages/map/components/steam.ts (1)
getSteamProfileOrBot(8-16)
🪛 GitHub Actions: Code quality
packages/map/components/Markers/StationMarker.tsx
[warning] 24-24: lint/suspicious/noDoubleEquals: Use !== instead of !=
[warning] 32-32: lint/suspicious/noDoubleEquals: Use !== instead of !=
packages/map/components/SpotlightSearch.tsx
[warning] 120-120: lint/complexity/useOptionalChain: Change to an optional chain.
[warning] 126-126: lint/complexity/useOptionalChain: Change to an optional chain.
🔇 Additional comments (5)
packages/map/components/serverTimes.json (1)
65-77: LGTM! New Xbox server entries added correctly.The JSON structure is valid, and the naming convention (
xbx1,xbx2,xbx3) is consistent with existing server codes. UsingoffsetSeconds: 0aligns with the "basic support" intent of this PR.If Xbox servers require timezone-specific offsets in the future (like
fr1with 3600 or others with negative offsets), these values can be updated accordingly.package.json (1)
13-13: Dependency update for Xbox type support.The version bump to
@simrail/types^0.0.5 is required for the new Xbox-related type definitions used throughout this PR.Per the PR description, ensure the corresponding simrail-types PR is merged before merging this PR.
packages/map/pages/index.tsx (1)
95-104: XBX server code extraction logic looks correct.The IIFE correctly handles the XBX prefix case by extracting the numeric suffix (e.g., "XBX1" from "XBX123") and falling back appropriately.
packages/map/components/SpotlightSearch.tsx (2)
97-99: Approved: Xbox placeholder handling for trains.The "Unknown [XBOX]" placeholder is consistent with the approach in
StationMarker.tsx. The TODO comment about future Xbox username lookup is helpful.
15-25: Remove the unuseduserclass andusernamesexport.The
userclass and the exportedusernamesarray are never instantiated or populated anywhere in the codebase and should be removed. (Note: the file does useusernamesCache, which is a separateuseRef<Map>that serves a different purpose.)
| if (station.DispatchedBy[0].SteamId != "null") { | ||
| const avatarRequest = await fetch( | ||
| `https://simrail-edr.emeraldnetwork.xyz/steam/${station.DispatchedBy[0].SteamId}`, | ||
| ); | ||
| const profile: ProfileResponse = await avatarRequest.json(); | ||
| setAvatar(profile.avatar); | ||
| setUsername(profile.personaname); | ||
| } |
There was a problem hiding this comment.
Use strict inequality (!==) as flagged by linter.
The pipeline flagged this line for using != instead of !==. Using strict equality is a JavaScript best practice to avoid type coercion issues.
Suggested fix
- if (station.DispatchedBy[0].SteamId != "null") {
+ if (station.DispatchedBy[0].SteamId !== "null") {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (station.DispatchedBy[0].SteamId != "null") { | |
| const avatarRequest = await fetch( | |
| `https://simrail-edr.emeraldnetwork.xyz/steam/${station.DispatchedBy[0].SteamId}`, | |
| ); | |
| const profile: ProfileResponse = await avatarRequest.json(); | |
| setAvatar(profile.avatar); | |
| setUsername(profile.personaname); | |
| } | |
| if (station.DispatchedBy[0].SteamId !== "null") { | |
| const avatarRequest = await fetch( | |
| `https://simrail-edr.emeraldnetwork.xyz/steam/${station.DispatchedBy[0].SteamId}`, | |
| ); | |
| const profile: ProfileResponse = await avatarRequest.json(); | |
| setAvatar(profile.avatar); | |
| setUsername(profile.personaname); | |
| } |
🧰 Tools
🪛 GitHub Actions: Code quality
[warning] 24-24: lint/suspicious/noDoubleEquals: Use !== instead of !=
🤖 Prompt for AI Agents
In @packages/map/components/Markers/StationMarker.tsx around lines 24 - 31, The
conditional in StationMarker.tsx uses non-strict inequality
(station.DispatchedBy[0].SteamId != "null"); replace it with strict inequality
(!==) to avoid type-coercion issues — update the if check around the
fetch/profile logic (the block that calls
fetch(...steam/${station.DispatchedBy[0].SteamId}), awaits avatarRequest.json(),
and calls setAvatar/setUsername) to use !== "null".
| if (station.DispatchedBy[0].XboxId != "null") { | ||
| setAvatar(null); | ||
| setUsername("Unknown [XBOX]"); | ||
| } |
There was a problem hiding this comment.
Use strict inequality (!==) and consider order-of-precedence logic.
- The pipeline flagged this line for using
!=instead of!==. - If both
SteamIdandXboxIdare present, the Xbox block will override the Steam profile data. If this is intentional, a comment would clarify the precedence. If not, consider usingelse if.
Suggested fix with else-if for mutual exclusivity
- if (station.DispatchedBy[0].SteamId != "null") {
+ if (station.DispatchedBy[0].SteamId !== "null") {
const avatarRequest = await fetch(
`https://simrail-edr.emeraldnetwork.xyz/steam/${station.DispatchedBy[0].SteamId}`,
);
const profile: ProfileResponse = await avatarRequest.json();
setAvatar(profile.avatar);
setUsername(profile.personaname);
- }
- if (station.DispatchedBy[0].XboxId != "null") {
+ } else if (station.DispatchedBy[0].XboxId !== "null") {
setAvatar(null);
setUsername("Unknown [XBOX]");
}🧰 Tools
🪛 GitHub Actions: Code quality
[warning] 32-32: lint/suspicious/noDoubleEquals: Use !== instead of !=
🤖 Prompt for AI Agents
In @packages/map/components/Markers/StationMarker.tsx around lines 32 - 35, The
code uses a non-strict inequality and may unintentionally override Steam data:
replace the `!=` check on station.DispatchedBy[0].XboxId with strict `!==`, and
change the conditional structure so Xbox handling does not unconditionally
override Steam handling (use `else if` or explicitly document precedence)
modifying the logic around setAvatar and setUsername in the StationMarker
component to preserve mutual exclusivity or clarify intent.
| if (station.DispatchedBy[0] && station.DispatchedBy[0].SteamId) { | ||
| username = | ||
| usernamesCache.current.get(station.DispatchedBy[0].SteamId) ?? | ||
| "Unknown"; | ||
| } |
There was a problem hiding this comment.
Use optional chaining as flagged by linter.
The pipeline flagged this for lint/complexity/useOptionalChain.
Suggested fix
- if (station.DispatchedBy[0] && station.DispatchedBy[0].SteamId) {
+ if (station.DispatchedBy[0]?.SteamId) {🧰 Tools
🪛 GitHub Actions: Code quality
[warning] 120-120: lint/complexity/useOptionalChain: Change to an optional chain.
🤖 Prompt for AI Agents
In @packages/map/components/SpotlightSearch.tsx around lines 120 - 124, The code
is accessing station.DispatchedBy[0].SteamId without optional chaining; replace
that block to safely handle missing arrays/entries by first extracting steamId
with optional chaining (e.g., const steamId =
station.DispatchedBy?.[0]?.SteamId) and then set username using
usernamesCache.current.get(steamId) ?? "Unknown", or if steamId is undefined
just assign "Unknown"; update the logic around the username variable in
SpotlightSearch (the station.DispatchedBy/SteamId access and
usernamesCache.current.get call) to use optional chaining and nullish coalescing
to satisfy the linter.
| if (station.DispatchedBy[0] && station.DispatchedBy[0].XboxId) { | ||
| username = "Unknown [XBOX]" // make function for getting xbox usernames https://panel.simrail.eu:8084/users-open/ | ||
| } |
There was a problem hiding this comment.
Use optional chaining as flagged by linter.
The pipeline flagged this for lint/complexity/useOptionalChain.
Suggested fix
- if (station.DispatchedBy[0] && station.DispatchedBy[0].XboxId) {
+ if (station.DispatchedBy[0]?.XboxId) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (station.DispatchedBy[0] && station.DispatchedBy[0].XboxId) { | |
| username = "Unknown [XBOX]" // make function for getting xbox usernames https://panel.simrail.eu:8084/users-open/ | |
| } | |
| if (station.DispatchedBy[0]?.XboxId) { | |
| username = "Unknown [XBOX]" // make function for getting xbox usernames https://panel.simrail.eu:8084/users-open/ | |
| } |
🧰 Tools
🪛 GitHub Actions: Code quality
[warning] 126-126: lint/complexity/useOptionalChain: Change to an optional chain.
🤖 Prompt for AI Agents
In @packages/map/components/SpotlightSearch.tsx around lines 126 - 128, Replace
the manual null-checks with optional chaining: update the conditional that
currently checks station.DispatchedBy[0] and station.DispatchedBy[0].XboxId to
use station.DispatchedBy?.[0]?.XboxId, leaving the username assignment to
"Unknown [XBOX]" in the same block; reference the station variable and the
username assignment in SpotlightSearch.tsx when making this change.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @packages/map/components/Markers/StationMarker.tsx:
- Around line 24-35: In StationMarker.tsx the dispatch ID checks can both run
and overwrite each other and use loose inequality; change the second check to an
else if so Xbox logic is mutually exclusive with the Steam fetch, and replace !=
with !== for both comparisons (refer to station.DispatchedBy[0].SteamId and
station.DispatchedBy[0].XboxId and the avatar/username setAvatar/setUsername
calls) so Steam fetch result isn't immediately overwritten and strict equality
is used.
In @packages/map/components/SpotlightSearch.tsx:
- Line 83: The effect calls getUsernames(userIDs, platforms) but does not await
it, causing a race where spotlight actions are built before the username cache
is populated; modify the effect to await getUsernames(...) (or refactor to
separate effects) so usernames are resolved before constructing the actions in
buildSpotlightActions/buildActions (or whatever function builds the actions),
ensuring the async call to getUsernames is awaited and errors handled so the
actions use populated usernames instead of "Unknown".
- Around line 119-128: The two separate checks for SteamId and XboxId can both
run and let Xbox overwrite a valid Steam username; change the second check to an
else if to make them mutually exclusive and use optional chaining on
station.DispatchedBy (e.g., station.DispatchedBy?.[0]?.SteamId and
station.DispatchedBy?.[0]?.XboxId) when reading IDs and when calling
usernamesCache.current.get to avoid pipeline warnings and runtime errors.
- Around line 61-80: The checks are inconsistent: trains compare
ControlledBySteamID/ControlledByXboxID to the string "null" using !=, but
stations compare DispatchedBy[0].SteamId/XboxId to actual null; update station
checks to mirror trains by first ensuring DispatchedBy exists and has an element
(e.g., DispatchedBy && DispatchedBy.length > 0) and then compare SteamId/XboxId
to the string "null" using strict inequality (!== "null"); also replace any
remaining != with !== for ControlledBySteamID/ControlledByXboxID comparisons to
use strict equality consistently in the code handling trains and stations
(references: trains, trains[i].TrainData.ControlledBySteamID,
ControlledByXboxID, stations, stations[i].DispatchedBy, SteamId, XboxId).
🧹 Nitpick comments (2)
packages/map/pages/index.tsx (2)
95-104: Consider extracting the server code derivation to a helper function.The inline IIFE works correctly but reduces readability. A named helper would clarify intent and make the logic easier to test.
Suggested refactor
+const getDisplayCode = (serverCode: string): string => { + const letters = serverCode.match(/[A-Za-z]+/)?.at(0) ?? serverCode.slice(0, 2); + if (letters.toUpperCase() === "XBX") { + const numMatch = serverCode.match(/^XBX(\d+)/i); + return numMatch ? `XBX${numMatch[1]}` : "XBX"; + } + return letters; +};Then use it in the JSX:
<FlagIcon code={getDisplayCode(server.ServerCode)} />
181-184: Add a fallback for unknown XBX server codes.If a new Xbox server (e.g.,
XBX4) is added to the API, it will pass through without remapping, causingflagPack["XBX4Flag"]lookup to fail and log an error with no flag displayed.Consider adding a default fallback for any unrecognized
XBX*code:Suggested approach using a mapping object
- let lang = code.toUpperCase(); - - if (lang === "XBX1") lang = "PL"; - if (lang === "XBX2") lang = "INT"; - if (lang === "XBX3") lang = "INT"; - if (lang === "EN") lang = "GB"; + const langMappings: Record<string, string> = { + XBX1: "PL", + XBX2: "INT", + XBX3: "INT", + EN: "GB", + }; + + let lang = code.toUpperCase(); + if (lang in langMappings) { + lang = langMappings[lang]; + } else if (lang.startsWith("XBX")) { + lang = "INT"; // Default fallback for unknown Xbox servers + }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
package.jsonpackages/map/components/Markers/StationMarker.tsxpackages/map/components/SelectedTrainPopup.tsxpackages/map/components/SpotlightSearch.tsxpackages/map/components/serverTimes.jsonpackages/map/pages/index.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
packages/map/components/SelectedTrainPopup.tsx (1)
packages/map/components/steam.ts (1)
getSteamProfileOrBot(8-16)
packages/map/components/SpotlightSearch.tsx (1)
packages/map/components/steam.ts (1)
getSteamProfileOrBot(8-16)
🪛 GitHub Actions: Code quality
packages/map/components/Markers/StationMarker.tsx
[warning] 24-24: lint/suspicious/noDoubleEquals: Use !== instead of !=
[warning] 32-32: lint/suspicious/noDoubleEquals: Use !== instead of !=
packages/map/components/SpotlightSearch.tsx
[warning] 62-62: lint/suspicious/noDoubleEquals: Use !== instead of !=
[warning] 66-66: lint/suspicious/noDoubleEquals: Use !== instead of !=
[warning] 119-119: lint/complexity/useOptionalChain: Change to an optional chain.
[warning] 125-125: lint/complexity/useOptionalChain: Change to an optional chain.
🔇 Additional comments (2)
package.json (1)
13-13: LGTM!The dependency bump to
@simrail/types^0.0.5 is necessary for the new Xbox-related type definitions used throughout this PR.packages/map/components/serverTimes.json (1)
66-77: LGTM!The new Xbox server entries (
xbx1,xbx2,xbx3) follow the existing structure and provide the necessary time offset configuration for Xbox servers.
| if (station.DispatchedBy[0].SteamId != "null") { | ||
| const avatarRequest = await fetch( | ||
| `https://simrail-edr.emeraldnetwork.xyz/steam/${station.DispatchedBy[0].SteamId}`, | ||
| ); | ||
| const profile: ProfileResponse = await avatarRequest.json(); | ||
| setAvatar(profile.avatar); | ||
| setUsername(profile.personaname); | ||
| } | ||
| if (station.DispatchedBy[0].XboxId != "null") { | ||
| setAvatar(null); | ||
| setUsername("Unknown [XBOX]"); | ||
| } |
There was a problem hiding this comment.
Logic issue: Both conditions can execute, causing unintended overwrites.
If both SteamId and XboxId are set (and neither equals "null"), the Steam profile will be fetched first, then immediately overwritten by the Xbox placeholder. Use else if for the Xbox condition to ensure mutual exclusivity.
Additionally, the pipeline flagged the use of != instead of !== for strict equality.
Proposed fix
if (station.DispatchedBy[0]) {
- if (station.DispatchedBy[0].SteamId != "null") {
+ if (station.DispatchedBy[0].SteamId !== "null") {
const avatarRequest = await fetch(
`https://simrail-edr.emeraldnetwork.xyz/steam/${station.DispatchedBy[0].SteamId}`,
);
const profile: ProfileResponse = await avatarRequest.json();
setAvatar(profile.avatar);
setUsername(profile.personaname);
- }
- if (station.DispatchedBy[0].XboxId != "null") {
+ } else if (station.DispatchedBy[0].XboxId !== "null") {
setAvatar(null);
setUsername("Unknown [XBOX]");
}🧰 Tools
🪛 GitHub Actions: Code quality
[warning] 24-24: lint/suspicious/noDoubleEquals: Use !== instead of !=
[warning] 32-32: lint/suspicious/noDoubleEquals: Use !== instead of !=
🤖 Prompt for AI Agents
In @packages/map/components/Markers/StationMarker.tsx around lines 24 - 35, In
StationMarker.tsx the dispatch ID checks can both run and overwrite each other
and use loose inequality; change the second check to an else if so Xbox logic is
mutually exclusive with the Steam fetch, and replace != with !== for both
comparisons (refer to station.DispatchedBy[0].SteamId and
station.DispatchedBy[0].XboxId and the avatar/username setAvatar/setUsername
calls) so Steam fetch result isn't immediately overwritten and strict equality
is used.
| if (selectedTrain) | ||
| getSteamProfileOrBot(selectedTrain.TrainData.ControlledBySteamID).then( | ||
| getSteamProfileOrBot(selectedTrain.TrainData.ControlledBySteamID).then( // support xbox players | ||
| // @ts-ignore | ||
| setData, | ||
| ); |
There was a problem hiding this comment.
Xbox support is incomplete in this component.
The comment suggests Xbox support, but the implementation only fetches Steam profiles. When ControlledBySteamID is null/"null" for Xbox players, getSteamProfileOrBot will return "BOT" instead of the expected "Unknown [XBOX]" shown elsewhere in the codebase.
Consider adding Xbox handling similar to StationMarker.tsx:
Suggested approach
useEffect(() => {
- if (selectedTrain)
- getSteamProfileOrBot(selectedTrain.TrainData.ControlledBySteamID).then( // support xbox players
- // @ts-ignore
- setData,
- );
+ if (selectedTrain) {
+ if (selectedTrain.TrainData.ControlledBySteamID !== "null") {
+ getSteamProfileOrBot(selectedTrain.TrainData.ControlledBySteamID).then(
+ // @ts-ignore
+ setData,
+ );
+ } else if (selectedTrain.TrainData.ControlledByXboxID !== "null") {
+ setAvatar(null);
+ setUsername("Unknown [XBOX]");
+ }
+ }
}, [selectedTrain]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (selectedTrain) | |
| getSteamProfileOrBot(selectedTrain.TrainData.ControlledBySteamID).then( | |
| getSteamProfileOrBot(selectedTrain.TrainData.ControlledBySteamID).then( // support xbox players | |
| // @ts-ignore | |
| setData, | |
| ); | |
| useEffect(() => { | |
| if (selectedTrain) { | |
| if (selectedTrain.TrainData.ControlledBySteamID !== "null") { | |
| getSteamProfileOrBot(selectedTrain.TrainData.ControlledBySteamID).then( | |
| // @ts-ignore | |
| setData, | |
| ); | |
| } else if (selectedTrain.TrainData.ControlledByXboxID !== "null") { | |
| setAvatar(null); | |
| setUsername("Unknown [XBOX]"); | |
| } | |
| } | |
| }, [selectedTrain]); |
| if (trains[i].Type === "user" && trains[i] != null) { | ||
| if (trains[i].TrainData.ControlledBySteamID != "null") { | ||
| userIDs.push(trains[i].TrainData.ControlledBySteamID); | ||
| platforms.push("steam"); | ||
| } | ||
| else if (trains[i].TrainData.ControlledByXboxID != "null") { | ||
| userIDs.push(trains[i].TrainData.ControlledByXboxID); | ||
| platforms.push("xbox"); | ||
| } | ||
| } | ||
| } | ||
| for (let i = 0; i < stations.length; i++) { | ||
| if (stations[i].DispatchedBy[0]) | ||
| if (stations[i].DispatchedBy[0] && stations[i].DispatchedBy[0].SteamId != null) { | ||
| userIDs.push(stations[i].DispatchedBy[0].SteamId); | ||
| platforms.push("steam"); | ||
| } | ||
| if (stations[i].DispatchedBy[0] && stations[i].DispatchedBy[0].XboxId != null) { | ||
| userIDs.push(stations[i].DispatchedBy[0].XboxId); | ||
| platforms.push("xbox"); | ||
| } |
There was a problem hiding this comment.
Inconsistent null checking between trains and stations.
Trains check against the string "null" (lines 62, 66), but stations check against actual null (lines 73, 77). Per the PR description mentioning "Change to string null to match api", if the API returns the string "null" for stations as well, these checks will fail to filter correctly.
Also, the pipeline flagged != usage—use !== for strict equality.
Proposed fix for consistent null handling
for (let i = 0; i < trains.length; i++) {
if (trains[i].Type === "user" && trains[i] != null) {
- if (trains[i].TrainData.ControlledBySteamID != "null") {
+ if (trains[i].TrainData.ControlledBySteamID !== "null") {
userIDs.push(trains[i].TrainData.ControlledBySteamID);
platforms.push("steam");
}
- else if (trains[i].TrainData.ControlledByXboxID != "null") {
+ else if (trains[i].TrainData.ControlledByXboxID !== "null") {
userIDs.push(trains[i].TrainData.ControlledByXboxID);
platforms.push("xbox");
}
}
}
for (let i = 0; i < stations.length; i++) {
- if (stations[i].DispatchedBy[0] && stations[i].DispatchedBy[0].SteamId != null) {
+ if (stations[i].DispatchedBy[0]?.SteamId && stations[i].DispatchedBy[0].SteamId !== "null") {
userIDs.push(stations[i].DispatchedBy[0].SteamId);
platforms.push("steam");
- }
- if (stations[i].DispatchedBy[0] && stations[i].DispatchedBy[0].XboxId != null) {
+ } else if (stations[i].DispatchedBy[0]?.XboxId && stations[i].DispatchedBy[0].XboxId !== "null") {
userIDs.push(stations[i].DispatchedBy[0].XboxId);
platforms.push("xbox");
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (trains[i].Type === "user" && trains[i] != null) { | |
| if (trains[i].TrainData.ControlledBySteamID != "null") { | |
| userIDs.push(trains[i].TrainData.ControlledBySteamID); | |
| platforms.push("steam"); | |
| } | |
| else if (trains[i].TrainData.ControlledByXboxID != "null") { | |
| userIDs.push(trains[i].TrainData.ControlledByXboxID); | |
| platforms.push("xbox"); | |
| } | |
| } | |
| } | |
| for (let i = 0; i < stations.length; i++) { | |
| if (stations[i].DispatchedBy[0]) | |
| if (stations[i].DispatchedBy[0] && stations[i].DispatchedBy[0].SteamId != null) { | |
| userIDs.push(stations[i].DispatchedBy[0].SteamId); | |
| platforms.push("steam"); | |
| } | |
| if (stations[i].DispatchedBy[0] && stations[i].DispatchedBy[0].XboxId != null) { | |
| userIDs.push(stations[i].DispatchedBy[0].XboxId); | |
| platforms.push("xbox"); | |
| } | |
| if (trains[i].Type === "user" && trains[i] != null) { | |
| if (trains[i].TrainData.ControlledBySteamID !== "null") { | |
| userIDs.push(trains[i].TrainData.ControlledBySteamID); | |
| platforms.push("steam"); | |
| } | |
| else if (trains[i].TrainData.ControlledByXboxID !== "null") { | |
| userIDs.push(trains[i].TrainData.ControlledByXboxID); | |
| platforms.push("xbox"); | |
| } | |
| } | |
| } | |
| for (let i = 0; i < stations.length; i++) { | |
| if (stations[i].DispatchedBy[0]?.SteamId && stations[i].DispatchedBy[0].SteamId !== "null") { | |
| userIDs.push(stations[i].DispatchedBy[0].SteamId); | |
| platforms.push("steam"); | |
| } else if (stations[i].DispatchedBy[0]?.XboxId && stations[i].DispatchedBy[0].XboxId !== "null") { | |
| userIDs.push(stations[i].DispatchedBy[0].XboxId); | |
| platforms.push("xbox"); | |
| } |
🧰 Tools
🪛 GitHub Actions: Code quality
[warning] 62-62: lint/suspicious/noDoubleEquals: Use !== instead of !=
[warning] 66-66: lint/suspicious/noDoubleEquals: Use !== instead of !=
🤖 Prompt for AI Agents
In @packages/map/components/SpotlightSearch.tsx around lines 61 - 80, The checks
are inconsistent: trains compare ControlledBySteamID/ControlledByXboxID to the
string "null" using !=, but stations compare DispatchedBy[0].SteamId/XboxId to
actual null; update station checks to mirror trains by first ensuring
DispatchedBy exists and has an element (e.g., DispatchedBy &&
DispatchedBy.length > 0) and then compare SteamId/XboxId to the string "null"
using strict inequality (!== "null"); also replace any remaining != with !== for
ControlledBySteamID/ControlledByXboxID comparisons to use strict equality
consistently in the code handling trains and stations (references: trains,
trains[i].TrainData.ControlledBySteamID, ControlledByXboxID, stations,
stations[i].DispatchedBy, SteamId, XboxId).
| } | ||
|
|
||
| getUsernames(userIDs); | ||
| getUsernames(userIDs, platforms); |
There was a problem hiding this comment.
Missing await causes race condition with username resolution.
getUsernames is async but not awaited. The spotlight actions are built immediately after (lines 85-140), before the cache is populated. This causes usernames to display as "Unknown" until the next render cycle.
Proposed fix
- getUsernames(userIDs, platforms);
+ getUsernames(userIDs, platforms).then(() => {
+ // Move action building logic here, or trigger a re-render
+ });Alternatively, consider restructuring the effect to properly await the usernames before building actions, or split into separate effects.
🤖 Prompt for AI Agents
In @packages/map/components/SpotlightSearch.tsx at line 83, The effect calls
getUsernames(userIDs, platforms) but does not await it, causing a race where
spotlight actions are built before the username cache is populated; modify the
effect to await getUsernames(...) (or refactor to separate effects) so usernames
are resolved before constructing the actions in
buildSpotlightActions/buildActions (or whatever function builds the actions),
ensuring the async call to getUsernames is awaited and errors handled so the
actions use populated usernames instead of "Unknown".
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/map/components/SpotlightSearch.tsx (1)
39-50: Async function called withoutawait— cache may not be populated before rendering.
getUsernamesis an async function, but it's called withoutawaitat line 83. This means the spotlight actions are built before the usernames cache is populated, resulting in "Unknown" being displayed until the next render cycle.Suggested fix
Wrap the effect body in an async IIFE or extract to an async function that's properly awaited:
// biome-ignore lint/correctness/useExhaustiveDependencies: useEffect(() => { + (async () => { const actionsGroups: SpotlightActionGroupData[] = []; if (!open) return; const userIDs = []; const platforms = []; // ... collection logic ... - getUsernames(userIDs, platforms); + await getUsernames(userIDs, platforms); // ... rendering logic ... setSpotlightActions(actionsGroups); + })(); }, [trains, stations, open, map?.panTo, setSelectedTrain, map?.setZoom]);Also applies to: 83-83
🤖 Fix all issues with AI agents
In @packages/map/components/SpotlightSearch.tsx:
- Around line 119-128: Use optional chaining on station.DispatchedBy and its IDs
and make the checks exclusive so Steam usernames take precedence over Xbox like
train handling: in the SpotlightSearch component, replace the two separate ifs
that inspect station.DispatchedBy[0].SteamId and .XboxId with guarded
optional-chaining checks (e.g., station.DispatchedBy?.[0]?.SteamId) and use an
if / else if (or early-return) so the SteamId branch sets username from
usernamesCache.current.get(…) ?? "Unknown" and only if SteamId is absent fall
back to the Xbox branch (which can call or be replaced by a helper to resolve
Xbox usernames).
🧹 Nitpick comments (3)
packages/map/components/SpotlightSearch.tsx (3)
15-24: Class name should use PascalCase.The class
usershould be renamed toUserto follow TypeScript/JavaScript naming conventions for classes.Suggested fix
-class user { +class User { username = ""; ID = ""; platform = ""; - constructor(username: string, ID: string, platform: string) { + constructor(username: string, id: string, platform: string) { this.username = username; - this.ID = ID; + this.ID = id; this.platform = platform; } } -export const usernames: user[] = []; +export const usernames: User[] = [];
60-71: Null check is in the wrong order and redundant.At line 61,
trains[i].Typeis accessed before the null checktrains[i] != null. Iftrains[i]were null, it would throw before reaching the null check. Giventrainsis typed asTrain[], the null check is likely unnecessary.Suggested simplification
for (let i = 0; i < trains.length; i++) { - if (trains[i].Type === "user" && trains[i] != null) { + if (trains[i].Type === "user") { if (trains[i].TrainData.ControlledBySteamID) { userIDs.push(trains[i].TrainData.ControlledBySteamID); platforms.push("steam"); } else if (trains[i].TrainData.ControlledByXboxID) { userIDs.push(trains[i].TrainData.ControlledByXboxID); platforms.push("xbox"); } } }
96-98: LGTM with TODO noted.The placeholder "Unknown [XBOX]" is acceptable for initial Xbox support. The TODO comment references a potential endpoint for Xbox username resolution.
Would you like me to open an issue to track implementing Xbox username resolution via the
https://panel.simrail.eu:8084/users-open/endpoint?
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/map/components/SpotlightSearch.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/map/components/SpotlightSearch.tsx (1)
packages/map/components/steam.ts (1)
getSteamProfileOrBot(8-16)
🪛 GitHub Actions: Code quality
packages/map/components/SpotlightSearch.tsx
[warning] 119-119: lint/complexity/useOptionalChain: Change to an optional chain.
[warning] 125-125: lint/complexity/useOptionalChain: Change to an optional chain.
🔇 Additional comments (1)
packages/map/components/SpotlightSearch.tsx (1)
72-81: Inconsistent platform handling between trains and stations.For trains (lines 62-69), Steam and Xbox are mutually exclusive (
if/else if), but for stations both IDs can be collected if present. Consider whether this asymmetry is intentional.
| if (station.DispatchedBy[0] && station.DispatchedBy[0].SteamId) { | ||
| username = | ||
| usernamesCache.current.get(station.DispatchedBy[0].SteamId) ?? | ||
| "Unknown"; | ||
| } | ||
|
|
||
| if (station.DispatchedBy[0] && station.DispatchedBy[0].XboxId) { | ||
| username = "Unknown [XBOX]" // make function for getting xbox usernames https://panel.simrail.eu:8084/users-open/ | ||
| } | ||
|
|
There was a problem hiding this comment.
Use optional chaining and clarify precedence logic.
The pipeline warnings indicate these conditions should use optional chaining. Additionally, since both if statements are not exclusive, Xbox will override Steam if both IDs exist — this differs from train handling where Steam takes precedence.
Fix for optional chaining and consistent precedence
- if (station.DispatchedBy[0] && station.DispatchedBy[0].SteamId) {
+ if (station.DispatchedBy[0]?.SteamId) {
username =
usernamesCache.current.get(station.DispatchedBy[0].SteamId) ??
"Unknown";
}
- if (station.DispatchedBy[0] && station.DispatchedBy[0].XboxId) {
+ else if (station.DispatchedBy[0]?.XboxId) {
username = "Unknown [XBOX]" // make function for getting xbox usernames https://panel.simrail.eu:8084/users-open/
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (station.DispatchedBy[0] && station.DispatchedBy[0].SteamId) { | |
| username = | |
| usernamesCache.current.get(station.DispatchedBy[0].SteamId) ?? | |
| "Unknown"; | |
| } | |
| if (station.DispatchedBy[0] && station.DispatchedBy[0].XboxId) { | |
| username = "Unknown [XBOX]" // make function for getting xbox usernames https://panel.simrail.eu:8084/users-open/ | |
| } | |
| if (station.DispatchedBy[0]?.SteamId) { | |
| username = | |
| usernamesCache.current.get(station.DispatchedBy[0].SteamId) ?? | |
| "Unknown"; | |
| } | |
| else if (station.DispatchedBy[0]?.XboxId) { | |
| username = "Unknown [XBOX]" // make function for getting xbox usernames https://panel.simrail.eu:8084/users-open/ | |
| } |
🧰 Tools
🪛 GitHub Actions: Code quality
[warning] 119-119: lint/complexity/useOptionalChain: Change to an optional chain.
[warning] 125-125: lint/complexity/useOptionalChain: Change to an optional chain.
🤖 Prompt for AI Agents
In @packages/map/components/SpotlightSearch.tsx around lines 119 - 128, Use
optional chaining on station.DispatchedBy and its IDs and make the checks
exclusive so Steam usernames take precedence over Xbox like train handling: in
the SpotlightSearch component, replace the two separate ifs that inspect
station.DispatchedBy[0].SteamId and .XboxId with guarded optional-chaining
checks (e.g., station.DispatchedBy?.[0]?.SteamId) and use an if / else if (or
early-return) so the SteamId branch sets username from
usernamesCache.current.get(…) ?? "Unknown" and only if SteamId is absent fall
back to the Xbox branch (which can call or be replaced by a helper to resolve
Xbox usernames).
1 participant
This makes the map not crash on xbox servers and supports their flags in the menu. Xbox users are shown as "Unknown [XBOX]" More can be done with the simrail user endpoint it seems, not tried it yet https://panel.simrail.eu:8084/users-open/[ID]. This does require the PR on simrail-types to be merged.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.