Conversation
|
|
|
Проверял только билдится без ошибок или нет, в игре функционал не проверял ( и то смогу проверить только вечером ) |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Фас, кролик, сделай за меня мою работу... |
📝 WalkthroughWalkthroughThis PR introduces a radar target tracking feature for the shuttle console, enabling operators to set target waypoints via coordinates or by clicking grid objects on the map. The implementation spans client UI components, shared state structures, and server-side request handlers to synchronize target information across the system. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant MapUI as MapScreen
participant NavUI as NavScreen
participant Window as ShuttleConsoleWindow
participant BUI as ShuttleConsoleBUI
participant Server as ShuttleSystem
participant Radar as RadarConsoleSystem
participant State as RadarConsoleComponent
rect rgba(100, 150, 255, 0.5)
Note over User,State: Target via Coordinate Input
User->>NavUI: Enter coordinates & click Set
NavUI->>Window: OnSetTargetCoordinates(shuttle, Vector2)
Window->>BUI: OnSetTargetCoordinates event
BUI->>Server: SetTargetCoordinatesRequest
Server->>Radar: SetTarget(entity, targetEntity, position)
Radar->>State: Update Target & TargetEntity
Radar->>Server: RefreshShuttleConsoles
Server->>NavUI: NavInterfaceState with new Target
NavUI->>User: Radar displays target
end
rect rgba(150, 255, 150, 0.5)
Note over User,State: Target via Map Object Click
User->>MapUI: Click grid object on map
MapUI->>Window: RequestTrackEntity(shuttle, gridEntity)
Window->>BUI: RequestTrackEntity event
BUI->>Server: SetTargetCoordinatesRequest (gridEntity)
Server->>Radar: SetTarget(entity, gridEntity, zero)
Radar->>State: Update TargetEntity & resolve position
Radar->>Server: RefreshShuttleConsoles
Server->>NavUI: NavInterfaceState with new TargetEntity
NavUI->>User: Radar displays tracked entity
end
rect rgba(255, 200, 100, 0.5)
Note over User,State: Toggle Target Visibility
User->>NavUI: Click Show/Hide toggle
NavUI->>Window: OnSetHideTarget(shuttle, hideFlag)
Window->>BUI: OnSetHideTarget event
BUI->>Server: SetHideTargetRequest(hideFlag)
Server->>Radar: SetHideTarget(entity, hideFlag)
Radar->>State: Update HideTarget flag
Radar->>Server: RefreshShuttleConsoles
Server->>NavUI: NavInterfaceState with new HideTarget
NavUI->>User: Target visibility toggled
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Content.Server/Shuttles/Systems/RadarConsoleSystem.cs (1)
35-38:⚠️ Potential issue | 🟡 MinorRemove
refqualifier to match the majority pattern across the codebase.The handler signature uses
ref BoundUIOpenedEvent args, but this is inconsistent with the codebase convention—only 11 of 35 similar handlers useref, while 24 omit it. Although the build succeeds and the signature is valid, therefqualifier does not match the predominant pattern. Either removerefto align with the standard approach, or document why it's intentional if there's a specific performance reason.
🤖 Fix all issues with AI agents
In `@Content.Client/_NF/Shuttles/UI/NavScreen.xaml.cs`:
- Around line 170-184: SetTargetCoords currently uses
float.TryParse(TargetX.Text, out outputVector.X) and
float.TryParse(TargetY.Text, out outputVector.Y) which is culture-sensitive and
will fail for locales using commas as decimal separators; update both parses in
SetTargetCoords to use the invariant-culture overload (e.g., TryParse with
NumberStyles/ CultureInfo.InvariantCulture) so coordinates from TargetX and
TargetY parse consistently across locales, leaving the rest of the method
(setting NavRadar.Target, NavRadar.TargetEntity, calling
_entManager.TryGetNetEntity and invoking OnSetTargetCoordinates) unchanged.
- Around line 57-60: Replace the fragile OnPressed handler on the TargetShow
toggle with the toggle-specific event so you read the new state from the event
args: stop using TargetShow.OnPressed and reading TargetShow.Pressed, instead
subscribe to TargetShow.OnToggled and call SetHideTarget with the inverse of
args.Pressed (use the args.Pressed value provided by the OnToggled handler to
determine the post-toggle state for SetHideTarget).
In `@Content.Client/Shuttles/UI/NavScreen.xaml`:
- Around line 146-170: The TargetCoordsBox section is missing the styled
PanelContainer wrapper used elsewhere; wrap the existing controls (the
controls:BoxContainer named TargetCoordsBox containing TargetX, TargetY,
TargetSet, TargetShow) inside the same PanelContainer/PanelStyle pattern used by
neighboring sections so it gains the consistent border/background and
margins—place the PanelContainer outer element around TargetCoordsBox, move the
Label and inner Horizontal BoxContainer into the PanelContainer content, and
apply the same style classes/margins used by the IFF Search/Maximum IFF Distance
panels to ensure visual consistency.
In `@Content.Client/Shuttles/UI/ShuttleConsoleWindow.xaml.cs`:
- Around line 58-64: Fix the typo in the trailing comment in
ShuttleConsoleWindow.xaml.cs: change the comment text "End Frontie" to "End
Frontier" near the MapContainer.RequestTrackEntity event hookup (the block that
invokes RequestTrackEntity). No code logic changes required—only update the
comment string so it reads "End Frontier".
In `@Content.Client/Shuttles/UI/ShuttleNavControl.xaml.cs`:
- Around line 598-616: The code divides by uiDistance (an int) when computing
uiX/uiY which will throw when uiDistance == 0; add a guard before the
confinement logic that checks if uiDistance == 0 and in that case skip the
division (e.g., leave uiPosition at center or return early) so the expressions
using uiXCentre * uiXOffset / uiDistance and uiYCentre * uiYOffset / uiDistance
are never evaluated; modify the block using uiDistance, uiX, uiY, uiXCentre,
uiXOffset, uiYCentre, uiYOffset, isOutsideRadarCircle and uiPosition in
ShuttleNavControl.xaml.cs to return or bypass the confinement math when
uiDistance == 0.
In `@Content.Shared/Shuttles/BUIStates/NavInterfaceState.cs`:
- Line 3: Remove the unused using directive for
Content.Shared._NF.Shuttles.Events at the top of NavInterfaceState.cs; open the
file containing the NavInterfaceState class and delete the line "using
Content.Shared._NF.Shuttles.Events;" so only required namespaces remain, then
save/compile to confirm there are no missing references.
In `@Resources/Locale/en-US/_NF/shuttles/console.ftl`:
- Around line 29-34: Key name mismatch: the localization key
shuttle-console-hide-target reads "Show" which conflicts with its name; rename
shuttle-console-hide-target (and its companion
shuttle-console-hide-target-description) to a name matching the displayed text
such as shuttle-console-show-target (and
shuttle-console-show-target-description) or shuttle-console-toggle-target (and
shuttle-console-toggle-target-description), update their values accordingly, and
search/update any code or other locale files that reference
shuttle-console-hide-target or shuttle-console-hide-target-description to use
the new key(s) so translations and callers remain consistent.
🧹 Nitpick comments (9)
Content.Client/Shuttles/UI/MapScreen.xaml.cs (2)
521-537: Track button has a fixed 32px width but displays a single Unicode character — consider whether this is accessible and readable at various UI scales.The
⌖glyph (shuttle-console-map-track) may not render in all fonts or may be hard to interpret. This is minor — a tooltip explaining the button's purpose would improve discoverability.💡 Proposed: add a ToolTip to the track button
var trackButton = new Button() { Text = Loc.GetString("shuttle-console-map-track"), MinWidth = 32, - MaxWidth = 32 + MaxWidth = 32, + ToolTip = Loc.GetString("shuttle-console-set-target-description") };
55-55:_maxMapDequeuechange is currently a no-op.The dequeue logic in
FrameUpdate(lines 577-583) is commented out, so changing the max interval from 0.25 to 0.10 has no runtime effect. This is fine to keep for when the code is re-enabled, just noting for awareness.Content.Shared/Shuttles/Events/SetTrackedCoordinatesRequest.cs (1)
10-23: Dual-purpose message — consider documenting the mutual exclusivity more prominently.
SetTargetCoordinatesRequestserves two roles: setting a coordinate target (withTrackedEntity = Invalid) and tracking an entity (withTrackedPosition = Zero). TheTrackedEntitydoc comment partially covers this, but the class-level doc doesn't mention that these two fields are effectively mutually exclusive. A brief note at the class level would help future maintainers.Content.Shared/Shuttles/BUIStates/NavInterfaceState.cs (1)
50-63: Inconsistent member style: auto-properties vs. plain fields.
TargetandTargetEntity(lines 53, 58) use auto-properties ({ get; set; }) whileHideTarget(line 63) and all other members in this class use plain fields. For a[NetSerializable]state DTO, consistency matters for maintainability.Proposed fix — use plain fields consistently
- public Vector2? Target { get; set; } + public Vector2? Target; - public NetEntity? TargetEntity { get; set; } + public NetEntity? TargetEntity;Content.Server/_NF/Shuttles/Systems/ShuttleSystem.cs (1)
150-181: Event handlers should beprivate, notpublic.
NfSetTargetCoordinatesandNfSetHideTargetare event handlers subscribed viaSubscribeLocalEvent(lines 25-26). All other event handlers in this file (OnSetInertiaDampening,OnSetMaxShuttleSpeed) areprivate. These should follow the same convention.Proposed fix
- public void NfSetTargetCoordinates(EntityUid uid, ShuttleConsoleComponent component, SetTargetCoordinatesRequest args) + private void NfSetTargetCoordinates(EntityUid uid, ShuttleConsoleComponent component, SetTargetCoordinatesRequest args)- public void NfSetHideTarget(EntityUid uid, ShuttleConsoleComponent component, SetHideTargetRequest args) + private void NfSetHideTarget(EntityUid uid, ShuttleConsoleComponent component, SetHideTargetRequest args)Content.Client/Shuttles/UI/ShuttleNavControl.xaml.cs (1)
571-656: Significant code duplication with grid IFF drawing logic.The target rendering block (lines 571–656) is largely copy-pasted from the grid IFF drawing block (lines 417–554), including viewport confinement, mouse-over detection, distance formatting, label positioning, and coordinate tooltip rendering. Consider extracting shared logic into a helper method to reduce maintenance burden.
Content.Server/Shuttles/Systems/RadarConsoleSystem.cs (1)
6-12: Duplicateusingdirectives.
Content.Shared.PowerCell(lines 6 and 10) andContent.Shared.Movement.Components(lines 7 and 11) are each imported twice. This appears to be a pre-existing issue that the new import on line 12 was appended alongside.Proposed fix
using Content.Shared.Shuttles.BUIStates; using Content.Shared.Shuttles.Components; using Content.Shared.Shuttles.Systems; using Content.Shared.PowerCell; using Content.Shared.Movement.Components; using Robust.Server.GameObjects; using Robust.Shared.Map; -using Content.Shared.PowerCell; -using Content.Shared.Movement.Components; using Content.Server.Shuttles.Components; // FrontierContent.Client/_NF/Shuttles/UI/NavScreen.xaml.cs (1)
104-119: State sync for target coordinates looks correct, minor whitespace inconsistency.The
_targetCoordsModifiedguard preventing overwrite of user-typed coordinates is a good pattern. The inversion ofHideTarget→TargetShow.Pressedon line 105 is consistent with the toggle semantics.Lines 104 and 119 use tab indentation while the rest of the file uses spaces — this creates mixed whitespace.
Content.Server/Shuttles/Systems/ShuttleConsoleSystem.cs (1)
421-421: Fallback state passesNetEntity.InvalidfortargetEntitywhere the parameter type isNetEntity?.All three fallback constructions (lines 421, 536, 561) pass
NetEntity.Invalidinstead ofnullfor thetargetEntityparameter (which is typedNetEntity?). This creates ambiguity between "no entity" (null) and "invalid entity" (NetEntity.Invalid). If downstream code checkstargetEntity == nullvstargetEntity == NetEntity.Invalid, behavior will differ.Consider using
nullto match the parameter's default value and clearly signal "no target entity":Proposed fix (line 421 shown; apply same to lines 536 and 561)
-navState = new NavInterfaceState(0f, null, null, new Dictionary<NetEntity, List<DockingPortState>>(), InertiaDampeningMode.Dampen, null, true, false, null, NetEntity.Invalid, true); +navState = new NavInterfaceState(0f, null, null, new Dictionary<NetEntity, List<DockingPortState>>(), InertiaDampeningMode.Dampen, null, true, false, null, null, true);#!/bin/bash # Check how TargetEntity is compared/used downstream to understand null vs Invalid semantics rg -n 'TargetEntity' --type=cs -C3 | head -80
| TargetX.OnTextChanged += _ => _targetCoordsModified = true; | ||
| TargetY.OnTextChanged += _ => _targetCoordsModified = true; | ||
| TargetSet.OnPressed += _ => SetTargetCoords(); | ||
| TargetShow.OnPressed += _ => SetHideTarget(!TargetShow.Pressed); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find patterns of OnPressed with Pressed read in this codebase
rg -n 'OnPressed.*Pressed' --type=cs | head -40Repository: space-exodus/Monolith
Length of output: 5614
🏁 Script executed:
#!/bin/bash
# Search for OnPressed callback patterns to understand event ordering
rg -n -B3 -A3 'OnPressed\s*\+=' --type=cs --glob '*UI*' | head -80Repository: space-exodus/Monolith
Length of output: 8230
🏁 Script executed:
#!/bin/bash
# Find the TargetShow control definition and understand its type
rg -n 'TargetShow' --type=cs Content.Client/_NF/Shuttles/UI/ | head -20Repository: space-exodus/Monolith
Length of output: 298
🏁 Script executed:
#!/bin/bash
# Find TargetShow definition in NavScreen
rg -n 'TargetShow' Content.Client/_NF/Shuttles/UI/NavScreen.xaml.cs -B2 -A2Repository: space-exodus/Monolith
Length of output: 445
🏁 Script executed:
#!/bin/bash
# Find SetHideTarget method implementation
rg -n 'SetHideTarget' Content.Client/_NF/Shuttles/UI/NavScreen.xaml.cs -B5 -A10Repository: space-exodus/Monolith
Length of output: 2089
🏁 Script executed:
#!/bin/bash
# Check if TargetShow is defined in XAML
cat Content.Client/_NF/Shuttles/UI/NavScreen.xaml | grep -n -i 'targetshow\|TargetShow' | head -20Repository: space-exodus/Monolith
Length of output: 142
🏁 Script executed:
#!/bin/bash
# Search for toggle button framework definitions to understand Pressed timing
find . -name '*.cs' -type f | xargs grep -l 'class.*Button' | grep -i 'userinterface\|ui\|framework' | head -10Repository: space-exodus/Monolith
Length of output: 711
🏁 Script executed:
#!/bin/bash
# Search for TargetShow property declaration in NavScreen
rg -n 'TargetShow' Content.Client/_NF/Shuttles/UI/ -B5 -A2 | head -60Repository: space-exodus/Monolith
Length of output: 1497
🏁 Script executed:
#!/bin/bash
# Search for Button or CheckButton class definitions that might implement toggle behavior
fd -e cs -p 'Button' Content.Client/UserInterface | xargs grep -l 'class.*Button.*{' | head -10Repository: space-exodus/Monolith
Length of output: 47
🏁 Script executed:
#!/antml:max_thinking
I need to find:
1. What type TargetShow is (is it a toggle button?)
2. When the Pressed state is updated relative to when OnPressed fires
3. Look at the framework's button implementation
</thinking>
#!/bin/bash
# Search for OnPressed event firing and Pressed state change patterns in framework code
rg -n 'OnPressed' Content.Client/UserInterface -B3 -A3 | grep -E '(OnPressed|Pressed\s*=|fire|emit)' | head -40Repository: space-exodus/Monolith
Length of output: 243
🏁 Script executed:
#!/bin/bash
# Look for CheckButton or toggle-specific button class in framework
find . -name '*.cs' -type f | xargs grep -l 'class CheckButton\|class.*Toggle' | head -10Repository: space-exodus/Monolith
Length of output: 730
🏁 Script executed:
#!/bin/bash
# Find CheckButton or BaseButton implementations in the framework
rg -n 'class CheckButton|class BaseButton' --type=cs | head -20Repository: space-exodus/Monolith
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Search for OnPressed event definition and when it fires
rg -n 'OnPressed' Content.Client/UserInterface -A5 --type=cs | grep -E '(OnPressed.*=|OnPressed\s*\?|InvokePressed|Pressed\s*=)' | head -40Repository: space-exodus/Monolith
Length of output: 5758
🏁 Script executed:
#!/bin/bash
# Look for the actual implementation of Pressed property setter in button classes
rg -n 'Pressed\s*=' Content.Client/UserInterface --type=cs -B3 -A3 | head -80Repository: space-exodus/Monolith
Length of output: 7273
🏁 Script executed:
#!/bin/bash
# Check what type TargetShow is by looking at NavScreen class properties/fields
ast-grep --pattern 'class NavScreen {
$$$
$_* TargetShow $_
$$$
}'Repository: space-exodus/Monolith
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Search for TargetShow declaration as a field or property in NavScreen
rg -n 'TargetShow' Content.Client/_NF/Shuttles/UI/NavScreen.xaml.cs --type=cs -B10 | grep -E '(TargetShow|public|private|Button|Check)'Repository: space-exodus/Monolith
Length of output: 398
🏁 Script executed:
#!/bin/bash
# Look for the full NavScreen.xaml.cs file to understand TargetShow type
wc -l Content.Client/_NF/Shuttles/UI/NavScreen.xaml.csRepository: space-exodus/Monolith
Length of output: 117
🏁 Script executed:
#!/bin/bash
# Read the beginning of the NavScreen file to see field declarations
head -100 Content.Client/_NF/Shuttles/UI/NavScreen.xaml.csRepository: space-exodus/Monolith
Length of output: 4899
🏁 Script executed:
#!/antml:thinking>
Let me search for button base classes and understand the event timing model used in this framework.
</thinking>
#!/bin/bash
# Search for BaseButton or Button class that has OnPressed and Pressed
rg -n 'class.*Button' Content.Client/UserInterface --type=cs -B2 -A10 | grep -E '(class|OnPressed|Pressed\s*{|event)' | head -50Repository: space-exodus/Monolith
Length of output: 211
🏁 Script executed:
#!/bin/bash
# Look for TargetShow in XAML designer file or similar
find Content.Client/_NF/Shuttles/UI -type f -name '*NavScreen*'Repository: space-exodus/Monolith
Length of output: 113
🏁 Script executed:
#!/bin/bash
# Search for how buttons with OnPressed work - look for event invocation in button implementations
rg -n 'OnPressed\?' Content.Client/UserInterface --type=cs -B5 -A2 | grep -E '(OnPressed\?\.Invoke|protected.*OnPressed|virtual.*OnPressed)' | head -20Repository: space-exodus/Monolith
Length of output: 171
🏁 Script executed:
#!/antml:thinking>
Let me search for CheckButton specifically, which is likely what TargetShow is since it has a Pressed property and OnPressed event.
</thinking>
#!/bin/bash
# Look for CheckButton class which typically has a Pressed property and OnPressed event
rg -n 'class CheckButton' Content.Client/UserInterface --type=cs -A30Repository: space-exodus/Monolith
Length of output: 211
🏁 Script executed:
#!/bin/bash
# Search for CheckButton in the entire UserInterface directory
rg -n 'class CheckButton' --type=csRepository: space-exodus/Monolith
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Look at the ConfirmButton implementation that showed OnPressed?.Invoke
rg -n 'class ConfirmButton' Content.Client/UserInterface/Controls/ConfirmButton.cs -A50Repository: space-exodus/Monolith
Length of output: 1800
🏁 Script executed:
#!/an/bin/bash
# Search for Button class in Robust.Client UserInterface
rg -n 'public.*Pressed.*{' --type=cs | head -30Repository: space-exodus/Monolith
Length of output: 164
🏁 Script executed:
#!/bin/bash
# Search for Button class definition
rg -n 'public.*class Button' --type=cs | head -10Repository: space-exodus/Monolith
Length of output: 242
🏁 Script executed:
#!/bin/bash
# Look for the Pressed property definition
rg -n 'public.*bool.*Pressed' --type=cs -A5 | head -50Repository: space-exodus/Monolith
Length of output: 3827
🏁 Script executed:
#!/bin/bash
# Check if this is Robust engine - look for Robust.Client references
head -20 Content.Client/_NF/Shuttles/UI/NavScreen.xaml.cs | grep -i robustRepository: space-exodus/Monolith
Length of output: 108
🏁 Script executed:
#!/bin/bash
# Search for Button.xaml or Button.xaml.cs
find . -name 'Button.xaml*' -type fRepository: space-exodus/Monolith
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Since this uses Robust framework, search for how OnToggled is used vs OnPressed with Pressed
rg -n 'OnToggled' Content.Client --type=cs | head -20Repository: space-exodus/Monolith
Length of output: 2609
🏁 Script executed:
#!/an/bin/bash
# Look for patterns where OnPressed reads Pressed vs OnToggled pattern
rg -n 'OnPressed.*Pressed|OnToggled' Content.Client/_NF/Shuttles/UI/ --type=cs -B2 -A2Repository: space-exodus/Monolith
Length of output: 2045
🏁 Script executed:
#!/bin/bash
# Search for documentation or comments about button pressed state timing
rg -n 'toggle|Pressed.*state|state.*change' Content.Client/_NF/Shuttles/UI/NavScreen.xaml.cs --type=cs -i -B2 -A2Repository: space-exodus/Monolith
Length of output: 208
🏁 Script executed:
#!/bin/bash
# Look at the full SetHideTarget usage context and any related comments
cat Content.Client/_NF/Shuttles/UI/NavScreen.xaml.cs | sed -n '180,191p'Repository: space-exodus/Monolith
Length of output: 512
🏁 Script executed:
#!/bin/bash
# Verify: look at what type TargetShow should be - search for its declaration in partial NavScreen class
rg -n 'partial class NavScreen' Content.Client/_NF/Shuttles/UI/NavScreen.xaml.cs -A100 | head -120Repository: space-exodus/Monolith
Length of output: 5252
🏁 Script executed:
#!/bin/bash
# Compare: show how OnToggled is properly used with args.Pressed vs OnPressed with direct Pressed read
rg -n 'OnToggled.*args.*Pressed' Content.Client --type=cs -B2 -A2 | head -40Repository: space-exodus/Monolith
Length of output: 3878
🏁 Script executed:
#!/bin/bash
# Final verification: confirm TargetShow is indeed a toggle button by searching for its declaration
rg -n 'TargetShow' Content.Client/_NF/Shuttles/UI/ --type=cs -B5 -A1 | grep -E '(TargetShow|CheckButton|Button|public)'Repository: space-exodus/Monolith
Length of output: 298
🏁 Script executed:
#!/an/bin/bash
# Check if there are any other OnPressed handlers that read Pressed directly (to find similar issues)
rg -n 'OnPressed.*Pressed' Content.Client --type=cs | grep -v OnToggledRepository: space-exodus/Monolith
Length of output: 38398
Consider using OnToggled event with args.Pressed instead of OnPressed with direct Pressed read.
Line 60 uses OnPressed to read TargetShow.Pressed directly, but the codebase convention for toggle buttons is to use OnToggled and receive the new state via args.Pressed. The current pattern is fragile and depends on undefined timing of when Pressed updates relative to the OnPressed event firing. Use:
TargetShow.OnToggled += args => SetHideTarget(!args.Pressed);This ensures you always receive the correct post-toggle state without relying on framework implementation details.
🤖 Prompt for AI Agents
In `@Content.Client/_NF/Shuttles/UI/NavScreen.xaml.cs` around lines 57 - 60,
Replace the fragile OnPressed handler on the TargetShow toggle with the
toggle-specific event so you read the new state from the event args: stop using
TargetShow.OnPressed and reading TargetShow.Pressed, instead subscribe to
TargetShow.OnToggled and call SetHideTarget with the inverse of args.Pressed
(use the args.Pressed value provided by the OnToggled handler to determine the
post-toggle state for SetHideTarget).
| private void SetTargetCoords() | ||
| { | ||
| Vector2 outputVector; | ||
| if (!float.TryParse(TargetX.Text, out outputVector.X)) | ||
| outputVector.X = 0.0f; | ||
|
|
||
| if (!float.TryParse(TargetY.Text, out outputVector.Y)) | ||
| outputVector.Y = 0.0f; | ||
|
|
||
| NavRadar.Target = outputVector; | ||
| NavRadar.TargetEntity = NetEntity.Invalid; | ||
| _entManager.TryGetNetEntity(_shuttleEntity, out var shuttle); | ||
| OnSetTargetCoordinates?.Invoke(shuttle, outputVector); | ||
| _targetCoordsModified = false; | ||
| } |
There was a problem hiding this comment.
float.TryParse is culture-sensitive — coordinate parsing may break for non-English locales.
float.TryParse(TargetX.Text, out outputVector.X) (and the Y equivalent) uses the current culture's number format by default. Players with locales that use , as the decimal separator (e.g., German, French, Russian) will fail to parse values like "123.4" and silently default to 0.0f.
Use the invariant culture overload:
Proposed fix
+using System.Globalization;
...
private void SetTargetCoords()
{
Vector2 outputVector;
- if (!float.TryParse(TargetX.Text, out outputVector.X))
+ if (!float.TryParse(TargetX.Text, NumberStyles.Float, CultureInfo.InvariantCulture, out outputVector.X))
outputVector.X = 0.0f;
- if (!float.TryParse(TargetY.Text, out outputVector.Y))
+ if (!float.TryParse(TargetY.Text, NumberStyles.Float, CultureInfo.InvariantCulture, out outputVector.Y))
outputVector.Y = 0.0f;
NavRadar.Target = outputVector;
NavRadar.TargetEntity = NetEntity.Invalid;
_entManager.TryGetNetEntity(_shuttleEntity, out var shuttle);
OnSetTargetCoordinates?.Invoke(shuttle, outputVector);
_targetCoordsModified = false;
}📝 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.
| private void SetTargetCoords() | |
| { | |
| Vector2 outputVector; | |
| if (!float.TryParse(TargetX.Text, out outputVector.X)) | |
| outputVector.X = 0.0f; | |
| if (!float.TryParse(TargetY.Text, out outputVector.Y)) | |
| outputVector.Y = 0.0f; | |
| NavRadar.Target = outputVector; | |
| NavRadar.TargetEntity = NetEntity.Invalid; | |
| _entManager.TryGetNetEntity(_shuttleEntity, out var shuttle); | |
| OnSetTargetCoordinates?.Invoke(shuttle, outputVector); | |
| _targetCoordsModified = false; | |
| } | |
| private void SetTargetCoords() | |
| { | |
| Vector2 outputVector; | |
| if (!float.TryParse(TargetX.Text, NumberStyles.Float, CultureInfo.InvariantCulture, out outputVector.X)) | |
| outputVector.X = 0.0f; | |
| if (!float.TryParse(TargetY.Text, NumberStyles.Float, CultureInfo.InvariantCulture, out outputVector.Y)) | |
| outputVector.Y = 0.0f; | |
| NavRadar.Target = outputVector; | |
| NavRadar.TargetEntity = NetEntity.Invalid; | |
| _entManager.TryGetNetEntity(_shuttleEntity, out var shuttle); | |
| OnSetTargetCoordinates?.Invoke(shuttle, outputVector); | |
| _targetCoordsModified = false; | |
| } |
🤖 Prompt for AI Agents
In `@Content.Client/_NF/Shuttles/UI/NavScreen.xaml.cs` around lines 170 - 184,
SetTargetCoords currently uses float.TryParse(TargetX.Text, out outputVector.X)
and float.TryParse(TargetY.Text, out outputVector.Y) which is culture-sensitive
and will fail for locales using commas as decimal separators; update both parses
in SetTargetCoords to use the invariant-culture overload (e.g., TryParse with
NumberStyles/ CultureInfo.InvariantCulture) so coordinates from TargetX and
TargetY parse consistently across locales, leaving the rest of the method
(setting NavRadar.Target, NavRadar.TargetEntity, calling
_entManager.TryGetNetEntity and invoking OnSetTargetCoordinates) unchanged.
| <controls:BoxContainer Orientation="Vertical" HorizontalExpand="True" Name="TargetCoordsBox"> | ||
| <controls:Label Text="{controls:Loc 'shuttle-console-target'}"/> | ||
| <controls:BoxContainer Orientation="Horizontal" | ||
| HorizontalExpand="True" | ||
| Align="Center" | ||
| Margin="0 0 0 3"> | ||
| <controls:LineEdit Name="TargetX" Access="Public" HorizontalExpand="True"/> | ||
| <controls:LineEdit Name="TargetY" Access="Public" HorizontalExpand="True"/> | ||
| <controls:Button Name="TargetSet" | ||
| Text="{controls:Loc 'shuttle-console-set-target'}" | ||
| TextAlign="Center" | ||
| MinWidth="20" | ||
| ToolTip="{controls:Loc 'shuttle-console-set-target-description'}" | ||
| StyleClasses="OpenRight" | ||
| Margin="0"/> | ||
| <controls:Button Name="TargetShow" | ||
| Text="{controls:Loc 'shuttle-console-hide-target'}" | ||
| TextAlign="Center" | ||
| ToggleMode="True" | ||
| MinWidth="20" | ||
| ToolTip="{controls:Loc 'shuttle-console-hide-target-description'}" | ||
| StyleClasses="OpenLeft" | ||
| Margin="0"/> | ||
| </controls:BoxContainer> | ||
| </controls:BoxContainer> |
There was a problem hiding this comment.
Missing panel styling wrapper — visual inconsistency with adjacent sections.
The "Radar Target" section is a bare BoxContainer without the styled PanelContainer wrapper (with border and background) that all other sections in this sidebar use (IFF Search at line 174, Maximum IFF Distance at line 186, dampener controls at line 113, etc.). This will make the target section look visually different from the rest of the nav panel.
Consider wrapping the TargetCoordsBox in the same styled panel pattern used by adjacent controls:
🎨 Proposed: wrap in styled PanelContainer
<!-- Frontier - settable target -->
+ <controls:PanelContainer VerticalExpand="True" HorizontalExpand="True" Margin="4 4 4 4">
+ <controls:PanelContainer.PanelOverride>
+ <graphics:StyleBoxFlat BorderThickness="2" BorderColor="#333333" BackgroundColor="#181818"/>
+ </controls:PanelContainer.PanelOverride>
<controls:BoxContainer Orientation="Vertical" HorizontalExpand="True" Name="TargetCoordsBox">
<controls:Label Text="{controls:Loc 'shuttle-console-target'}"/>
...
</controls:BoxContainer>
+ </controls:PanelContainer>
<!-- End Frontier - settable target -->📝 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.
| <controls:BoxContainer Orientation="Vertical" HorizontalExpand="True" Name="TargetCoordsBox"> | |
| <controls:Label Text="{controls:Loc 'shuttle-console-target'}"/> | |
| <controls:BoxContainer Orientation="Horizontal" | |
| HorizontalExpand="True" | |
| Align="Center" | |
| Margin="0 0 0 3"> | |
| <controls:LineEdit Name="TargetX" Access="Public" HorizontalExpand="True"/> | |
| <controls:LineEdit Name="TargetY" Access="Public" HorizontalExpand="True"/> | |
| <controls:Button Name="TargetSet" | |
| Text="{controls:Loc 'shuttle-console-set-target'}" | |
| TextAlign="Center" | |
| MinWidth="20" | |
| ToolTip="{controls:Loc 'shuttle-console-set-target-description'}" | |
| StyleClasses="OpenRight" | |
| Margin="0"/> | |
| <controls:Button Name="TargetShow" | |
| Text="{controls:Loc 'shuttle-console-hide-target'}" | |
| TextAlign="Center" | |
| ToggleMode="True" | |
| MinWidth="20" | |
| ToolTip="{controls:Loc 'shuttle-console-hide-target-description'}" | |
| StyleClasses="OpenLeft" | |
| Margin="0"/> | |
| </controls:BoxContainer> | |
| </controls:BoxContainer> | |
| <controls:PanelContainer VerticalExpand="True" HorizontalExpand="True" Margin="4 4 4 4"> | |
| <controls:PanelContainer.PanelOverride> | |
| <graphics:StyleBoxFlat BorderThickness="2" BorderColor="#333333" BackgroundColor="#181818"/> | |
| </controls:PanelContainer.PanelOverride> | |
| <controls:BoxContainer Orientation="Vertical" HorizontalExpand="True" Name="TargetCoordsBox"> | |
| <controls:Label Text="{controls:Loc 'shuttle-console-target'}"/> | |
| <controls:BoxContainer Orientation="Horizontal" | |
| HorizontalExpand="True" | |
| Align="Center" | |
| Margin="0 0 0 3"> | |
| <controls:LineEdit Name="TargetX" Access="Public" HorizontalExpand="True"/> | |
| <controls:LineEdit Name="TargetY" Access="Public" HorizontalExpand="True"/> | |
| <controls:Button Name="TargetSet" | |
| Text="{controls:Loc 'shuttle-console-set-target'}" | |
| TextAlign="Center" | |
| MinWidth="20" | |
| ToolTip="{controls:Loc 'shuttle-console-set-target-description'}" | |
| StyleClasses="OpenRight" | |
| Margin="0"/> | |
| <controls:Button Name="TargetShow" | |
| Text="{controls:Loc 'shuttle-console-hide-target'}" | |
| TextAlign="Center" | |
| ToggleMode="True" | |
| MinWidth="20" | |
| ToolTip="{controls:Loc 'shuttle-console-hide-target-description'}" | |
| StyleClasses="OpenLeft" | |
| Margin="0"/> | |
| </controls:BoxContainer> | |
| </controls:BoxContainer> | |
| </controls:PanelContainer> |
🤖 Prompt for AI Agents
In `@Content.Client/Shuttles/UI/NavScreen.xaml` around lines 146 - 170, The
TargetCoordsBox section is missing the styled PanelContainer wrapper used
elsewhere; wrap the existing controls (the controls:BoxContainer named
TargetCoordsBox containing TargetX, TargetY, TargetSet, TargetShow) inside the
same PanelContainer/PanelStyle pattern used by neighboring sections so it gains
the consistent border/background and margins—place the PanelContainer outer
element around TargetCoordsBox, move the Label and inner Horizontal BoxContainer
into the PanelContainer content, and apply the same style classes/margins used
by the IFF Search/Maximum IFF Distance panels to ensure visual consistency.
|
|
||
| // Frontier: entity tracking | ||
| MapContainer.RequestTrackEntity += (ent, trackEntity) => | ||
| { | ||
| RequestTrackEntity?.Invoke(ent, trackEntity); | ||
| }; | ||
| // End Frontie |
There was a problem hiding this comment.
Typo in comment: "End Frontie" → "End Frontier".
✏️ Fix
- // End Frontie
+ // End Frontier📝 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.
| // Frontier: entity tracking | |
| MapContainer.RequestTrackEntity += (ent, trackEntity) => | |
| { | |
| RequestTrackEntity?.Invoke(ent, trackEntity); | |
| }; | |
| // End Frontie | |
| // Frontier: entity tracking | |
| MapContainer.RequestTrackEntity += (ent, trackEntity) => | |
| { | |
| RequestTrackEntity?.Invoke(ent, trackEntity); | |
| }; | |
| // End Frontier |
🤖 Prompt for AI Agents
In `@Content.Client/Shuttles/UI/ShuttleConsoleWindow.xaml.cs` around lines 58 -
64, Fix the typo in the trailing comment in ShuttleConsoleWindow.xaml.cs: change
the comment text "End Frontie" to "End Frontier" near the
MapContainer.RequestTrackEntity event hookup (the block that invokes
RequestTrackEntity). No code logic changes required—only update the comment
string so it reads "End Frontier".
| var uiXCentre = (int) Width / 2; | ||
| var uiYCentre = (int) Height / 2; | ||
| var uiXOffset = uiPosition.X - uiXCentre; | ||
| var uiYOffset = uiPosition.Y - uiYCentre; | ||
| var uiDistance = (int) Math.Sqrt(Math.Pow(uiXOffset, 2) + Math.Pow(uiYOffset, 2)); | ||
| var uiX = uiXCentre * uiXOffset / uiDistance; | ||
| var uiY = uiYCentre * uiYOffset / uiDistance; | ||
|
|
||
| var isOutsideRadarCircle = uiDistance > Math.Abs(uiX) && uiDistance > Math.Abs(uiY); | ||
| if (isOutsideRadarCircle) | ||
| { | ||
| // 0.95f for offsetting the icons slightly away from edge of radar so it doesnt clip. | ||
| uiX = uiXCentre * uiXOffset / uiDistance * 0.95f; | ||
| uiY = uiYCentre * uiYOffset / uiDistance * 0.95f; | ||
| uiPosition = new Vector2( | ||
| x: uiX + uiXCentre, | ||
| y: uiY + uiYCentre | ||
| ); | ||
| } |
There was a problem hiding this comment.
Division by zero when target is at radar center.
When uiDistance is 0 (target position coincides with radar center), lines 603–604 divide by zero: uiXCentre * uiXOffset / uiDistance. This will throw a DivideByZeroException since uiDistance is an int.
Add a guard to skip the confinement logic when uiDistance is 0:
Proposed fix
var uiDistance = (int) Math.Sqrt(Math.Pow(uiXOffset, 2) + Math.Pow(uiYOffset, 2));
- var uiX = uiXCentre * uiXOffset / uiDistance;
- var uiY = uiYCentre * uiYOffset / uiDistance;
-
- var isOutsideRadarCircle = uiDistance > Math.Abs(uiX) && uiDistance > Math.Abs(uiY);
+ var isOutsideRadarCircle = false;
+ if (uiDistance > 0)
+ {
+ var uiX = uiXCentre * uiXOffset / uiDistance;
+ var uiY = uiYCentre * uiYOffset / uiDistance;
+ isOutsideRadarCircle = uiDistance > Math.Abs(uiX) && uiDistance > Math.Abs(uiY);
+ if (isOutsideRadarCircle)
+ {
+ uiX = uiXCentre * uiXOffset / uiDistance * 0.95f;
+ uiY = uiYCentre * uiYOffset / uiDistance * 0.95f;
+ uiPosition = new Vector2(
+ x: uiX + uiXCentre,
+ y: uiY + uiYCentre
+ );
+ }
+ }Note: the same pre-existing bug exists in the grid IFF drawing code around line 433, but it should be fixed here at minimum since this is new code.
📝 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.
| var uiXCentre = (int) Width / 2; | |
| var uiYCentre = (int) Height / 2; | |
| var uiXOffset = uiPosition.X - uiXCentre; | |
| var uiYOffset = uiPosition.Y - uiYCentre; | |
| var uiDistance = (int) Math.Sqrt(Math.Pow(uiXOffset, 2) + Math.Pow(uiYOffset, 2)); | |
| var uiX = uiXCentre * uiXOffset / uiDistance; | |
| var uiY = uiYCentre * uiYOffset / uiDistance; | |
| var isOutsideRadarCircle = uiDistance > Math.Abs(uiX) && uiDistance > Math.Abs(uiY); | |
| if (isOutsideRadarCircle) | |
| { | |
| // 0.95f for offsetting the icons slightly away from edge of radar so it doesnt clip. | |
| uiX = uiXCentre * uiXOffset / uiDistance * 0.95f; | |
| uiY = uiYCentre * uiYOffset / uiDistance * 0.95f; | |
| uiPosition = new Vector2( | |
| x: uiX + uiXCentre, | |
| y: uiY + uiYCentre | |
| ); | |
| } | |
| var uiXCentre = (int) Width / 2; | |
| var uiYCentre = (int) Height / 2; | |
| var uiXOffset = uiPosition.X - uiXCentre; | |
| var uiYOffset = uiPosition.Y - uiYCentre; | |
| var uiDistance = (int) Math.Sqrt(Math.Pow(uiXOffset, 2) + Math.Pow(uiYOffset, 2)); | |
| var isOutsideRadarCircle = false; | |
| if (uiDistance > 0) | |
| { | |
| var uiX = uiXCentre * uiXOffset / uiDistance; | |
| var uiY = uiYCentre * uiYOffset / uiDistance; | |
| isOutsideRadarCircle = uiDistance > Math.Abs(uiX) && uiDistance > Math.Abs(uiY); | |
| if (isOutsideRadarCircle) | |
| { | |
| // 0.95f for offsetting the icons slightly away from edge of radar so it doesnt clip. | |
| uiX = uiXCentre * uiXOffset / uiDistance * 0.95f; | |
| uiY = uiYCentre * uiYOffset / uiDistance * 0.95f; | |
| uiPosition = new Vector2( | |
| x: uiX + uiXCentre, | |
| y: uiY + uiYCentre | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@Content.Client/Shuttles/UI/ShuttleNavControl.xaml.cs` around lines 598 - 616,
The code divides by uiDistance (an int) when computing uiX/uiY which will throw
when uiDistance == 0; add a guard before the confinement logic that checks if
uiDistance == 0 and in that case skip the division (e.g., leave uiPosition at
center or return early) so the expressions using uiXCentre * uiXOffset /
uiDistance and uiYCentre * uiYOffset / uiDistance are never evaluated; modify
the block using uiDistance, uiX, uiY, uiXCentre, uiXOffset, uiYCentre,
uiYOffset, isOutsideRadarCircle and uiPosition in ShuttleNavControl.xaml.cs to
return or bypass the confinement math when uiDistance == 0.
| using Robust.Shared.Map; | ||
| using Robust.Shared.Serialization; | ||
| using Content.Shared._NF.Shuttles.Events; // Frontier - InertiaDampeningMode access | ||
| using Content.Shared._NF.Shuttles.Events; // Frontier |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "NavInterfaceState.cs" -x cat {}Repository: space-exodus/Monolith
Length of output: 3025
🏁 Script executed:
rg "SetTargetCoordinatesRequest|SetHideTargetRequest|Events" "Content.Shared/Shuttles/BUIStates/NavInterfaceState.cs"Repository: space-exodus/Monolith
Length of output: 118
Remove unused import.
The Content.Shared._NF.Shuttles.Events namespace is not used anywhere in this file. No types from this namespace are referenced.
Proposed fix
-using Content.Shared._NF.Shuttles.Events; // Frontier📝 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.
| using Content.Shared._NF.Shuttles.Events; // Frontier |
🤖 Prompt for AI Agents
In `@Content.Shared/Shuttles/BUIStates/NavInterfaceState.cs` at line 3, Remove the
unused using directive for Content.Shared._NF.Shuttles.Events at the top of
NavInterfaceState.cs; open the file containing the NavInterfaceState class and
delete the line "using Content.Shared._NF.Shuttles.Events;" so only required
namespaces remain, then save/compile to confirm there are no missing references.
| # Radar Target Buttons | ||
| shuttle-console-target = Radar Target | ||
| shuttle-console-set-target = Set | ||
| shuttle-console-set-target-description = Sets a target waypoint coordinate on the radar console. | ||
| shuttle-console-hide-target = Show | ||
| shuttle-console-hide-target-description = Toggles the visibility of the target waypoint on the radar console. |
There was a problem hiding this comment.
Confusing key name: shuttle-console-hide-target resolves to "Show".
The localization key is named hide-target but its value is "Show". While the user-facing text makes sense for a toggle button, the key name implies the opposite action. This will confuse translators and developers maintaining this file.
Consider renaming to shuttle-console-show-target or shuttle-console-toggle-target to align with the displayed text.
🤖 Prompt for AI Agents
In `@Resources/Locale/en-US/_NF/shuttles/console.ftl` around lines 29 - 34, Key
name mismatch: the localization key shuttle-console-hide-target reads "Show"
which conflicts with its name; rename shuttle-console-hide-target (and its
companion shuttle-console-hide-target-description) to a name matching the
displayed text such as shuttle-console-show-target (and
shuttle-console-show-target-description) or shuttle-console-toggle-target (and
shuttle-console-toggle-target-description), update their values accordingly, and
search/update any code or other locale files that reference
shuttle-console-hide-target or shuttle-console-hide-target-description to use
the new key(s) so translations and callers remain consistent.
Порт возможности ставить метки с new frontier (ссылка на пр)
Summary by CodeRabbit
Release Notes
New Features