Skip to content

Updare StationRecords#478

Merged
Litogin merged 10 commits intocorvax-team:masterfrom
Litogin:RCZ
Mar 1, 2026
Merged

Updare StationRecords#478
Litogin merged 10 commits intocorvax-team:masterfrom
Litogin:RCZ

Conversation

@Litogin
Copy link
Collaborator

@Litogin Litogin commented Feb 1, 2026

Описание PR

Обновление станционных записей

ТРЕБУЕТСЯ ОБНОВИТЬ ЗАПИСИ ВРУЧНУЮ ИГРОКАМ

Почему / Баланс

выполнение таска PRT-109

Технические детали

Медиа

image

Требования

Согласие с условиями

  • Я согласен с условиями LICENSE и CLA.

Критические изменения

Список изменений

🆑 Litogin

  • wl-add: Добавлены общие сведенья
  • wl-add: Теперь в записях автоматически генерируются некоторые поля
  • wl-remove: Удалены поля: Имя, дата рождения, биологический вид, рост
  • wl-add: Теперь можно распечатать записи

Summary by CodeRabbit

  • New Features

    • General Records added to profiles: Full Name, Date of Birth, Country, Confederation with selector and editor sync.
    • Print support for station, medical, and criminal records: print buttons, print actions, and server-side paper output.
  • UI

    • Record views show expanded fields (Full Name, DOB, Confederation, Country, Species, Height, Languages).
    • Disabled horizontal scrolling in several viewers and added print buttons where applicable.
  • Localization

    • Russian locale keys and confederation options added.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 1, 2026

📝 Walkthrough

Walkthrough

Adds FullName, DateOfBirth, Confederation, and Country fields across shared profile, server model, DB migrations, client UIs, confederation prototypes/localization, and implements record printing flows for station/medical/criminal consoles.

Changes

Cohort / File(s) Summary
Shared — Profile & Records
Content.Shared/Preferences/HumanoidCharacterProfile.cs, Content.Shared/StationRecords/GeneralStationRecord.cs, Content.Shared/_WL/Records/PrintStationRecord.cs, Content.Shared/_WL/Records/ConfederationRecordsPrototype.cs
Adds FullName/DateOfBirth/Confederation/Country to HumanoidCharacterProfile and GeneralStationRecord; new PrintStationRecord message and confederation prototype type.
Server — DB, Model & Systems
Content.Server.Database/Model.cs, Content.Server.Database/Migrations/Postgres/...UpdateRecords.*, Content.Server.Database/Migrations/Sqlite/...UpdateRecords.*, Content.Server.Database/Migrations/...ServerDbContextModelSnapshot.cs, Content.Server/Database/ServerDbBase.cs, Content.Server/StationRecords/Systems/StationRecordsSystem.cs, Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs, Content.Server/_WL/MedicalRecords/MedicalRecordsConsoleSystem.cs, Content.Server/CriminalRecords/Systems/CriminalRecordsConsoleSystem.cs
Adds new Profile columns (full_name, date_of_birth, confederation, country) via migrations and model snapshot; propagates fields in DB↔HumanoidCharacterProfile conversions; extends CreateGeneralRecord/signatures; implements printing orchestration and per-frame print processing.
Server — Components
Content.Server/StationRecords/Components/GeneralStationRecordConsoleComponent.cs, Content.Shared/_WL/MedicalRecords/MedicalRecordsConsoleComponent.cs, Content.Shared/CriminalRecords/Components/CriminalRecordsConsoleComponent.cs
Adds print-related component fields (CanPrintEntries, TimePrint, TimePrintRemaining, PrintAudio, PrintPaperId, ContextPrint) to console components.
Client — Records UI & Editor
Content.Client/_WL/Records/RecordsTab.xaml, Content.Client/_WL/Records/RecordsTab.xaml.cs, Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs
Adds Name/Age/Country editors and a Confederation OptionButton; exposes OnGeneralRecord* events and syncs HumanoidProfileEditor to profile fields and confederation list.
Client — Console Windows & Bound UI
Content.Client/StationRecords/GeneralRecord.xaml, Content.Client/StationRecords/GeneralRecord.xaml.cs, Content.Client/StationRecords/GeneralStationRecordConsoleWindow.xaml, Content.Client/StationRecords/GeneralStationRecordConsoleWindow.xaml.cs, Content.Client/StationRecords/GeneralStationRecordConsoleBoundUserInterface.cs, Content.Client/_WL/MedicalRecords/*, Content.Client/CriminalRecords/*
Adds Print button and OnPrint/OnPrinted callbacks; switches some ScrollContainers to HScrollEnabled="False"; composes richer record text including FullName/DOB/Confederation/Country with prototype lookups; forwards print messages from client bound UI.
Localization & Prototypes
Resources/Locale/ru-RU/_WL/records/records.ftl, Resources/Prototypes/_WL/Records/сonfederationRecords.yml, Resources/Locale/ru-RU/_WL/station-records/general-station-records.ftl
Adds localization keys for general-record fields and confederations; adds confederation prototypes YAML; small localization text tweak.

Sequence Diagram(s)

sequenceDiagram
    participant User as User
    participant ClientUI as Client UI\n(RecordsTab / Console Window)
    participant BoundUI as Bound UI\n(BoundUserInterface)
    participant Server as Server\n(Console System)
    participant Paper as PaperSystem
    participant DB as Database

    User->>ClientUI: Press Print
    ClientUI->>BoundUI: Invoke OnPrint (id)
    BoundUI->>Server: SendMessage(PrintStationRecord(id))
    Server->>DB: Read record/profile (full_name, dob, confederation, country, etc.)
    Server->>Paper: Spawn paper & write assembled content
    Server->>Server: Play print audio, set print timer
    Server-->>BoundUI: Update UI state (printing, then complete)
    BoundUI-->>ClientUI: Reflect print completion
    ClientUI-->>User: UI re-enabled / print finished
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

S: Needs Review

Suggested reviewers

  • tau27

Poem

🐰 I hopped through fields both wide and small,
FullName, DOB, Confederation — I added them all.
Prototypes listed, printouts take flight,
Records now richer, tidy and bright,
Hop, merge, and celebrate this small delight!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Updare StationRecords' contains a typo ('Updare' instead of 'Update') and is only partially related to the main changeset, which adds WL-Records general-record fields (name, age, country, confederation) across multiple systems including UI, database migrations, and printing functionality. Correct the typo to 'Update StationRecords' and consider whether the title should be more specific, such as 'Add WL-Records fields to StationRecords' to better reflect the comprehensive scope of changes including database schema, UI components, and printing features.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Fix all issues with AI agents
In `@Content.Client/_WL/Records/RecordsTab.xaml`:
- Around line 10-38: Replace the hard-coded Russian label texts with
localization tokens (use the {Loc <Key>} syntax) for the header Label ("ОБЩИЕ
СВЕДЕНИЯ") and the field Labels "ФИО:", "Дата рождения:", "Конфедерация:", and
"Государство:" (refer to UI elements Label, NameEdit, AgeEdit,
ConfederationButton, CountryEdit to locate them) and add corresponding entries
to the app locale resource(s) with matching keys and Russian values so the UI
can be translated for other locales.

In `@Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs`:
- Around line 895-930: Add a bounds check before indexing _confederations in
SetConfederation to avoid out-of-range access while retaining the UI sync call;
specifically, in SetConfederation check that args.Id is within _confederations
(e.g., cast to uint and compare against _confederations.Count) and return early
if out of range, but keep the existing _confederationButton.SelectId(args.Id)
and the Profile = Profile.WithConfederation(_confederations[args.Id]); call
order intact so selection state stays synchronized.
- Around line 1989-2006: The confederation dropdown can retain a previous
selection when Profile?.Confederation is null/empty or doesn't match any entry;
update the loop controlling _confederationButton so it tracks whether a match
was found while iterating _confederations and, after the loop, call
_confederationButton.Clear() if no match was found (ensure you handle
_confederationButton and _confederations being non-null before operating);
mirror the pattern used in UpdateSexControls() to clear the control when the
profile value is empty or unmatched.

In `@Content.Shared/Preferences/HumanoidCharacterProfile.cs`:
- Around line 811-814: The assignments for FullName, DateOfBirth, Confederation,
and Country must be validated like the existing
MedicalRecord/SecurityRecord/EmploymentRecord handling: apply
FormattedMessage.RemoveMarkupOrThrow() to strip/validate markup and enforce a
max-length check using appropriate constants (create or reuse constants similar
to the others, e.g., MaxFullNameLength, MaxConfederationLength,
MaxCountryLength, MaxDateOfBirthLength) before assigning to the fields in
HumanoidCharacterProfile; throw or handle errors consistently with the pattern
used for MedicalRecord/SecurityRecord/EmploymentRecord when validation fails.
🧹 Nitpick comments (4)
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (2)

199-207: Avoid hard‑coded confederation strings in code.
These are user‑facing values; hard-coding blocks localization and future updates. Consider sourcing from localization/prototypes or config instead.


755-760: Remove the commented‑out handler stub.
Leaving dead code here adds noise and invites confusion.

🧹 Proposed cleanup
-            //_confederationButton.OnItemSelected += args =>
-            //{
-
-            //};
Content.Shared/Preferences/HumanoidCharacterProfile.cs (2)

358-365: Minor style inconsistency: missing blank lines between property declarations.

Other property declarations in this file (e.g., MedicalRecord, SecurityRecord, EmploymentRecord at lines 350-357) have blank lines separating them. Consider adding blank lines for consistency.

🔧 Suggested formatting
         public string EmploymentRecord { get; set; } = string.Empty;
+
         [DataField]
         public string FullName { get; set; } = string.Empty;
+
         [DataField]
         public string DateOfBirth { get; set; } = string.Empty;
+
         [DataField]
         public string Confederation { get; set; } = string.Empty;
+
         [DataField]
         public string Country { get; set; } = string.Empty;

203-206: Missing comment on line 206.

Line 206 is missing the // WL-Records comment that lines 203-205 have. This affects consistency when searching for WL-Records related code.

🔧 Suggested fix
-            Country = country;
+            Country = country; // WL-Records

Comment on lines +10 to +38
<Label Text="ОБЩИЕ СВЕДЕНИЯ" StyleClasses="LabelBig" FontColorOverride="#2196F3"/>
<PanelContainer StyleClasses="Inset" Margin="0 5 0 0" HorizontalExpand="True">
<PanelContainer.PanelOverride>
<graphics:StyleBoxFlat BackgroundColor="#1E1E1E" ContentMarginLeftOverride="5" ContentMarginBottomOverride="5"/>
</PanelContainer.PanelOverride>
<BoxContainer Orientation="Vertical" HorizontalExpand="True">

<BoxContainer Orientation="Horizontal" HorizontalExpand="True">
<Label Text="ФИО:" />
<Control MinWidth="10"/>
<LineEdit Name="NameEdit" MinWidth="250" Access="Public"/>
</BoxContainer>

<BoxContainer Orientation="Horizontal" HorizontalExpand="True">
<Label Text="Дата рождения:" />
<Control MinWidth="10"/>
<LineEdit Name="AgeEdit" MinWidth="150" Access="Public"/>
</BoxContainer>

<BoxContainer Orientation="Horizontal" HorizontalExpand="True">
<Label Text="Конфедерация:" />
<Control MinWidth="10"/>
<OptionButton Name="ConfederationButton" HorizontalAlignment="Right" Access="Public"/>
</BoxContainer>

<BoxContainer Orientation="Horizontal" HorizontalExpand="True">
<Label Text="Государство:" />
<Control MinWidth="10"/>
<LineEdit Name="CountryEdit" MinWidth="250" Access="Public"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Localize the new general‑info labels.
Other sections use {Loc ...}; these hard-coded strings won’t translate for non‑Russian locales. Please add localization keys and reference them here.

💡 Suggested XAML change (add matching locale entries)
- <Label Text="ОБЩИЕ СВЕДЕНИЯ" StyleClasses="LabelBig" FontColorOverride="#2196F3"/>
+ <Label Text="{Loc 'records-general-info'}" StyleClasses="LabelBig" FontColorOverride="#2196F3"/>

- <Label Text="ФИО:" />
+ <Label Text="{Loc 'records-full-name'}" />

- <Label Text="Дата рождения:" />
+ <Label Text="{Loc 'records-date-of-birth'}" />

- <Label Text="Конфедерация:" />
+ <Label Text="{Loc 'records-confederation'}" />

- <Label Text="Государство:" />
+ <Label Text="{Loc 'records-country'}" />
📝 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.

Suggested change
<Label Text="ОБЩИЕ СВЕДЕНИЯ" StyleClasses="LabelBig" FontColorOverride="#2196F3"/>
<PanelContainer StyleClasses="Inset" Margin="0 5 0 0" HorizontalExpand="True">
<PanelContainer.PanelOverride>
<graphics:StyleBoxFlat BackgroundColor="#1E1E1E" ContentMarginLeftOverride="5" ContentMarginBottomOverride="5"/>
</PanelContainer.PanelOverride>
<BoxContainer Orientation="Vertical" HorizontalExpand="True">
<BoxContainer Orientation="Horizontal" HorizontalExpand="True">
<Label Text="ФИО:" />
<Control MinWidth="10"/>
<LineEdit Name="NameEdit" MinWidth="250" Access="Public"/>
</BoxContainer>
<BoxContainer Orientation="Horizontal" HorizontalExpand="True">
<Label Text="Дата рождения:" />
<Control MinWidth="10"/>
<LineEdit Name="AgeEdit" MinWidth="150" Access="Public"/>
</BoxContainer>
<BoxContainer Orientation="Horizontal" HorizontalExpand="True">
<Label Text="Конфедерация:" />
<Control MinWidth="10"/>
<OptionButton Name="ConfederationButton" HorizontalAlignment="Right" Access="Public"/>
</BoxContainer>
<BoxContainer Orientation="Horizontal" HorizontalExpand="True">
<Label Text="Государство:" />
<Control MinWidth="10"/>
<LineEdit Name="CountryEdit" MinWidth="250" Access="Public"/>
<Label Text="{Loc 'records-general-info'}" StyleClasses="LabelBig" FontColorOverride="#2196F3"/>
<PanelContainer StyleClasses="Inset" Margin="0 5 0 0" HorizontalExpand="True">
<PanelContainer.PanelOverride>
<graphics:StyleBoxFlat BackgroundColor="#1E1E1E" ContentMarginLeftOverride="5" ContentMarginBottomOverride="5"/>
</PanelContainer.PanelOverride>
<BoxContainer Orientation="Vertical" HorizontalExpand="True">
<BoxContainer Orientation="Horizontal" HorizontalExpand="True">
<Label Text="{Loc 'records-full-name'}" />
<Control MinWidth="10"/>
<LineEdit Name="NameEdit" MinWidth="250" Access="Public"/>
</BoxContainer>
<BoxContainer Orientation="Horizontal" HorizontalExpand="True">
<Label Text="{Loc 'records-date-of-birth'}" />
<Control MinWidth="10"/>
<LineEdit Name="AgeEdit" MinWidth="150" Access="Public"/>
</BoxContainer>
<BoxContainer Orientation="Horizontal" HorizontalExpand="True">
<Label Text="{Loc 'records-confederation'}" />
<Control MinWidth="10"/>
<OptionButton Name="ConfederationButton" HorizontalAlignment="Right" Access="Public"/>
</BoxContainer>
<BoxContainer Orientation="Horizontal" HorizontalExpand="True">
<Label Text="{Loc 'records-country'}" />
<Control MinWidth="10"/>
<LineEdit Name="CountryEdit" MinWidth="250" Access="Public"/>
🤖 Prompt for AI Agents
In `@Content.Client/_WL/Records/RecordsTab.xaml` around lines 10 - 38, Replace the
hard-coded Russian label texts with localization tokens (use the {Loc <Key>}
syntax) for the header Label ("ОБЩИЕ СВЕДЕНИЯ") and the field Labels "ФИО:",
"Дата рождения:", "Конфедерация:", and "Государство:" (refer to UI elements
Label, NameEdit, AgeEdit, ConfederationButton, CountryEdit to locate them) and
add corresponding entries to the app locale resource(s) with matching keys and
Russian values so the UI can be translated for other locales.

Comment on lines +895 to +930
private void OnGeneralRecordNameChanged(string content)
{
if (Profile is null)
return;

Profile = Profile.WithFullName(content);
SetDirty();
}
private void OnGeneralRecordDateOfBirthChanged(string content)
{
if (Profile is null)
return;

Profile = Profile.WithDateOfBirth(content);
SetDirty();
}
private void OnGeneralRecordCountryChanged(string content)
{
if (Profile is null)
return;

Profile = Profile.WithCountry(content);
SetDirty();
}
private void SetConfederation(OptionButton.ItemSelectedEventArgs args)
{
if (_confederationButton is null)
return;

if (Profile is null)
return;

_confederationButton.SelectId(args.Id);
Profile = Profile.WithConfederation(_confederations[args.Id]);
SetDirty();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's understand the project structure
git ls-files | grep -E "\.(xaml|xaml\.cs)$" | head -20

Repository: corvax-team/ss14-wl

Length of output: 1256


🏁 Script executed:

# Find OptionButton definition
rg "class OptionButton" -A 10

Repository: corvax-team/ss14-wl

Length of output: 1104


🏁 Script executed:

# Look for SelectId method definition
rg "void SelectId|SelectId\(" -B 2 -A 5

Repository: corvax-team/ss14-wl

Length of output: 50375


🏁 Script executed:

# Search for OnItemSelected handler in the HumanoidProfileEditor file
rg "OnItemSelected|SetConfederation" Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs -B 2 -A 5

Repository: corvax-team/ss14-wl

Length of output: 2389


🏁 Script executed:

# Find OptionButton class definition
rg "class OptionButton" -A 30 --type cs

Repository: corvax-team/ss14-wl

Length of output: 2175


🏁 Script executed:

# Search for SelectId method implementation
rg "void SelectId|SelectId\(" --type cs -B 2 -A 10 | head -100

Repository: corvax-team/ss14-wl

Length of output: 9087


🏁 Script executed:

# Look for OptionButton in the UI framework
find . -name "*.cs" -type f | xargs grep -l "class OptionButton" 2>/dev/null

Repository: corvax-team/ss14-wl

Length of output: 127


🏁 Script executed:

# Look for OptionButton usage to understand the Select vs SelectId pattern
rg "\.Select\(|\.SelectId\(" --type cs | head -50

Repository: corvax-team/ss14-wl

Length of output: 7466


🏁 Script executed:

# Check if there's a RobustToolbox or external reference
find . -name "*.csproj" -type f | head -5 | xargs grep -l "RobustToolbox\|OptionButton" 2>/dev/null

Repository: corvax-team/ss14-wl

Length of output: 256


🏁 Script executed:

# Look at the HumanoidProfileEditor file directly around the SetConfederation method
sed -n '890,935p' Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs

Repository: corvax-team/ss14-wl

Length of output: 1285


🏁 Script executed:

# Check for bounds checking patterns in the codebase with _confederations
rg "_confederations\[" Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs -B 3 -A 3

Repository: corvax-team/ss14-wl

Length of output: 713


🏁 Script executed:

# Check the .csproj file for RobustToolbox reference
cat Content.Client/Content.Client.csproj | grep -A 5 -B 5 "RobustToolbox\|OptionButton"

Repository: corvax-team/ss14-wl

Length of output: 1109


🏁 Script executed:

# Look for more context on how confederations is populated and used
rg "_confederations" Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs -B 2 -A 2

Repository: corvax-team/ss14-wl

Length of output: 1087


🏁 Script executed:

# Check if there's any validation before indexing _confederations anywhere else in this file
rg "args.Id|args\.Id" Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs -B 3 -A 3

Repository: corvax-team/ss14-wl

Length of output: 1673


🏁 Script executed:

# Search for OptionButton in RobustToolbox directory if it exists
find . -name "OptionButton.cs" -o -name "*OptionButton*" -type f 2>/dev/null | grep -v ".xaml\|StyleSheetlets"

Repository: corvax-team/ss14-wl

Length of output: 125


🏁 Script executed:

# Check if RobustToolbox is a submodule or external
ls -la RobustToolbox 2>/dev/null || echo "RobustToolbox not found as directory"

Repository: corvax-team/ss14-wl

Length of output: 175


🏁 Script executed:

# Look at other similar validation patterns in the codebase for args.Id
rg "args\.Id.*>=" --type cs -B 2 -A 2 | head -50

Repository: corvax-team/ss14-wl

Length of output: 45


🏁 Script executed:

# Check the Options.OnItemSelected pattern in the same file to see if there's validation
sed -n '700,750p' Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs

Repository: corvax-team/ss14-wl

Length of output: 2081


🏁 Script executed:

# Look for bounds validation pattern with ItemSelectedEventArgs
rg "ItemSelectedEventArgs" --type cs -B 5 -A 8 | head -80

Repository: corvax-team/ss14-wl

Length of output: 6927


🏁 Script executed:

# Search for any error handling or validation of args.Id in handlers
rg "args\.Id.*<|args\.Id.*>=" --type cs

Repository: corvax-team/ss14-wl

Length of output: 45


🏁 Script executed:

# Check if there are any cases where handlers guard against invalid IDs
rg "if.*args\.Id|if.*args\.ButtonId" --type cs -B 2 -A 5 | head -60

Repository: corvax-team/ss14-wl

Length of output: 4621


Add bounds validation before accessing _confederations, but keep SelectId to maintain selection state sync.

The bounds check is good defensive programming: if ((uint) args.Id >= (uint) _confederations.Count) return; before indexing _confederations[args.Id]. However, removing SelectId is inconsistent with the same pattern used in other handlers in this file (e.g., SexButton, SpeciesButton, PreferenceUnavailableButton all call SelectId in their OnItemSelected handlers). The ubiquitous use of SelectId within item selection handlers throughout the codebase indicates it does not re-emit OnItemSelected—this pattern would be broken framework-wide if it did. Keep the SelectId call for proper UI state synchronization; just add the bounds check.

🤖 Prompt for AI Agents
In `@Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs` around lines 895 -
930, Add a bounds check before indexing _confederations in SetConfederation to
avoid out-of-range access while retaining the UI sync call; specifically, in
SetConfederation check that args.Id is within _confederations (e.g., cast to
uint and compare against _confederations.Count) and return early if out of
range, but keep the existing _confederationButton.SelectId(args.Id) and the
Profile = Profile.WithConfederation(_confederations[args.Id]); call order intact
so selection state stays synchronized.

Comment on lines +1989 to +2006
if (_generalRecordNameEdit != null)
_generalRecordNameEdit.Text = Profile?.FullName ?? "";

if (_generalRecordAgeEdit != null)
_generalRecordAgeEdit.Text = Profile?.DateOfBirth ?? "";

if (_generalRecordCountryEdit != null)
_generalRecordCountryEdit.Text = Profile?.Country ?? "";

if (_confederationButton != null)
for (var i = 0; i < _confederations.Count; i++)
{

if (Profile?.Confederation.Equals(_confederations[i]) == true)
{
_confederationButton.SelectId(i);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's find and examine the HumanoidProfileEditor.xaml.cs file
fd -t f "HumanoidProfileEditor.xaml.cs" | head -5

Repository: corvax-team/ss14-wl

Length of output: 116


🏁 Script executed:

#!/bin/bash
# Read the file around lines 1989-2006
wc -l Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs
echo "---"
sed -n '1980,2020p' Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs

Repository: corvax-team/ss14-wl

Length of output: 1642


🏁 Script executed:

#!/bin/bash
# Search for _confederations initialization and usage
echo "=== _confederations initialization ==="
rg "_confederations\s*=" Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs -B 2 -A 2

echo -e "\n=== _confederationButton initialization ==="
rg "_confederationButton\s*=" Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs -B 2 -A 2

echo -e "\n=== OptionButton class/type check ==="
rg "OptionButton" Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs -B 1 -A 1 | head -30

Repository: corvax-team/ss14-wl

Length of output: 862


🏁 Script executed:

#!/bin/bash
# Look at the SetConfederation method and surrounding context
echo "=== SetConfederation method ==="
rg "SetConfederation" Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs -B 2 -A 10

echo -e "\n=== OptionButton definition/API ==="
fd -t f "OptionButton" --extension cs | head -5

Repository: corvax-team/ss14-wl

Length of output: 1082


🏁 Script executed:

#!/bin/bash
# Find and examine OptionButton class
cat -n Content.Client/Stylesheets/Sheetlets/OptionButtonSheetlet.cs | head -100

Repository: corvax-team/ss14-wl

Length of output: 1442


🏁 Script executed:

#!/bin/bash
# Search for OptionButton class definition and methods
echo "=== Searching for OptionButton class ==="
rg "class OptionButton" --type cs

echo -e "\n=== Searching for SelectId method ==="
rg "SelectId\(" Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs -B 1 -A 3

echo -e "\n=== Searching for OptionButton methods in codebase ==="
rg "\.OptionButton\?\." Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs -o

Repository: corvax-team/ss14-wl

Length of output: 2312


🏁 Script executed:

#!/bin/bash
# Look at how other controls handle missing/no match scenarios
echo "=== UpdateGenders/UpdateSpecies patterns ==="
rg "UpdateGenders|UpdateSpecies|UpdateSex" Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs -A 8 | head -60

echo -e "\n=== Profile class to check Confederation property ==="
fd -t f "Profile\\.cs" | head -3

Repository: corvax-team/ss14-wl

Length of output: 1650


🏁 Script executed:

#!/bin/bash
# Get the full UpdateSexControls method
echo "=== Full UpdateSexControls method ==="
rg "private void UpdateSexControls" Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs -A 20

echo -e "\n=== Full UpdateGenderControls method ==="
rg "private void UpdateGenderControls" Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs -A 15

echo -e "\n=== Check Confederation property in Profile ==="
rg "Confederation" Content.Shared/Preferences/HumanoidCharacterProfile.cs -B 2 -A 2

Repository: corvax-team/ss14-wl

Length of output: 3062


🏁 Script executed:

#!/bin/bash
# Search for how Clear() is used with option buttons
echo "=== Clear() usage with buttons ==="
rg "Button\.Clear\(\)" Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs -B 2 -A 2

echo -e "\n=== Check if confederation can be empty in practice ==="
rg "Profile\.Confederation|WithConfederation" Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs | head -20

Repository: corvax-team/ss14-wl

Length of output: 443


Reset confederation selection when profile value is empty or unmatched.

The current logic persists previous selection when Profile?.Confederation is empty (default string.Empty) or unmatched, which can mislead users. Calling _confederationButton.Clear() when no match is found would address this, following the established pattern in UpdateSexControls().

Note: The suggested diff in the original comment assumes index 0 is a "None/—" entry, but _confederations actually contains four real confederations (sorted alphabetically), so that approach would incorrectly select the first confederation for empty profiles. Instead, clear the selection when no match is found:

🔧 Corrected approach
            if (_confederationButton != null)
-                for (var i = 0; i < _confederations.Count; i++)
+            {
+                var idx = _confederations.FindIndex(c =>
+                    string.Equals(Profile?.Confederation, c, StringComparison.CurrentCultureIgnoreCase));
+
+                if (idx >= 0)
+                    _confederationButton.SelectId(idx);
+                else
+                    _confederationButton.Clear();
+            }
📝 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.

Suggested change
if (_generalRecordNameEdit != null)
_generalRecordNameEdit.Text = Profile?.FullName ?? "";
if (_generalRecordAgeEdit != null)
_generalRecordAgeEdit.Text = Profile?.DateOfBirth ?? "";
if (_generalRecordCountryEdit != null)
_generalRecordCountryEdit.Text = Profile?.Country ?? "";
if (_confederationButton != null)
for (var i = 0; i < _confederations.Count; i++)
{
if (Profile?.Confederation.Equals(_confederations[i]) == true)
{
_confederationButton.SelectId(i);
}
}
if (_generalRecordNameEdit != null)
_generalRecordNameEdit.Text = Profile?.FullName ?? "";
if (_generalRecordAgeEdit != null)
_generalRecordAgeEdit.Text = Profile?.DateOfBirth ?? "";
if (_generalRecordCountryEdit != null)
_generalRecordCountryEdit.Text = Profile?.Country ?? "";
if (_confederationButton != null)
{
var idx = _confederations.FindIndex(c =>
string.Equals(Profile?.Confederation, c, StringComparison.CurrentCultureIgnoreCase));
if (idx >= 0)
_confederationButton.SelectId(idx);
else
_confederationButton.Clear();
}
🤖 Prompt for AI Agents
In `@Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs` around lines 1989 -
2006, The confederation dropdown can retain a previous selection when
Profile?.Confederation is null/empty or doesn't match any entry; update the loop
controlling _confederationButton so it tracks whether a match was found while
iterating _confederations and, after the loop, call _confederationButton.Clear()
if no match was found (ensure you handle _confederationButton and
_confederations being non-null before operating); mirror the pattern used in
UpdateSexControls() to clear the control when the profile value is empty or
unmatched.

Comment on lines +811 to +814
var fullName = FullName;
var dateOfBirth = DateOfBirth;
var confederation = Confederation;
var country = Country;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing input validation for new record fields.

The new fields (FullName, DateOfBirth, Confederation, Country) are assigned directly without validation, unlike the existing record fields (MedicalRecord, SecurityRecord, EmploymentRecord) which have both length limits and markup removal (see lines 802-810).

This inconsistency could allow:

  • Markup injection if FormattedMessage.RemoveMarkupOrThrow() isn't applied
  • Unbounded string lengths if no max length is enforced
🛡️ Suggested validation
             // WL-Records-Start
             var medicalRecord = MedicalRecord.Length > MaxRecordLength
                 ? FormattedMessage.RemoveMarkupOrThrow(MedicalRecord)[..MaxRecordLength]
                 : FormattedMessage.RemoveMarkupOrThrow(MedicalRecord);
             var securityRecord = SecurityRecord.Length > MaxRecordLength
                 ? FormattedMessage.RemoveMarkupOrThrow(SecurityRecord)[..MaxRecordLength]
                 : FormattedMessage.RemoveMarkupOrThrow(SecurityRecord);
             var employmentRecord = EmploymentRecord.Length > MaxRecordLength
                 ? FormattedMessage.RemoveMarkupOrThrow(EmploymentRecord)[..MaxRecordLength]
                 : FormattedMessage.RemoveMarkupOrThrow(EmploymentRecord);
-            var fullName = FullName;
-            var dateOfBirth = DateOfBirth;
-            var confederation = Confederation;
-            var country = Country;
+            var fullName = FullName.Length > maxNameLength
+                ? FormattedMessage.RemoveMarkupOrThrow(FullName)[..maxNameLength]
+                : FormattedMessage.RemoveMarkupOrThrow(FullName);
+            var dateOfBirth = FormattedMessage.RemoveMarkupOrThrow(DateOfBirth);
+            var confederation = FormattedMessage.RemoveMarkupOrThrow(Confederation);
+            var country = FormattedMessage.RemoveMarkupOrThrow(Country);
             // WL-Records-End

Note: Adjust the max length constants as appropriate for each field type.

🤖 Prompt for AI Agents
In `@Content.Shared/Preferences/HumanoidCharacterProfile.cs` around lines 811 -
814, The assignments for FullName, DateOfBirth, Confederation, and Country must
be validated like the existing MedicalRecord/SecurityRecord/EmploymentRecord
handling: apply FormattedMessage.RemoveMarkupOrThrow() to strip/validate markup
and enforce a max-length check using appropriate constants (create or reuse
constants similar to the others, e.g., MaxFullNameLength,
MaxConfederationLength, MaxCountryLength, MaxDateOfBirthLength) before assigning
to the fields in HumanoidCharacterProfile; throw or handle errors consistently
with the pattern used for MedicalRecord/SecurityRecord/EmploymentRecord when
validation fails.

private OptionButton? _confederationButton; // WL-Recordss

private List<string> _confederations = new List<string>() {
"Конфедерация Орионских Государств", "СоцКон", "Межвидовой Альянс", "Священная Империя Эдема" };
Copy link
Collaborator

Choose a reason for hiding this comment

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

Нужен метафронт, дальние земли и отсутствтует

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs (1)

18-26: ⚠️ Potential issue | 🔴 Critical

Missing IoCManager.InjectDependencies(this)_prototypeManager will be null at runtime.

The [Dependency] attribute on Line 18 requires IoCManager.InjectDependencies(this) to be called (typically in the constructor) to populate the field. Without it, _prototypeManager stays default! (null), causing a NullReferenceException when GenerateMedicalRecord is invoked.

🐛 Proposed fix
     public MedicalRecordsConsoleWindow()
     {
         RobustXamlLoader.Load(this);
+        IoCManager.InjectDependencies(this);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs` around
lines 18 - 26, The constructor for MedicalRecordsConsoleWindow is missing a call
to IoCManager.InjectDependencies(this), so the [Dependency] field
_prototypeManager remains uninitialized; update the MedicalRecordsConsoleWindow
constructor to call IoCManager.InjectDependencies(this) (before using
RobustXamlLoader.Load if your pattern requires injection first) so
_prototypeManager is populated and GenerateMedicalRecord (and any other methods
using _prototypeManager) won't throw NullReferenceException.
🧹 Nitpick comments (7)
Content.Shared/StationRecords/GeneralStationRecord.cs (1)

62-76: Consider adding XML doc comments for the new fields.

The existing fields in this record (Name, Age, Species, etc.) all have XML documentation. The new WL-Records fields lack them, which is a minor consistency gap.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Shared/StationRecords/GeneralStationRecord.cs` around lines 62 - 76,
Add XML documentation for the newly added fields on GeneralStationRecord:
Fullname, DateOfBirth, Confederation, Country, and Height. For each field (e.g.,
public string Fullname, public string DateOfBirth, public string Confederation,
public string Country, public int Height) add a concise /// <summary>…</summary>
comment describing the purpose/format (e.g., full legal name, birth date format,
confederation identifier, country of origin, height in centimeters) to match the
style of the existing XML docs on other fields.
Content.Server/StationRecords/Systems/StationRecordsSystem.cs (1)

137-159: Method has 17 parameters — consider grouping into a data object.

CreateGeneralRecord now accepts 17 parameters, making call sites unreadable (see Line 106). The WL-Records fields (fullName, dateOfBirth, confederation, country, height) could be bundled into a small record/struct to reduce parameter count and improve maintainability.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Server/StationRecords/Systems/StationRecordsSystem.cs` around lines
137 - 159, Create a small value object (e.g., WlPersonalInfo or WlRecords) to
encapsulate the WL-Records fields (fullName, dateOfBirth, confederation,
country, height) and replace those five parameters in the CreateGeneralRecord
method signature with a single parameter of that type; update the
CreateGeneralRecord declaration and all call sites (including the caller that
passes these values) to construct and pass the new object, and ensure any
serialization/assignment inside CreateGeneralRecord uses the new object's
properties instead of individual parameters.
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (2)

757-761: Remove commented-out dead code.

This block is a leftover from development. The confederation button handler is already wired via _recordsTab.OnGeneralRecordConfederationChanged += SetConfederation; at line 856. Remove this to avoid confusion.

Proposed fix
-            //_confederationButton.OnItemSelected += args =>
-            //{
-
-            //};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs` around lines 757 -
761, Remove the dead commented-out handler block for
_confederationButton.OnItemSelected; it's unused and duplicates the actual
wiring via _recordsTab.OnGeneralRecordConfederationChanged += SetConfederation.
Delete the commented lines starting with //_confederationButton.OnItemSelected
and the inner commented braces to clean up the file and avoid confusion while
keeping SetConfederation and _recordsTab intact.

858-872: Confederations are sorted by localization key, not by localized display name.

_confederations.Sort at line 860 compares a.Name and b.Name which are prototype localization keys (e.g., "records-confederation-social"), not the actual localized strings. This means the dropdown order is determined by key alphabetical order rather than the user-visible text. Compare with RefreshSpecies() at line 1111 which uses the same approach — so this may be a pre-existing pattern, but it can produce a non-intuitive ordering for the user.

Sort by localized name instead
-            _confederations.Sort((a, b) => string.Compare(a.Name, b.Name, StringComparison.CurrentCultureIgnoreCase));
+            _confederations.Sort((a, b) => string.Compare(Loc.GetString(a.Name), Loc.GetString(b.Name), StringComparison.CurrentCultureIgnoreCase));
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs` around lines 858 -
872, The confederation list is currently sorted by prototype localization key
via _confederations.Sort((a, b) => string.Compare(a.Name, b.Name, ...)), which
orders by keys not user-visible text; change the sort to compare the localized
display names (use Loc.GetString(a.Name) vs Loc.GetString(b.Name) with
StringComparison.CurrentCultureIgnoreCase) before you call
_recordsTab.ConfederationButton.AddItem so the dropdown is ordered by the actual
localized names while leaving the subsequent ID check (_confederations[i].ID ==
"NoConfederation") and AddItem/SelectId logic unchanged.
Content.Client/StationRecords/GeneralStationRecordConsoleBoundUserInterface.cs (1)

26-26: Misleading lambda parameter name: id should be content.

OnPrint is Action<string> where the string represents record content to print, not an identifier. Compare with OnDeleted += id => which correctly uses id for Action<uint>.

-        _window.OnPrint += id => SendMessage(new PrintStationRecord(id));
+        _window.OnPrint += content => SendMessage(new PrintStationRecord(content));
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@Content.Client/StationRecords/GeneralStationRecordConsoleBoundUserInterface.cs`
at line 26, The lambda uses a misleading parameter name: change the
_window.OnPrint handler to use a descriptive parameter (e.g., content) instead
of id and pass that content into PrintStationRecord; locate the OnPrint
subscription in GeneralStationRecordConsoleBoundUserInterface (compare with the
OnDeleted => id => handler) and update the lambda parameter name and usage so it
reflects Action<string> content rather than an identifier.
Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs (2)

52-52: Hardcoded prototype ID "Paper" — extract to a constant or component field.

If a different paper prototype is ever needed (e.g., a pre-stamped or styled variant), this would require a code change. Consider making it configurable via the component.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs`
at line 52, The code hardcodes the paper prototype id in
GeneralStationRecordConsoleSystem (the Spawn("Paper",
Transform(ent.Owner).Coordinates) call) which should be configurable; change
this to read a prototype id from the relevant component or a private constant
(e.g., add a string PaperPrototype field/property on the
StationRecordConsoleComponent or a private const string PaperProto) and use that
value in the Spawn call instead of the literal "Paper" so variants can be
provided via the component or easily changed in one place.

50-50: Hardcoded audio path — consider using a SoundSpecifier on the component.

Using a raw string path "/Audio/Machines/printer.ogg" is fragile and not overridable per-prototype. The idiomatic approach in SS14 is to declare a SoundSpecifier field on the component (or use a prototype-defined sound) so it can be configured in YAML without code changes.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs`
at line 50, The code currently calls
_audioSystem.PlayPvs("/Audio/Machines/printer.ogg", ent.Owner) with a hardcoded
path; instead add a SoundSpecifier field/property on the corresponding component
(e.g., GeneralStationRecordConsoleComponent) to hold the sound (allowing
YAML/prototype overrides) and change the system to pass that SoundSpecifier to
the audio system (use the appropriate Play/PlayPvs overload that accepts a
SoundSpecifier and entity/transform). Update the component defaults in
prototypes/YAML to reference the desired sound and remove the string literal
from GeneralStationRecordConsoleSystem.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs`:
- Around line 110-119: The GenerateMedicalRecord method currently misuses the
null-coalescing operator due to operator precedence and risks throwing from
_prototypeManager.Index<SpeciesPrototype>(stationRecord.Species); fix by
parenthesizing each concatenation before applying ?? fallbacks and guard the
species lookup with TryIndex: ensure expressions like
(Loc.GetString("records-full-name-edit") + (stationRecord.Fullname ??
stationRecord.Name)) and (Loc.GetString("records-date-of-birth-edit") +
(stationRecord.DateOfBirth ?? "N|A")) are used, replace the direct Index call
with TryIndex for SpeciesPrototype to safely get the species name (with a
fallback via Loc.GetString), and remove the redundant trailing """ ?? ... by
applying the final overall fallback to the constructed string if needed (e.g.,
use (...) ?? Loc.GetString("medical-records-console-no-record")); keep
references to GenerateMedicalRecord, GeneralStationRecord,
stationRecord.Fullname, stationRecord.DateOfBirth, stationRecord.Height,
stationRecord.MedicalRecord, _prototypeManager.TryIndex and Loc.GetString when
making changes.

In `@Content.Client/CriminalRecords/CriminalRecordsConsoleWindow.xaml.cs`:
- Around line 271-281: The concatenations in the SecurityRecord.Text raw string
use "??" incorrectly due to operator precedence so the fallbacks never run;
update the expression so each nullable field is parenthesized (e.g.
Loc.GetString("records-full-name-edit") + (stationRecord.Fullname ??
stationRecord.Name)) and similarly for DateOfBirth, Country and the
Confederation/Species lookups; also protect the prototype lookups used in
_proto.Index<ConfederationRecordsPrototype>(stationRecord.Confederation) and
_proto.Index<SpeciesPrototype>(stationRecord.Species) by checking or using a
safe lookup (e.g. _proto.HasIndex or TryIndex and fall back to
Loc.GetString("N|A") or a default name) before calling Loc.GetString, and remove
the final ?? on the raw string (or instead conditionally set SecurityRecord.Text
to the "no-security-record" message if stationRecord is null/empty).

In `@Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs`:
- Line 207: The inline comment trailing the private field _confederations
contains a typo ("// WL-Recordss"); update the comment on the field declaration
for _confederations (private List<ConfederationRecordsPrototype> _confederations
= new();) to correct the typo (e.g., "// WL-Records") or remove the comment if
unnecessary so the comment reads cleanly.

In `@Content.Client/StationRecords/GeneralRecord.xaml`:
- Line 22: Replace the hardcoded Russian label on the PrintButton with a
localization key: update the PrintButton Text attribute (Name="PrintButton") to
use the same localization binding style used elsewhere in this XAML (e.g.,
Text="{Loc 'stationrecords.print'}" or similar key naming), and then add the
matching key/value entry into the appropriate localization .ftl files (e.g.,
stationrecords.print = "Распечатать") so the UI uses the localized string.

In `@Content.Client/StationRecords/GeneralRecord.xaml.cs`:
- Around line 27-36: The concatenations in the Record.Text assignment use +
before ?? so the fallback operands are never used; parenthesize or restructure
each expression so the null-coalescing applies to the field (e.g.
(record.Fullname ?? record.Name) combined with Loc.GetString), and remove the
final """ ?? ... since a raw string is never null; additionally guard prototype
lookups by checking record.Confederation and record.Species before calling
prototypeManager.Index (e.g. string.IsNullOrEmpty(record.Confederation) or
prototypeManager.HasIndex/TryIndex equivalents) and use a safe fallback like
Loc.GetString("N|A") when the prototype id is empty/invalid so
prototypeManager.Index<ConfederationRecordsPrototype>(record.Confederation) and
prototypeManager.Index<SpeciesPrototype>(record.Species) cannot throw.

In `@Content.Client/StationRecords/GeneralStationRecordConsoleWindow.xaml.cs`:
- Line 1: Remove the unused using directive referencing
Content.Shared.EntityEffects.Effects.EntitySpawning from
GeneralStationRecordConsoleWindow.xaml.cs; locate the top-of-file import lines
and delete the line "using Content.Shared.EntityEffects.Effects.EntitySpawning;"
so the file no longer contains an unreferenced namespace.

In `@Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs`:
- Around line 48-56: The handler OnRecordPrint must not trust args.Content from
the client; instead, fetch the console's ActiveKey and the station's record via
the same store/lookup used by UpdateUserInterface (use the
GeneralStationRecordConsoleComponent.ActiveKey and the station record
storage/read methods) and build the printable string server-side, then set that
on the spawned paper via _paperSystem.SetContent; also add an access check
analogous to OnRecordDelete (check the appropriate permission flag or method on
GeneralStationRecordConsoleComponent such as CanDeleteEntries or
introduce/consult a CanPrintEntries check) and bail out (and optionally log or
play denial audio) if the caller lacks permission before spawning/printing.

In `@Resources/Locale/ru-RU/_WL/records/records.ftl`:
- Line 6: Fix the typo in the localization string for the key records-general by
replacing "ОБЩИЕ СВЕДЕНЬЯ" with the correct Russian word "ОБЩИЕ СВЕДЕНИЯ" in
Resources/Locale/ru-RU/_WL/records/records.ftl so the records-general entry uses
the proper spelling.
- Line 29: Update the localization string identified by the key
records-confederation-no: correct the misspelling "конфедирации" to
"конфедерации" and replace the colloquial "Нету" with the formal "Нет" so the
value becomes "Нет конфедерации". Locate the records-confederation-no entry in
Resources/Locale/ru-RU/_WL/records/records.ftl and update only the string value
accordingly.

---

Outside diff comments:
In `@Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs`:
- Around line 18-26: The constructor for MedicalRecordsConsoleWindow is missing
a call to IoCManager.InjectDependencies(this), so the [Dependency] field
_prototypeManager remains uninitialized; update the MedicalRecordsConsoleWindow
constructor to call IoCManager.InjectDependencies(this) (before using
RobustXamlLoader.Load if your pattern requires injection first) so
_prototypeManager is populated and GenerateMedicalRecord (and any other methods
using _prototypeManager) won't throw NullReferenceException.

---

Duplicate comments:
In `@Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs`:
- Around line 926-937: SetConfederation currently indexes
_confederations[_args.Id] without validating args.Id, risking
IndexOutOfRangeException; add a guard that verifies args.Id is within
0.._confederations.Count-1 (and non-negative) before calling
_confederationButton.SelectId or using _confederations[args.Id]; if out of
range, bail out (or handle gracefully) so Profile =
Profile.WithConfederation(...) and SetDirty() only run for valid indexes.
- Around line 2005-2012: The current loop only sets a matching confederation but
never clears a previous selection when Profile?.Confederation is null or no
match is found; update the block that references _confederationButton,
_confederations and Profile?.Confederation to track whether a match was found
while iterating and after the loop clear the control if none matched (e.g. call
_confederationButton.SelectId(-1) or _confederationButton.ClearSelection()
depending on available API) so the UI doesn't retain a stale selection.

---

Nitpick comments:
In `@Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs`:
- Around line 757-761: Remove the dead commented-out handler block for
_confederationButton.OnItemSelected; it's unused and duplicates the actual
wiring via _recordsTab.OnGeneralRecordConfederationChanged += SetConfederation.
Delete the commented lines starting with //_confederationButton.OnItemSelected
and the inner commented braces to clean up the file and avoid confusion while
keeping SetConfederation and _recordsTab intact.
- Around line 858-872: The confederation list is currently sorted by prototype
localization key via _confederations.Sort((a, b) => string.Compare(a.Name,
b.Name, ...)), which orders by keys not user-visible text; change the sort to
compare the localized display names (use Loc.GetString(a.Name) vs
Loc.GetString(b.Name) with StringComparison.CurrentCultureIgnoreCase) before you
call _recordsTab.ConfederationButton.AddItem so the dropdown is ordered by the
actual localized names while leaving the subsequent ID check
(_confederations[i].ID == "NoConfederation") and AddItem/SelectId logic
unchanged.

In
`@Content.Client/StationRecords/GeneralStationRecordConsoleBoundUserInterface.cs`:
- Line 26: The lambda uses a misleading parameter name: change the
_window.OnPrint handler to use a descriptive parameter (e.g., content) instead
of id and pass that content into PrintStationRecord; locate the OnPrint
subscription in GeneralStationRecordConsoleBoundUserInterface (compare with the
OnDeleted => id => handler) and update the lambda parameter name and usage so it
reflects Action<string> content rather than an identifier.

In `@Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs`:
- Line 52: The code hardcodes the paper prototype id in
GeneralStationRecordConsoleSystem (the Spawn("Paper",
Transform(ent.Owner).Coordinates) call) which should be configurable; change
this to read a prototype id from the relevant component or a private constant
(e.g., add a string PaperPrototype field/property on the
StationRecordConsoleComponent or a private const string PaperProto) and use that
value in the Spawn call instead of the literal "Paper" so variants can be
provided via the component or easily changed in one place.
- Line 50: The code currently calls
_audioSystem.PlayPvs("/Audio/Machines/printer.ogg", ent.Owner) with a hardcoded
path; instead add a SoundSpecifier field/property on the corresponding component
(e.g., GeneralStationRecordConsoleComponent) to hold the sound (allowing
YAML/prototype overrides) and change the system to pass that SoundSpecifier to
the audio system (use the appropriate Play/PlayPvs overload that accepts a
SoundSpecifier and entity/transform). Update the component defaults in
prototypes/YAML to reference the desired sound and remove the string literal
from GeneralStationRecordConsoleSystem.

In `@Content.Server/StationRecords/Systems/StationRecordsSystem.cs`:
- Around line 137-159: Create a small value object (e.g., WlPersonalInfo or
WlRecords) to encapsulate the WL-Records fields (fullName, dateOfBirth,
confederation, country, height) and replace those five parameters in the
CreateGeneralRecord method signature with a single parameter of that type;
update the CreateGeneralRecord declaration and all call sites (including the
caller that passes these values) to construct and pass the new object, and
ensure any serialization/assignment inside CreateGeneralRecord uses the new
object's properties instead of individual parameters.

In `@Content.Shared/StationRecords/GeneralStationRecord.cs`:
- Around line 62-76: Add XML documentation for the newly added fields on
GeneralStationRecord: Fullname, DateOfBirth, Confederation, Country, and Height.
For each field (e.g., public string Fullname, public string DateOfBirth, public
string Confederation, public string Country, public int Height) add a concise
/// <summary>…</summary> comment describing the purpose/format (e.g., full legal
name, birth date format, confederation identifier, country of origin, height in
centimeters) to match the style of the existing XML docs on other fields.

Comment on lines +110 to +119
private string GenerateMedicalRecord(GeneralStationRecord stationRecord)
{
var medicalRecord = $"""
{Loc.GetString("records-full-name-edit") + stationRecord.Fullname ?? stationRecord.Name}
{Loc.GetString("records-date-of-birth-edit") + stationRecord.DateOfBirth ?? "N|A"}
{Loc.GetString("records-species") +
Loc.GetString(_prototypeManager.Index<SpeciesPrototype>(stationRecord.Species).Name)}
{Loc.GetString("records-height", ("height", stationRecord.Height))}
{stationRecord.MedicalRecord}
""" ?? Loc.GetString("medical-records-console-no-record");
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Same operator precedence bug as in GeneralRecord.xaml.cs?? fallbacks are unreachable.

Identical issue: + binds tighter than ??, so the fallback values after ?? are dead code. The trailing """ ?? Loc.GetString(...) on the raw string literal is also always a no-op. Additionally, _prototypeManager.Index<SpeciesPrototype>(stationRecord.Species) will throw on invalid/empty species IDs.

See the fix pattern proposed in GeneralRecord.xaml.cs — apply the same parenthesization and TryIndex guards here.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs` around
lines 110 - 119, The GenerateMedicalRecord method currently misuses the
null-coalescing operator due to operator precedence and risks throwing from
_prototypeManager.Index<SpeciesPrototype>(stationRecord.Species); fix by
parenthesizing each concatenation before applying ?? fallbacks and guard the
species lookup with TryIndex: ensure expressions like
(Loc.GetString("records-full-name-edit") + (stationRecord.Fullname ??
stationRecord.Name)) and (Loc.GetString("records-date-of-birth-edit") +
(stationRecord.DateOfBirth ?? "N|A")) are used, replace the direct Index call
with TryIndex for SpeciesPrototype to safely get the species name (with a
fallback via Loc.GetString), and remove the redundant trailing """ ?? ... by
applying the final overall fallback to the constructed string if needed (e.g.,
use (...) ?? Loc.GetString("medical-records-console-no-record")); keep
references to GenerateMedicalRecord, GeneralStationRecord,
stationRecord.Fullname, stationRecord.DateOfBirth, stationRecord.Height,
stationRecord.MedicalRecord, _prototypeManager.TryIndex and Loc.GetString when
making changes.

Comment on lines +271 to +281
SecurityRecord.Text = $"""
{Loc.GetString("records-full-name-edit") + stationRecord.Fullname ?? stationRecord.Name}
{Loc.GetString("records-date-of-birth-edit") + stationRecord.DateOfBirth ?? "N|A"}
{Loc.GetString("records-confederation-edit") +
Loc.GetString(_proto.Index<ConfederationRecordsPrototype>(stationRecord.Confederation).Name) ?? "N|A"}
{Loc.GetString("records-country-edit") + stationRecord.Country ?? "N|A"}
{Loc.GetString("records-species") +
Loc.GetString(_proto.Index<SpeciesPrototype>(stationRecord.Species).Name)}
{Loc.GetString("records-height", ("height", stationRecord.Height))}
{stationRecord.EmploymentRecord}
""" ?? Loc.GetString("criminal-records-console-no-security-record");
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: ?? fallbacks are all dead code due to operator precedence.

In C#, + binds tighter than ??. So for example:

Loc.GetString("records-full-name-edit") + stationRecord.Fullname ?? stationRecord.Name

is parsed as:

(Loc.GetString("records-full-name-edit") + stationRecord.Fullname) ?? stationRecord.Name

Since Loc.GetString(...) always returns a non-null string, and string + null yields a non-null string, the ?? fallback never fires — on any of the four lines.

Additionally, the raw string literal ($""" ... """) is never null, so ?? Loc.GetString("criminal-records-console-no-security-record") on line 281 is also dead code.

Finally, if stationRecord.Confederation is null or an unregistered prototype ID, _proto.Index<ConfederationRecordsPrototype>(...) will throw.

Proposed fix — parenthesize `??` and guard proto lookups
-        SecurityRecord.Text = $"""
-                {Loc.GetString("records-full-name-edit") + stationRecord.Fullname ?? stationRecord.Name}
-                {Loc.GetString("records-date-of-birth-edit") + stationRecord.DateOfBirth ?? "N|A"}
-                {Loc.GetString("records-confederation-edit") +
-                Loc.GetString(_proto.Index<ConfederationRecordsPrototype>(stationRecord.Confederation).Name) ?? "N|A"}
-                {Loc.GetString("records-country-edit") + stationRecord.Country ?? "N|A"}
-                {Loc.GetString("records-species") +
-                Loc.GetString(_proto.Index<SpeciesPrototype>(stationRecord.Species).Name)}
-                {Loc.GetString("records-height", ("height", stationRecord.Height))}
-                {stationRecord.EmploymentRecord}
-                """ ?? Loc.GetString("criminal-records-console-no-security-record");
+        var confederation = _proto.TryIndex<ConfederationRecordsPrototype>(stationRecord.Confederation, out var confProto)
+            ? Loc.GetString(confProto.Name)
+            : "N|A";
+
+        SecurityRecord.Text = $"""
+                {Loc.GetString("records-full-name-edit")}{stationRecord.Fullname ?? stationRecord.Name}
+                {Loc.GetString("records-date-of-birth-edit")}{stationRecord.DateOfBirth ?? "N|A"}
+                {Loc.GetString("records-confederation-edit")}{confederation}
+                {Loc.GetString("records-country-edit")}{stationRecord.Country ?? "N|A"}
+                {Loc.GetString("records-species")}{Loc.GetString(_proto.Index<SpeciesPrototype>(stationRecord.Species).Name)}
+                {Loc.GetString("records-height", ("height", stationRecord.Height))}
+                {stationRecord.EmploymentRecord}
+                """;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/CriminalRecords/CriminalRecordsConsoleWindow.xaml.cs` around
lines 271 - 281, The concatenations in the SecurityRecord.Text raw string use
"??" incorrectly due to operator precedence so the fallbacks never run; update
the expression so each nullable field is parenthesized (e.g.
Loc.GetString("records-full-name-edit") + (stationRecord.Fullname ??
stationRecord.Name)) and similarly for DateOfBirth, Country and the
Confederation/Species lookups; also protect the prototype lookups used in
_proto.Index<ConfederationRecordsPrototype>(stationRecord.Confederation) and
_proto.Index<SpeciesPrototype>(stationRecord.Species) by checking or using a
safe lookup (e.g. _proto.HasIndex or TryIndex and fall back to
Loc.GetString("N|A") or a default name) before calling Loc.GetString, and remove
the final ?? on the raw string (or instead conditionally set SecurityRecord.Text
to the "no-security-record" message if stationRecord is null/empty).

private OptionButton? _confederationButton; // WL-Records


private List<ConfederationRecordsPrototype> _confederations = new(); // WL-Recordss
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Typo in comment: // WL-Recordss — double 's'.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs` at line 207, The
inline comment trailing the private field _confederations contains a typo ("//
WL-Recordss"); update the comment on the field declaration for _confederations
(private List<ConfederationRecordsPrototype> _confederations = new();) to
correct the typo (e.g., "// WL-Records") or remove the comment if unnecessary so
the comment reads cleanly.

<Label Name="Fingerprint"/>
<Label Name="Dna"/>
<Button Visible="False" Name="DeleteButton" Text="{Loc 'general-station-record-console-delete'}"/>
<Button Name="PrintButton" Text="Распечатать"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Hardcoded Russian string — use a localization key instead.

"Распечатать" should be localized like the other UI strings in this file (e.g., via {Loc '...'}).

Proposed fix
-        <Button Name="PrintButton" Text="Распечатать"/>
+        <Button Name="PrintButton" Text="{Loc 'general-station-record-console-print'}"/>

Then add the corresponding entry in the localization .ftl files.

📝 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.

Suggested change
<Button Name="PrintButton" Text="Распечатать"/>
<Button Name="PrintButton" Text="{Loc 'general-station-record-console-print'}"/>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/StationRecords/GeneralRecord.xaml` at line 22, Replace the
hardcoded Russian label on the PrintButton with a localization key: update the
PrintButton Text attribute (Name="PrintButton") to use the same localization
binding style used elsewhere in this XAML (e.g., Text="{Loc
'stationrecords.print'}" or similar key naming), and then add the matching
key/value entry into the appropriate localization .ftl files (e.g.,
stationrecords.print = "Распечатать") so the UI uses the localized string.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

♻️ Duplicate comments (6)
Content.Client/StationRecords/GeneralRecord.xaml.cs (1)

27-29: ⚠️ Potential issue | 🟡 Minor

Index can still throw if record.Confederation is non-empty but not a valid prototype ID.

The string.IsNullOrEmpty check guards against empty values, but if record.Confederation contains an invalid prototype ID, prototypeManager.Index<ConfederationRecordsPrototype>() will throw. Consider using TryIndex for defensive lookup.

🛡️ Proposed fix using TryIndex
-        var confederation = !string.IsNullOrEmpty(record.Confederation)
-             ? prototypeManager.Index<ConfederationRecordsPrototype>(record.Confederation).Name
-             : Loc.GetString("generic-not-available-shorthand");
+        var confederation = !string.IsNullOrEmpty(record.Confederation)
+            && prototypeManager.TryIndex<ConfederationRecordsPrototype>(record.Confederation, out var confProto)
+             ? confProto.Name
+             : Loc.GetString("generic-not-available-shorthand");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/StationRecords/GeneralRecord.xaml.cs` around lines 27 - 29,
The current confederation lookup uses
prototypeManager.Index<ConfederationRecordsPrototype>(record.Confederation)
which will throw if record.Confederation is non-empty but not a valid prototype
ID; replace it with
prototypeManager.TryIndex<ConfederationRecordsPrototype>(record.Confederation,
out var confProto) and set confederation = confProto != null ? confProto.Name :
Loc.GetString("generic-not-available-shorthand") (while still short-circuiting
empty/null values), ensuring you reference the existing variable name
confederation and the prototype type ConfederationRecordsPrototype in
GeneralRecord.xaml.cs.
Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs (1)

69-76: ⚠️ Potential issue | 🟠 Major

Missing CanPrintEntries access control check.

Unlike OnRecordDelete (Line 58) which checks CanDeleteEntries before proceeding, OnRecordPrint does not verify CanPrintEntries. A client could send print requests even when printing is disabled or during an ongoing print operation.

🛡️ Proposed fix — add access control check
     private void OnRecordPrint(Entity<GeneralStationRecordConsoleComponent> ent, ref PrintStationRecord args)
     {
+        if (!ent.Comp.CanPrintEntries)
+            return;

         var owning = _station.GetOwningStation(ent.Owner);

         if (owning == null)
             return;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs`
around lines 69 - 76, In OnRecordPrint (GeneralStationRecordConsoleComponent)
add the same access-control check used in OnRecordDelete: after obtaining the
owning station, fetch the console component and verify component.CanPrintEntries
(and that it's not already printing/locked) before proceeding; if the check
fails, return early. This prevents clients from sending PrintStationRecord
requests when printing is disabled or already in progress.
Resources/Locale/ru-RU/_WL/records/records.ftl (2)

6-6: ⚠️ Potential issue | 🟡 Minor

Typo: "СВЕДЕНЬЯ" should be "СВЕДЕНИЯ".

The correct Russian word is "СВЕДЕНИЯ" (information/details).

-records-general = ОБЩИЕ СВЕДЕНЬЯ
+records-general = ОБЩИЕ СВЕДЕНИЯ
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resources/Locale/ru-RU/_WL/records/records.ftl` at line 6, Fix the typo in
the localization key records-general in the FTL file by replacing the incorrect
word "СВЕДЕНЬЯ" with the correct Russian "СВЕДЕНИЯ" so the value reads
records-general = ОБЩИЕ СВЕДЕНИЯ; update only the string value for the
records-general entry.

31-31: ⚠️ Potential issue | 🟡 Minor

Typo and colloquial usage: "Нету конфедирации" should be "Нет конфедерации".

Two issues: "конфедирации" has a misspelled vowel (should be "конфедерации"), and "Нету" is colloquial — "Нет" is preferred for UI text.

-records-confederation-no = Нету конфедирации
+records-confederation-no = Нет конфедерации
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resources/Locale/ru-RU/_WL/records/records.ftl` at line 31, The translation
string for the key records-confederation-no contains a typo and colloquial
wording; change its value from "Нету конфедирации" to the formal, correctly
spelled "Нет конфедерации" by editing the records-confederation-no entry to use
"Нет конфедерации".
Content.Client/CriminalRecords/CriminalRecordsConsoleWindow.xaml.cs (1)

283-296: ⚠️ Potential issue | 🔴 Critical

Guard prototype lookups and localize the fallback text.

Line 284 and Line 295 use _proto.Index<...> directly; invalid/missing prototype IDs can throw and break record rendering. Line 285 also hardcodes "N/A" instead of localized fallback.

Suggested fix
-        var confederation = !string.IsNullOrEmpty(stationRecord.Confederation)
-            ? Loc.GetString(_proto.Index<ConfederationRecordsPrototype>(stationRecord.Confederation).Name)
-            : "N/A";
+        var confederation = !string.IsNullOrEmpty(stationRecord.Confederation) &&
+                            _proto.TryIndex<ConfederationRecordsPrototype>(stationRecord.Confederation, out var confProto)
+            ? Loc.GetString(confProto.Name)
+            : Loc.GetString("generic-not-available-shorthand");
+
+        var species = _proto.TryIndex<SpeciesPrototype>(stationRecord.Species, out var speciesProto)
+            ? Loc.GetString(speciesProto.Name)
+            : Loc.GetString("generic-not-available-shorthand");

         SecurityRecord.Text = $"""
                 {Loc.GetString("records-full-name-edit")} {(!string.IsNullOrEmpty(stationRecord.Fullname)
                 ? stationRecord.Fullname : stationRecord.Name)}
                 {Loc.GetString("records-date-of-birth-edit")}  {(!string.IsNullOrEmpty(stationRecord.DateOfBirth)
                 ? stationRecord.DateOfBirth : Loc.GetString("generic-not-available-shorthand"))}
                 {Loc.GetString("records-confederation-edit")} {confederation}
                 {Loc.GetString("records-country-edit")} {(!string.IsNullOrEmpty(stationRecord.Country)
                 ? stationRecord.Country : Loc.GetString("generic-not-available-shorthand"))}
-                {Loc.GetString("records-species")} {Loc.GetString(_proto.Index<SpeciesPrototype>(stationRecord.Species).Name)}
+                {Loc.GetString("records-species")} {species}
                 {Loc.GetString("records-height", ("height", stationRecord.Height))}
                 {(!string.IsNullOrEmpty(stationRecord.SecurityRecord) ? stationRecord.SecurityRecord
                 : Loc.GetString("criminal-records-console-no-security-record"))}
                 """;
#!/bin/bash
# Verify direct prototype indexing in this file.
rg -n -C2 '_proto\.Index<.*Prototype>\(' Content.Client/CriminalRecords/CriminalRecordsConsoleWindow.xaml.cs
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/CriminalRecords/CriminalRecordsConsoleWindow.xaml.cs` around
lines 283 - 296, The code uses
_proto.Index<ConfederationRecordsPrototype>(stationRecord.Confederation) and
_proto.Index<SpeciesPrototype>(stationRecord.Species) directly and hardcodes
"N/A" for confederation; guard those prototype lookups and provide localized
fallbacks: in the block that builds SecurityRecord.Text, replace direct
_proto.Index calls with safe lookups (try to resolve prototype via
_proto.HasIndex/try-get pattern or catch the exception) and if the prototype is
missing use Loc.GetString("generic-not-available-shorthand") (or another
appropriate localization key) for confederation and species names; update the
confederation variable creation and the species interpolation to use the guarded
lookup results so rendering never throws.
Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs (1)

127-127: ⚠️ Potential issue | 🔴 Critical

Guard species prototype lookup to avoid rendering crashes.

Line 127 uses _prototypeManager.Index<SpeciesPrototype>(stationRecord.Species) directly; invalid IDs can throw during UI rendering.

Suggested fix
-                {Loc.GetString("records-species")} {Loc.GetString(_prototypeManager.Index<SpeciesPrototype>(stationRecord.Species).Name)}
+                {Loc.GetString("records-species")} {( _prototypeManager.TryIndex<SpeciesPrototype>(stationRecord.Species, out var speciesProto)
+                    ? Loc.GetString(speciesProto.Name)
+                    : Loc.GetString("generic-not-available-shorthand"))}
#!/bin/bash
# Verify direct species prototype indexing usage in this file.
rg -n -C2 '_prototypeManager\.Index<SpeciesPrototype>\(' Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs` at
line 127, The direct call to
_prototypeManager.Index<SpeciesPrototype>(stationRecord.Species) can throw for
invalid IDs; change the lookup in the UI render to a guarded lookup using the
prototype manager's safe API (e.g., TryIndex or a HasIndex check) for
SpeciesPrototype and fall back to a safe string (like
Loc.GetString("records-unknown-species") or stationRecord.Species) when the
prototype is missing; update the expression around Loc.GetString(...) that
currently uses _prototypeManager.Index<SpeciesPrototype>(stationRecord.Species)
so it first attempts a TryIndex<SpeciesPrototype>(stationRecord.Species, out var
proto) and uses proto.Name when present, otherwise the fallback.
🧹 Nitpick comments (3)
Content.Server/_WL/MedicalRecords/MedicalRecordsConsoleSystem.cs (1)

106-132: Consider simplifying the control flow.

The return on line 130 is redundant since it's at the end of the method's only execution path. The local variable could also be inlined for simpler logic.

♻️ Suggested simplification
 private void ProcessPrintingAnimation(EntityUid uid, float frameTime, MedicalRecordsConsoleComponent comp)
 {
-    if (comp.TimePrintRemaining > 0)
-    {
-        comp.TimePrintRemaining -= frameTime;
-
-        var PrintSoundEnd = comp.TimePrintRemaining <= 0;
+    if (comp.TimePrintRemaining <= 0)
+        return;

-        if (PrintSoundEnd)
-        {
-            var printed = Spawn(comp.PrintPaperId, Transform(uid).Coordinates);
+    comp.TimePrintRemaining -= frameTime;

-            if (TryComp<PaperComponent>(printed, out var paper))
-                _paperSystem.SetContent((printed, paper), comp.ContextPrint);
+    if (comp.TimePrintRemaining > 0)
+        return;

-            comp.ContextPrint = string.Empty;
+    var printed = Spawn(comp.PrintPaperId, Transform(uid).Coordinates);

-            comp.CanPrintEntries = true;
+    if (TryComp<PaperComponent>(printed, out var paper))
+        _paperSystem.SetContent((printed, paper), comp.ContextPrint);

-            var ent = new Entity<MedicalRecordsConsoleComponent>(uid, comp);
+    comp.ContextPrint = string.Empty;
+    comp.CanPrintEntries = true;

-            UpdateUserInterface(ent);
-        }
-
-        return;
-    }
+    UpdateUserInterface(new Entity<MedicalRecordsConsoleComponent>(uid, comp));
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Server/_WL/MedicalRecords/MedicalRecordsConsoleSystem.cs` around
lines 106 - 132, In ProcessPrintingAnimation, remove the redundant trailing
"return" and inline the PrintSoundEnd local: decrement comp.TimePrintRemaining,
then directly check "if (comp.TimePrintRemaining <= 0)" (instead of using
PrintSoundEnd) and run the spawn/SetContent/clear/UpdateUserInterface logic
inside that branch; this simplifies the control flow while keeping the same
behavior for comp.TimePrintRemaining, PrintPaperId, ContextPrint,
CanPrintEntries and the call to UpdateUserInterface.
Content.Client/StationRecords/GeneralStationRecordConsoleWindow.xaml (1)

23-25: PrintButton is always visible regardless of CanPrintRecords state.

The button has no visibility binding. If the console doesn't support printing (CanPrintRecords = false), the button would still appear but presumably do nothing. Consider binding visibility to the print capability state, similar to how delete functionality might be handled.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/StationRecords/GeneralStationRecordConsoleWindow.xaml` around
lines 23 - 25, The PrintButton is missing a visibility binding to the console
capability flag; update the PrintButton element in
GeneralStationRecordConsoleWindow.xaml to bind its Visibility (or IsVisible) to
the viewmodel property CanPrintRecords so the button is hidden when printing is
not supported, using the existing boolean-to-visibility converter or the
project’s standard visibility binding pattern (e.g., bind PrintButton.Visibility
to CanPrintRecords with a BooleanToVisibilityConverter or invert if needed);
ensure the binding target is the DataContext that exposes CanPrintRecords so the
button's visibility reflects the console capability at runtime.
Content.Server/CriminalRecords/Systems/CriminalRecordsConsoleSystem.cs (1)

96-159: Consider extracting shared printing logic.

The OnPrinted and ProcessPrintingAnimation methods duplicate patterns found in GeneralStationRecordConsoleSystem and MedicalRecordsConsoleSystem. Consider extracting this to a shared helper or base class to reduce maintenance burden across the three console systems.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Server/CriminalRecords/Systems/CriminalRecordsConsoleSystem.cs`
around lines 96 - 159, Both OnPrinted and ProcessPrintingAnimation duplicate
printing behavior found in GeneralStationRecordConsoleSystem and
MedicalRecordsConsoleSystem; extract the shared printing flow into a reusable
helper or base class. Create a new internal helper (e.g.,
CriminalRecordPrintHelper or an abstract base CriminalRecordsConsoleBase) that
encapsulates: preparing ContextPrint from a record (used by OnPrinted), spawning
paper and setting content, playing audio, and toggling
CanPrintEntries/TimePrintRemaining (used by ProcessPrintingAnimation); then
update OnPrinted to call the helper to build context and trigger the print
sequence and update ProcessPrintingAnimation to call the helper’s
finalize/timeout method to spawn paper and update UI (referencing OnPrinted,
ProcessPrintingAnimation, UpdateUserInterface, _paperSystem.SetContent,
_audioSystem.PlayPvs to locate code to extract).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs`:
- Around line 129-130: The UI is rendering the wrong field
(stationRecord.SecurityRecord) in MedicalRecordsConsoleWindow.xaml.cs; replace
that usage with stationRecord.MedicalRecord and preserve the null/empty fallback
to Loc.GetString("medical-records-console-no-record") so the medical console
shows the correct domain data. Locate the expression that currently references
stationRecord.SecurityRecord and swap it to stationRecord.MedicalRecord, keeping
the existing conditional logic intact.

In `@Content.Server/_WL/MedicalRecords/MedicalRecordsConsoleSystem.cs`:
- Line 91: The call to
_prototypeManager.Index<SpeciesPrototype>(record.Species).Name can throw if the
species prototype is missing; replace it with
_prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var
speciesProto) and use speciesProto?.Name or a safe fallback (e.g.,
Loc.GetString("records-species-unknown") or record.Species) when speciesProto is
null, then pass that result into Loc.GetString so the UI won’t crash on missing
prototypes in MedicalRecordsConsoleSystem/wherever the record rendering occurs.
- Line 112: The local variable PrintSoundEnd violates C# naming conventions;
rename it to printSoundEnd where it is declared and used (the assignment "var
PrintSoundEnd = comp.TimePrintRemaining <= 0;" and any subsequent references) so
the local follows camelCase; ensure all references to PrintSoundEnd in
MedicalRecordsConsoleSystem (related to comp.TimePrintRemaining check) are
updated to printSoundEnd.

In `@Content.Server/CriminalRecords/Systems/CriminalRecordsConsoleSystem.cs`:
- Around line 133-159: The local variable PrintSoundEnd in
ProcessPrintingAnimation should be renamed to follow C# camelCase conventions
(printSoundEnd); update the declaration and any references inside the method (in
CriminalRecordsConsoleComponent's ProcessPrintingAnimation where PrintSoundEnd
is used to gate spawning the paper, setting ContextPrint, CanPrintEntries and
calling UpdateUserInterface) to use printSoundEnd so naming is consistent with
style guidelines.

In `@Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs`:
- Around line 92-94: The conditional currently checks record.EmploymentRecord
but then emits record.SecurityRecord and uses the wrong fallback localization;
update the output to emit record.EmploymentRecord when
!string.IsNullOrEmpty(record.EmploymentRecord) and replace the fallback
localization key "criminal-records-console-no-security-record" with the correct
employment fallback (e.g. "criminal-records-console-no-employment-record")
inside the method in GeneralStationRecordConsoleSystem where the record string
is composed so the employment block shows the employment data and a matching
fallback message.
- Around line 79-81: The code in GeneralStationRecordConsoleSystem that sets the
confederation string uses
_prototypeManager.Index<ConfederationRecordsPrototype>(record.Confederation)
which will throw for invalid IDs; change this to a defensive lookup using
_prototypeManager.TryIndex<ConfederationRecordsPrototype>(record.Confederation,
out var proto) and, when TryIndex returns false (or record.Confederation is
empty), fall back to Loc.GetString("generic-not-available-shorthand"); keep the
existing empty-string check for record.Confederation and only call TryIndex when
non-empty, then use proto.Name when TryIndex succeeds.

In `@Content.Shared/_WL/MedicalRecords/MedicalRecordsUi.cs`:
- Around line 19-25: Rename the incorrectly named field and constructor
parameter to match the rest of the codebase: change the readonly field
CanPrintedRecords to CanPrintRecords in the MedicalRecordsConsoleState class and
update the constructor parameter from canPrintedRecords to canPrintRecords (and
assignment inside the constructor), then update all usages/references to
MedicalRecordsConsoleState.CanPrintedRecords to use CanPrintRecords so it
matches GeneralStationRecordConsoleState.

In
`@Content.Shared/CriminalRecords/Components/CriminalRecordsConsoleComponent.cs`:
- Around line 57-58: CriminalRecordsConsoleComponent's CanPrintEntries field is
missing an explicit initializer and defaults to false while
GeneralStationRecordConsoleComponent and MedicalRecordsConsoleComponent default
it to true; update the CriminalRecordsConsoleComponent by initializing the
public bool CanPrintEntries to true so its default state matches the other
record console components (refer to the CanPrintEntries field on
CriminalRecordsConsoleComponent and the similar defaults in
GeneralStationRecordConsoleComponent and MedicalRecordsConsoleComponent).

In `@Content.Shared/StationRecords/GeneralRecordsUi.cs`:
- Around line 47-48: Rename the typo'd parameter canPrintRerods to
canPrintRecords wherever it's declared and referenced (e.g., in the
GeneralRecordsUi constructor/method signature and any call sites or
assignments), update the parameter name in the method signature that currently
lists "bool canPrintRerods" and adjust all usages inside GeneralRecordsUi and
callers to use "canPrintRecords" to keep identifiers consistent and avoid
compile errors.

---

Duplicate comments:
In `@Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs`:
- Line 127: The direct call to
_prototypeManager.Index<SpeciesPrototype>(stationRecord.Species) can throw for
invalid IDs; change the lookup in the UI render to a guarded lookup using the
prototype manager's safe API (e.g., TryIndex or a HasIndex check) for
SpeciesPrototype and fall back to a safe string (like
Loc.GetString("records-unknown-species") or stationRecord.Species) when the
prototype is missing; update the expression around Loc.GetString(...) that
currently uses _prototypeManager.Index<SpeciesPrototype>(stationRecord.Species)
so it first attempts a TryIndex<SpeciesPrototype>(stationRecord.Species, out var
proto) and uses proto.Name when present, otherwise the fallback.

In `@Content.Client/CriminalRecords/CriminalRecordsConsoleWindow.xaml.cs`:
- Around line 283-296: The code uses
_proto.Index<ConfederationRecordsPrototype>(stationRecord.Confederation) and
_proto.Index<SpeciesPrototype>(stationRecord.Species) directly and hardcodes
"N/A" for confederation; guard those prototype lookups and provide localized
fallbacks: in the block that builds SecurityRecord.Text, replace direct
_proto.Index calls with safe lookups (try to resolve prototype via
_proto.HasIndex/try-get pattern or catch the exception) and if the prototype is
missing use Loc.GetString("generic-not-available-shorthand") (or another
appropriate localization key) for confederation and species names; update the
confederation variable creation and the species interpolation to use the guarded
lookup results so rendering never throws.

In `@Content.Client/StationRecords/GeneralRecord.xaml.cs`:
- Around line 27-29: The current confederation lookup uses
prototypeManager.Index<ConfederationRecordsPrototype>(record.Confederation)
which will throw if record.Confederation is non-empty but not a valid prototype
ID; replace it with
prototypeManager.TryIndex<ConfederationRecordsPrototype>(record.Confederation,
out var confProto) and set confederation = confProto != null ? confProto.Name :
Loc.GetString("generic-not-available-shorthand") (while still short-circuiting
empty/null values), ensuring you reference the existing variable name
confederation and the prototype type ConfederationRecordsPrototype in
GeneralRecord.xaml.cs.

In `@Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs`:
- Around line 69-76: In OnRecordPrint (GeneralStationRecordConsoleComponent) add
the same access-control check used in OnRecordDelete: after obtaining the owning
station, fetch the console component and verify component.CanPrintEntries (and
that it's not already printing/locked) before proceeding; if the check fails,
return early. This prevents clients from sending PrintStationRecord requests
when printing is disabled or already in progress.

In `@Resources/Locale/ru-RU/_WL/records/records.ftl`:
- Line 6: Fix the typo in the localization key records-general in the FTL file
by replacing the incorrect word "СВЕДЕНЬЯ" with the correct Russian "СВЕДЕНИЯ"
so the value reads records-general = ОБЩИЕ СВЕДЕНИЯ; update only the string
value for the records-general entry.
- Line 31: The translation string for the key records-confederation-no contains
a typo and colloquial wording; change its value from "Нету конфедирации" to the
formal, correctly spelled "Нет конфедерации" by editing the
records-confederation-no entry to use "Нет конфедерации".

---

Nitpick comments:
In `@Content.Client/StationRecords/GeneralStationRecordConsoleWindow.xaml`:
- Around line 23-25: The PrintButton is missing a visibility binding to the
console capability flag; update the PrintButton element in
GeneralStationRecordConsoleWindow.xaml to bind its Visibility (or IsVisible) to
the viewmodel property CanPrintRecords so the button is hidden when printing is
not supported, using the existing boolean-to-visibility converter or the
project’s standard visibility binding pattern (e.g., bind PrintButton.Visibility
to CanPrintRecords with a BooleanToVisibilityConverter or invert if needed);
ensure the binding target is the DataContext that exposes CanPrintRecords so the
button's visibility reflects the console capability at runtime.

In `@Content.Server/_WL/MedicalRecords/MedicalRecordsConsoleSystem.cs`:
- Around line 106-132: In ProcessPrintingAnimation, remove the redundant
trailing "return" and inline the PrintSoundEnd local: decrement
comp.TimePrintRemaining, then directly check "if (comp.TimePrintRemaining <= 0)"
(instead of using PrintSoundEnd) and run the
spawn/SetContent/clear/UpdateUserInterface logic inside that branch; this
simplifies the control flow while keeping the same behavior for
comp.TimePrintRemaining, PrintPaperId, ContextPrint, CanPrintEntries and the
call to UpdateUserInterface.

In `@Content.Server/CriminalRecords/Systems/CriminalRecordsConsoleSystem.cs`:
- Around line 96-159: Both OnPrinted and ProcessPrintingAnimation duplicate
printing behavior found in GeneralStationRecordConsoleSystem and
MedicalRecordsConsoleSystem; extract the shared printing flow into a reusable
helper or base class. Create a new internal helper (e.g.,
CriminalRecordPrintHelper or an abstract base CriminalRecordsConsoleBase) that
encapsulates: preparing ContextPrint from a record (used by OnPrinted), spawning
paper and setting content, playing audio, and toggling
CanPrintEntries/TimePrintRemaining (used by ProcessPrintingAnimation); then
update OnPrinted to call the helper to build context and trigger the print
sequence and update ProcessPrintingAnimation to call the helper’s
finalize/timeout method to spawn paper and update UI (referencing OnPrinted,
ProcessPrintingAnimation, UpdateUserInterface, _paperSystem.SetContent,
_audioSystem.PlayPvs to locate code to extract).

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 867d9b4 and 4297bd9.

📒 Files selected for processing (22)
  • Content.Client/CriminalRecords/CriminalRecordsConsoleBoundUserInterface.cs
  • Content.Client/CriminalRecords/CriminalRecordsConsoleWindow.xaml
  • Content.Client/CriminalRecords/CriminalRecordsConsoleWindow.xaml.cs
  • Content.Client/StationRecords/GeneralRecord.xaml
  • Content.Client/StationRecords/GeneralRecord.xaml.cs
  • Content.Client/StationRecords/GeneralStationRecordConsoleWindow.xaml
  • Content.Client/StationRecords/GeneralStationRecordConsoleWindow.xaml.cs
  • Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleBoundUserInterface.cs
  • Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml
  • Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs
  • Content.Server/CriminalRecords/Systems/CriminalRecordsConsoleSystem.cs
  • Content.Server/StationRecords/Components/GeneralStationRecordConsoleComponent.cs
  • Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs
  • Content.Server/_WL/MedicalRecords/MedicalRecordsConsoleSystem.cs
  • Content.Shared/CriminalRecords/Components/CriminalRecordsConsoleComponent.cs
  • Content.Shared/CriminalRecords/CriminalRecordsUi.cs
  • Content.Shared/StationRecords/GeneralRecordsUi.cs
  • Content.Shared/_WL/MedicalRecords/MedicalRecordsConsoleComponent.cs
  • Content.Shared/_WL/MedicalRecords/MedicalRecordsUi.cs
  • Content.Shared/_WL/Records/PrintStationRecord.cs
  • Resources/Locale/ru-RU/_WL/records/records.ftl
  • Resources/Locale/ru-RU/_WL/station-records/general-station-records.ftl
✅ Files skipped from review due to trivial changes (1)
  • Resources/Locale/ru-RU/_WL/station-records/general-station-records.ftl
🚧 Files skipped from review as they are similar to previous changes (2)
  • Content.Client/StationRecords/GeneralRecord.xaml
  • Content.Client/CriminalRecords/CriminalRecordsConsoleWindow.xaml

? record.Fullname : record.Name)}
{Loc.GetString("records-date-of-birth-edit")} {(!string.IsNullOrEmpty(record.DateOfBirth)
? record.DateOfBirth : Loc.GetString("generic-not-available-shorthand"))}
{Loc.GetString("records-species")} {Loc.GetString(_prototypeManager.Index<SpeciesPrototype>(record.Species).Name)}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use TryIndex to avoid potential exception on missing prototype.

_prototypeManager.Index<SpeciesPrototype>() throws KeyNotFoundException if the species prototype doesn't exist. If record data is corrupted or references a removed species, this will crash.

🛡️ Proposed fix using TryIndex
-{Loc.GetString("records-species")} {Loc.GetString(_prototypeManager.Index<SpeciesPrototype>(record.Species).Name)}
+{Loc.GetString("records-species")} {(_prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var speciesProto) ? Loc.GetString(speciesProto.Name) : Loc.GetString("generic-not-available-shorthand"))}
📝 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.

Suggested change
{Loc.GetString("records-species")} {Loc.GetString(_prototypeManager.Index<SpeciesPrototype>(record.Species).Name)}
{Loc.GetString("records-species")} {(_prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var speciesProto) ? Loc.GetString(speciesProto.Name) : Loc.GetString("generic-not-available-shorthand"))}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Server/_WL/MedicalRecords/MedicalRecordsConsoleSystem.cs` at line 91,
The call to _prototypeManager.Index<SpeciesPrototype>(record.Species).Name can
throw if the species prototype is missing; replace it with
_prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var
speciesProto) and use speciesProto?.Name or a safe fallback (e.g.,
Loc.GetString("records-species-unknown") or record.Species) when speciesProto is
null, then pass that result into Loc.GetString so the UI won’t crash on missing
prototypes in MedicalRecordsConsoleSystem/wherever the record rendering occurs.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

♻️ Duplicate comments (3)
Content.Client/StationRecords/GeneralRecord.xaml.cs (1)

55-55: ⚠️ Potential issue | 🟡 Minor

prototypeManager.Index<SpeciesPrototype> can throw if record.Species is invalid.

This is inconsistent with the defensive TryIndex used for confederation. Use TryIndex for safety.

🛡️ Proposed fix
-                {Loc.GetString("records-species")} {Loc.GetString(prototypeManager.Index<SpeciesPrototype>(record.Species).Name)}
+                {Loc.GetString("records-species")} {(prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var specProto)
+                ? Loc.GetString(specProto.Name) : Loc.GetString("generic-not-available-shorthand"))}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/StationRecords/GeneralRecord.xaml.cs` at line 55, The call to
prototypeManager.Index<SpeciesPrototype>(record.Species) can throw for invalid
IDs; replace it with prototypeManager.TryIndex<SpeciesPrototype>(record.Species,
out var speciesProto) and use speciesProto?.Name (or a fallback string such as
record.Species or a localized "unknown" token) when building the display string,
mirroring the defensive pattern used for confederation handling; update the line
that currently constructs "{Loc.GetString("records-species")}
{Loc.GetString(prototypeManager.Index<SpeciesPrototype>(record.Species).Name)}"
to use the TryIndex result and a safe fallback.
Content.Server/_WL/MedicalRecords/MedicalRecordsConsoleSystem.cs (1)

103-103: ⚠️ Potential issue | 🟠 Major

Use TryIndex to avoid potential exception on missing species prototype.

_prototypeManager.Index<SpeciesPrototype>() throws KeyNotFoundException if the species prototype doesn't exist. If record data is corrupted or references a removed species, this will crash.

🛡️ Proposed fix using TryIndex
-{Loc.GetString("records-species")} {Loc.GetString(_prototypeManager.Index<SpeciesPrototype>(record.Species).Name)}
+{Loc.GetString("records-species")} {(_prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var speciesProto) ? Loc.GetString(speciesProto.Name) : Loc.GetString("generic-not-available-shorthand"))}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Server/_WL/MedicalRecords/MedicalRecordsConsoleSystem.cs` at line
103, The current usage of
_prototypeManager.Index<SpeciesPrototype>(record.Species).Name can throw if the
species prototype is missing; change this to use
_prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var
speciesProto) and if that returns false use a safe fallback (e.g., a localized
"UnknownSpecies" key or record.Species) when building the string passed to
Loc.GetString in MedicalRecordsConsoleSystem (the line that constructs
"{Loc.GetString("records-species")} {Loc.GetString(...)}"); ensure you reference
the TryIndex result (speciesProto?.Name) or fallback value rather than directly
calling Index.
Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs (1)

140-140: ⚠️ Potential issue | 🟠 Major

Use TryIndex for SpeciesPrototype lookup to avoid exceptions.

_prototypeManager.Index<SpeciesPrototype>() will throw if the species ID is invalid or removed. Use TryIndex with a fallback.

🛡️ Proposed fix using TryIndex
-                {Loc.GetString("records-species")} {Loc.GetString(_prototypeManager.Index<SpeciesPrototype>(stationRecord.Species).Name)}
+                {Loc.GetString("records-species")} {(_prototypeManager.TryIndex<SpeciesPrototype>(stationRecord.Species, out var speciesProto) ? Loc.GetString(speciesProto.Name) : Loc.GetString("generic-not-available-shorthand"))}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs` at
line 140, The current call to
_prototypeManager.Index<SpeciesPrototype>(stationRecord.Species) can throw if
the species ID is invalid; replace it with
_prototypeManager.TryIndex<SpeciesPrototype>(stationRecord.Species, out var
speciesProto) and use speciesProto?.Name (or a fallback like the raw
stationRecord.Species or a localized "unknown" string via Loc.GetString) when
building the string passed to Loc.GetString so that missing prototypes do not
cause exceptions in the UI; update the expression around
Loc.GetString("records-species") to use the TryIndex result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs`:
- Line 19: The class’s injected fields (e.g., the [Dependency] IPrototypeManager
_prototypeManager used by GenerateMedicalRecord and the other dependencies
declared nearby) are never populated because IoC injection isn’t run; add a call
to IoCManager.InjectDependencies(this) in the MedicalRecordsConsoleWindow
constructor (or initialization path that always runs before
GenerateMedicalRecord) so the [Dependency]-annotated fields are populated before
use.
- Around line 125-133: stationRecord.Languages is iterated using
_prototypeManager.Index<LanguagePrototype> which throws if a prototype is
missing; replace the Index call with
_prototypeManager.TryIndex<LanguagePrototype>(id, out var proto) and handle the
missing case (e.g., skip the entry or append a fallback label) before calling
Loc.GetString(proto.Name) so the client no longer crashes when a language ID is
invalid; update the loop around Languages and the Loc.GetString usage to use the
TryIndex result.

In `@Content.Client/CriminalRecords/CriminalRecordsConsoleWindow.xaml.cs`:
- Line 299: The species prototype lookup uses
_proto.Index<SpeciesPrototype>(stationRecord.Species) which can throw for
invalid IDs; change it to use
_proto.TryIndex<SpeciesPrototype>(stationRecord.Species, out var speciesProto)
and use speciesProto?.Name (or fall back to stationRecord.Species or a localized
"Unknown" string via Loc.GetString) when composing the Loc.GetString call so the
UI no longer throws on bad prototype IDs; update the expression around
Loc.GetString("records-species") to use the TryIndex result accordingly.

In `@Content.Client/StationRecords/GeneralRecord.xaml.cs`:
- Around line 37-45: The loop building the languages string uses
prototypeManager.Index<LanguagePrototype>(...) which will throw for invalid IDs;
change it to use prototypeManager.TryIndex<LanguagePrototype>(id, out var proto)
inside the loop over record.Languages and skip any entries where TryIndex
returns false, then call Loc.GetString(proto.Name) only for valid prototypes and
append the comma/period separators as before; update references in
GeneralRecord.xaml.cs (the for loop that reads record.Languages and the use of
Loc.GetString) to use TryIndex and continue on invalid IDs.

In `@Content.Server/_WL/MedicalRecords/MedicalRecordsConsoleSystem.cs`:
- Around line 106-107: In the MedicalRecordsConsoleSystem rendering where the
printed record uses record.SecurityRecord, replace that field with the correct
medical notes field (record.MedicalRecord) so the console shows medical data
instead of security notes; locate the output code in MedicalRecordsConsoleSystem
(the template/string construction that references record.SecurityRecord) and
swap the property to record.MedicalRecord, ensuring null/empty checks remain
(e.g., keep the existing !string.IsNullOrEmpty(...) conditional) and preserve
the fallback to Loc.GetString("medical-records-console-no-record").
- Around line 88-96: The loop building the languages string uses
_prototypeManager.Index<LanguagePrototype>(record.Languages[i]) which throws if
a prototype ID is missing; replace this with
_prototypeManager.TryIndex<LanguagePrototype>(id, out var proto) inside the
loop, and when TryIndex returns false use a safe fallback (e.g., the raw id or a
localized "Unknown" token) before calling Loc.GetString; ensure you still append
commas/periods exactly as the existing logic does and update references to
LanguagePrototype and record.Languages accordingly.

In `@Content.Server/CriminalRecords/Systems/CriminalRecordsConsoleSystem.cs`:
- Line 122: The species lookup currently calls
_prototypeManager.Index<SpeciesPrototype>(record.Species) which can throw for an
invalid ID; change it to use
_prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var
speciesProto) and handle the failure (e.g., fall back to a safe string like
Loc.GetString("records-unknown") or record.Species) before calling
Loc.GetString(speciesProto.Name) so the console display no longer throws; update
the code around the existing Loc.GetString call in CriminalRecordsConsoleSystem
to use speciesProto when TryIndex succeeds and a fallback when it fails.

In `@Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs`:
- Line 107: The code calls
_prototypeManager.Index<SpeciesPrototype>(record.Species) which can throw for
invalid IDs; change this to use
_prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var
speciesProto) and handle the false case safely (e.g., use a fallback string or
empty name) before calling Loc.GetString so GeneralStationRecordConsoleSystem no
longer risk-throws when record.Species is invalid.
- Around line 89-97: The loop in GeneralStationRecordConsoleSystem that builds
the languages string uses _prototypeManager.Index<LanguagePrototype> which
throws for invalid IDs; change it to use
_prototypeManager.TryIndex<LanguagePrototype> (or TryIndex) when iterating
record.Languages, only append the prototype.Name when TryIndex returns true, and
otherwise skip the invalid ID (optionally log a warning via the system logger);
ensure commas and the final period are only added between/after successfully
appended language names so empty or missing prototypes don't create trailing
punctuation.

In `@Content.Server/StationRecords/Systems/StationRecordsSystem.cs`:
- Around line 103-108: CreateGeneralRecord currently returns early when
TryComp<LanguagesComponent>(player, out var languages) yields null, which
silently blocks record creation; instead, if languages is null, set a safe empty
fallback (e.g., an empty list/collection of known languages or a newly
constructed LanguagesComponent with no languages) and continue so record
creation proceeds. Locate the TryComp<LanguagesComponent>(player, out var
languages) usage in CreateGeneralRecord and replace the early return with a
fallback assignment to an empty languages collection, ensuring subsequent code
that reads languages uses that fallback.

---

Duplicate comments:
In `@Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs`:
- Line 140: The current call to
_prototypeManager.Index<SpeciesPrototype>(stationRecord.Species) can throw if
the species ID is invalid; replace it with
_prototypeManager.TryIndex<SpeciesPrototype>(stationRecord.Species, out var
speciesProto) and use speciesProto?.Name (or a fallback like the raw
stationRecord.Species or a localized "unknown" string via Loc.GetString) when
building the string passed to Loc.GetString so that missing prototypes do not
cause exceptions in the UI; update the expression around
Loc.GetString("records-species") to use the TryIndex result.

In `@Content.Client/StationRecords/GeneralRecord.xaml.cs`:
- Line 55: The call to prototypeManager.Index<SpeciesPrototype>(record.Species)
can throw for invalid IDs; replace it with
prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var
speciesProto) and use speciesProto?.Name (or a fallback string such as
record.Species or a localized "unknown" token) when building the display string,
mirroring the defensive pattern used for confederation handling; update the line
that currently constructs "{Loc.GetString("records-species")}
{Loc.GetString(prototypeManager.Index<SpeciesPrototype>(record.Species).Name)}"
to use the TryIndex result and a safe fallback.

In `@Content.Server/_WL/MedicalRecords/MedicalRecordsConsoleSystem.cs`:
- Line 103: The current usage of
_prototypeManager.Index<SpeciesPrototype>(record.Species).Name can throw if the
species prototype is missing; change this to use
_prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var
speciesProto) and if that returns false use a safe fallback (e.g., a localized
"UnknownSpecies" key or record.Species) when building the string passed to
Loc.GetString in MedicalRecordsConsoleSystem (the line that constructs
"{Loc.GetString("records-species")} {Loc.GetString(...)}"); ensure you reference
the TryIndex result (speciesProto?.Name) or fallback value rather than directly
calling Index.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4297bd9 and ca1db0b.

📒 Files selected for processing (14)
  • Content.Client/CriminalRecords/CriminalRecordsConsoleWindow.xaml
  • Content.Client/CriminalRecords/CriminalRecordsConsoleWindow.xaml.cs
  • Content.Client/StationRecords/GeneralRecord.xaml.cs
  • Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml
  • Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs
  • Content.Server/CriminalRecords/Systems/CriminalRecordsConsoleSystem.cs
  • Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs
  • Content.Server/StationRecords/Systems/StationRecordsSystem.cs
  • Content.Server/_WL/MedicalRecords/MedicalRecordsConsoleSystem.cs
  • Content.Shared/CriminalRecords/Components/CriminalRecordsConsoleComponent.cs
  • Content.Shared/StationRecords/GeneralRecordsUi.cs
  • Content.Shared/StationRecords/GeneralStationRecord.cs
  • Content.Shared/_WL/MedicalRecords/MedicalRecordsUi.cs
  • Resources/Locale/ru-RU/_WL/records/records.ftl
🚧 Files skipped from review as they are similar to previous changes (4)
  • Content.Shared/_WL/MedicalRecords/MedicalRecordsUi.cs
  • Content.Shared/StationRecords/GeneralRecordsUi.cs
  • Content.Client/CriminalRecords/CriminalRecordsConsoleWindow.xaml
  • Content.Shared/CriminalRecords/Components/CriminalRecordsConsoleComponent.cs

[GenerateTypedNameReferences]
public sealed partial class MedicalRecordsConsoleWindow : FancyWindow
{
[Dependency] private readonly IPrototypeManager _prototypeManager = default!;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Missing IoCManager.InjectDependencies(this) — dependency will be null.

The [Dependency] attribute on _prototypeManager requires IoCManager.InjectDependencies(this) to be called in the constructor. Without it, _prototypeManager remains null and will throw NullReferenceException when GenerateMedicalRecord is called.

🐛 Proposed fix
 public MedicalRecordsConsoleWindow()
 {
+    IoCManager.InjectDependencies(this);
     RobustXamlLoader.Load(this);

Also applies to: 26-29

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs` at
line 19, The class’s injected fields (e.g., the [Dependency] IPrototypeManager
_prototypeManager used by GenerateMedicalRecord and the other dependencies
declared nearby) are never populated because IoC injection isn’t run; add a call
to IoCManager.InjectDependencies(this) in the MedicalRecordsConsoleWindow
constructor (or initialization path that always runs before
GenerateMedicalRecord) so the [Dependency]-annotated fields are populated before
use.

Comment on lines +125 to +133
for (int i = 0; i < stationRecord.Languages.Count; i++)
{
languages += Loc.GetString(_prototypeManager.Index<LanguagePrototype>(stationRecord.Languages[i]).Name);

if (i != stationRecord.Languages.Count - 1)
languages += ", ";
else
languages += ".";
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use TryIndex for LanguagePrototype lookup to avoid exceptions.

Same issue as server-side: _prototypeManager.Index<LanguagePrototype>() throws if the prototype doesn't exist. Invalid or removed language IDs in station records will crash the client.

🛡️ Proposed fix using TryIndex
         for (int i = 0; i < stationRecord.Languages.Count; i++)
         {
-            languages += Loc.GetString(_prototypeManager.Index<LanguagePrototype>(stationRecord.Languages[i]).Name);
+            if (_prototypeManager.TryIndex<LanguagePrototype>(stationRecord.Languages[i], out var langProto))
+                languages += Loc.GetString(langProto.Name);
+            else
+                languages += Loc.GetString("generic-not-available-shorthand");

             if (i != stationRecord.Languages.Count - 1)
                 languages += ", ";
             else
                 languages += ".";
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/_WL/MedicalRecords/MedicalRecordsConsoleWindow.xaml.cs` around
lines 125 - 133, stationRecord.Languages is iterated using
_prototypeManager.Index<LanguagePrototype> which throws if a prototype is
missing; replace the Index call with
_prototypeManager.TryIndex<LanguagePrototype>(id, out var proto) and handle the
missing case (e.g., skip the entry or append a fallback label) before calling
Loc.GetString(proto.Name) so the client no longer crashes when a language ID is
invalid; update the loop around Languages and the Loc.GetString usage to use the
TryIndex result.

{Loc.GetString("records-confederation-edit")} {confederation}
{Loc.GetString("records-country-edit")} {(!string.IsNullOrEmpty(stationRecord.Country)
? stationRecord.Country : Loc.GetString("generic-not-available-shorthand"))}
{Loc.GetString("records-species")} {Loc.GetString(_proto.Index<SpeciesPrototype>(stationRecord.Species).Name)}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

_proto.Index<SpeciesPrototype> can throw if record.Species is invalid.

The confederation lookup defensively uses TryIndex, but the species lookup uses Index directly, which will throw if record.Species contains an invalid prototype ID. This is inconsistent with the defensive pattern used a few lines above.

🛡️ Proposed fix — use TryIndex for species lookup
-                {Loc.GetString("records-species")} {Loc.GetString(_proto.Index<SpeciesPrototype>(stationRecord.Species).Name)}
+                {Loc.GetString("records-species")} {(_proto.TryIndex<SpeciesPrototype>(stationRecord.Species, out var specProto)
+                ? Loc.GetString(specProto.Name) : Loc.GetString("generic-not-available-shorthand"))}
📝 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.

Suggested change
{Loc.GetString("records-species")} {Loc.GetString(_proto.Index<SpeciesPrototype>(stationRecord.Species).Name)}
{Loc.GetString("records-species")} {(_proto.TryIndex<SpeciesPrototype>(stationRecord.Species, out var specProto)
? Loc.GetString(specProto.Name) : Loc.GetString("generic-not-available-shorthand"))}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/CriminalRecords/CriminalRecordsConsoleWindow.xaml.cs` at line
299, The species prototype lookup uses
_proto.Index<SpeciesPrototype>(stationRecord.Species) which can throw for
invalid IDs; change it to use
_proto.TryIndex<SpeciesPrototype>(stationRecord.Species, out var speciesProto)
and use speciesProto?.Name (or fall back to stationRecord.Species or a localized
"Unknown" string via Loc.GetString) when composing the Loc.GetString call so the
UI no longer throws on bad prototype IDs; update the expression around
Loc.GetString("records-species") to use the TryIndex result accordingly.

Comment on lines +37 to +45
for (int i = 0; i < record.Languages.Count; i++)
{
languages += Loc.GetString(prototypeManager.Index<LanguagePrototype>(record.Languages[i]).Name);

if (i != record.Languages.Count - 1)
languages += ", ";
else
languages += ".";
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

prototypeManager.Index<LanguagePrototype> can throw if a language ID is invalid.

The loop assumes all language IDs in record.Languages are valid prototypes. If any ID is invalid or missing, this will throw. Use TryIndex and skip invalid entries.

🛡️ Proposed fix
         string languages = string.Empty;

         for (int i = 0; i < record.Languages.Count; i++)
         {
-            languages += Loc.GetString(prototypeManager.Index<LanguagePrototype>(record.Languages[i]).Name);
+            if (!prototypeManager.TryIndex<LanguagePrototype>(record.Languages[i], out var langProto))
+                continue;
+            
+            if (languages.Length > 0)
+                languages += ", ";
+            languages += Loc.GetString(langProto.Name);
+        }

-            if (i != record.Languages.Count - 1)
-                languages += ", ";
-            else
-                languages += ".";
-        }
+        if (languages.Length > 0)
+            languages += ".";
+        else
+            languages = Loc.GetString("generic-not-available-shorthand");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Client/StationRecords/GeneralRecord.xaml.cs` around lines 37 - 45,
The loop building the languages string uses
prototypeManager.Index<LanguagePrototype>(...) which will throw for invalid IDs;
change it to use prototypeManager.TryIndex<LanguagePrototype>(id, out var proto)
inside the loop over record.Languages and skip any entries where TryIndex
returns false, then call Loc.GetString(proto.Name) only for valid prototypes and
append the comma/period separators as before; update references in
GeneralRecord.xaml.cs (the for loop that reads record.Languages and the use of
Loc.GetString) to use TryIndex and continue on invalid IDs.

Comment on lines +88 to +96
for (int i = 0; i < record.Languages.Count; i++)
{
languages += Loc.GetString(_prototypeManager.Index<LanguagePrototype>(record.Languages[i]).Name);

if (i != record.Languages.Count - 1)
languages += ", ";
else
languages += ".";
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use TryIndex for LanguagePrototype lookup to avoid exceptions.

_prototypeManager.Index<LanguagePrototype>() throws KeyNotFoundException if a language prototype ID in record.Languages doesn't exist. This can crash if record data references a removed or invalid language.

🛡️ Proposed fix using TryIndex
         for (int i = 0; i < record.Languages.Count; i++)
         {
-            languages += Loc.GetString(_prototypeManager.Index<LanguagePrototype>(record.Languages[i]).Name);
+            if (_prototypeManager.TryIndex<LanguagePrototype>(record.Languages[i], out var langProto))
+                languages += Loc.GetString(langProto.Name);
+            else
+                languages += Loc.GetString("generic-not-available-shorthand");

             if (i != record.Languages.Count - 1)
                 languages += ", ";
             else
                 languages += ".";
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Server/_WL/MedicalRecords/MedicalRecordsConsoleSystem.cs` around
lines 88 - 96, The loop building the languages string uses
_prototypeManager.Index<LanguagePrototype>(record.Languages[i]) which throws if
a prototype ID is missing; replace this with
_prototypeManager.TryIndex<LanguagePrototype>(id, out var proto) inside the
loop, and when TryIndex returns false use a safe fallback (e.g., the raw id or a
localized "Unknown" token) before calling Loc.GetString; ensure you still append
commas/periods exactly as the existing logic does and update references to
LanguagePrototype and record.Languages accordingly.

Comment on lines +106 to +107
{(!string.IsNullOrEmpty(record.SecurityRecord) ? record.SecurityRecord
: Loc.GetString("medical-records-console-no-record"))}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Wrong field: uses SecurityRecord instead of MedicalRecord.

This is the medical records console but it renders record.SecurityRecord. This exposes the wrong domain data and will show security notes instead of medical notes on printed records.

🐛 Proposed fix
-            {(!string.IsNullOrEmpty(record.SecurityRecord) ? record.SecurityRecord
+            {(!string.IsNullOrEmpty(record.MedicalRecord) ? record.MedicalRecord
             : Loc.GetString("medical-records-console-no-record"))}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Server/_WL/MedicalRecords/MedicalRecordsConsoleSystem.cs` around
lines 106 - 107, In the MedicalRecordsConsoleSystem rendering where the printed
record uses record.SecurityRecord, replace that field with the correct medical
notes field (record.MedicalRecord) so the console shows medical data instead of
security notes; locate the output code in MedicalRecordsConsoleSystem (the
template/string construction that references record.SecurityRecord) and swap the
property to record.MedicalRecord, ensuring null/empty checks remain (e.g., keep
the existing !string.IsNullOrEmpty(...) conditional) and preserve the fallback
to Loc.GetString("medical-records-console-no-record").

{Loc.GetString("records-confederation-edit")} {confederation}
{Loc.GetString("records-country-edit")} {(!string.IsNullOrEmpty(record.Country)
? record.Country : Loc.GetString("generic-not-available-shorthand"))}
{Loc.GetString("records-species")} {Loc.GetString(_prototypeManager.Index<SpeciesPrototype>(record.Species).Name)}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

_prototypeManager.Index<SpeciesPrototype> can throw if record.Species is invalid.

The confederation lookup defensively uses TryIndex (line 109), but the species lookup uses Index directly. For consistency and safety, use TryIndex here as well.

🛡️ Proposed fix
-                {Loc.GetString("records-species")} {Loc.GetString(_prototypeManager.Index<SpeciesPrototype>(record.Species).Name)}
+                {Loc.GetString("records-species")} {(_prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var specProto)
+                ? Loc.GetString(specProto.Name) : Loc.GetString("generic-not-available-shorthand"))}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Server/CriminalRecords/Systems/CriminalRecordsConsoleSystem.cs` at
line 122, The species lookup currently calls
_prototypeManager.Index<SpeciesPrototype>(record.Species) which can throw for an
invalid ID; change it to use
_prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var
speciesProto) and handle the failure (e.g., fall back to a safe string like
Loc.GetString("records-unknown") or record.Species) before calling
Loc.GetString(speciesProto.Name) so the console display no longer throws; update
the code around the existing Loc.GetString call in CriminalRecordsConsoleSystem
to use speciesProto when TryIndex succeeds and a fallback when it fails.

Comment on lines +89 to +97
for (int i = 0; i < record.Languages.Count; i++)
{
languages += Loc.GetString(_prototypeManager.Index<LanguagePrototype>(record.Languages[i]).Name);

if (i != record.Languages.Count - 1)
languages += ", ";
else
languages += ".";
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

_prototypeManager.Index<LanguagePrototype> can throw if a language ID is invalid.

The loop assumes all language prototype IDs are valid. Use TryIndex to handle potentially invalid or missing prototypes gracefully.

🛡️ Proposed fix
             string languages = string.Empty;

-            for (int i = 0; i < record.Languages.Count; i++)
+            var validLanguages = new List<string>();
+            foreach (var langId in record.Languages)
             {
-                languages += Loc.GetString(_prototypeManager.Index<LanguagePrototype>(record.Languages[i]).Name);
-
-                if (i != record.Languages.Count - 1)
-                    languages += ", ";
-                else
-                    languages += ".";
+                if (_prototypeManager.TryIndex<LanguagePrototype>(langId, out var langProto))
+                    validLanguages.Add(Loc.GetString(langProto.Name));
             }
+            languages = validLanguages.Count > 0 
+                ? string.Join(", ", validLanguages) + "."
+                : Loc.GetString("generic-not-available-shorthand");
📝 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.

Suggested change
for (int i = 0; i < record.Languages.Count; i++)
{
languages += Loc.GetString(_prototypeManager.Index<LanguagePrototype>(record.Languages[i]).Name);
if (i != record.Languages.Count - 1)
languages += ", ";
else
languages += ".";
}
string languages = string.Empty;
var validLanguages = new List<string>();
foreach (var langId in record.Languages)
{
if (_prototypeManager.TryIndex<LanguagePrototype>(langId, out var langProto))
validLanguages.Add(Loc.GetString(langProto.Name));
}
languages = validLanguages.Count > 0
? string.Join(", ", validLanguages) + "."
: Loc.GetString("generic-not-available-shorthand");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs`
around lines 89 - 97, The loop in GeneralStationRecordConsoleSystem that builds
the languages string uses _prototypeManager.Index<LanguagePrototype> which
throws for invalid IDs; change it to use
_prototypeManager.TryIndex<LanguagePrototype> (or TryIndex) when iterating
record.Languages, only append the prototype.Name when TryIndex returns true, and
otherwise skip the invalid ID (optionally log a warning via the system logger);
ensure commas and the final period are only added between/after successfully
appended language names so empty or missing prototypes don't create trailing
punctuation.

{Loc.GetString("records-confederation-edit")} {confederation}
{Loc.GetString("records-country-edit")} {(!string.IsNullOrEmpty(record.Country)
? record.Country : Loc.GetString("generic-not-available-shorthand"))}
{Loc.GetString("records-species")} {Loc.GetString(_prototypeManager.Index<SpeciesPrototype>(record.Species).Name)}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

_prototypeManager.Index<SpeciesPrototype> can throw if record.Species is invalid.

This is inconsistent with the defensive TryIndex used for confederation lookup. Use TryIndex for safety.

🛡️ Proposed fix
-                {Loc.GetString("records-species")} {Loc.GetString(_prototypeManager.Index<SpeciesPrototype>(record.Species).Name)}
+                {Loc.GetString("records-species")} {(_prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var specProto)
+                ? Loc.GetString(specProto.Name) : Loc.GetString("generic-not-available-shorthand"))}
📝 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.

Suggested change
{Loc.GetString("records-species")} {Loc.GetString(_prototypeManager.Index<SpeciesPrototype>(record.Species).Name)}
{Loc.GetString("records-species")} {(_prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var specProto)
? Loc.GetString(specProto.Name) : Loc.GetString("generic-not-available-shorthand"))}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Server/StationRecords/Systems/GeneralStationRecordConsoleSystem.cs`
at line 107, The code calls
_prototypeManager.Index<SpeciesPrototype>(record.Species) which can throw for
invalid IDs; change this to use
_prototypeManager.TryIndex<SpeciesPrototype>(record.Species, out var
speciesProto) and handle the false case safely (e.g., use a fallback string or
empty name) before calling Loc.GetString so GeneralStationRecordConsoleSystem no
longer risk-throws when record.Species is invalid.

Comment on lines +103 to +108
TryComp<LanguagesComponent>(player, out var languages);
TryComp<FingerprintComponent>(player, out var fingerprintComponent);
TryComp<DnaComponent>(player, out var dnaComponent);

if (languages == null)
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Early return on missing LanguagesComponent silently blocks record creation.

If an entity lacks a LanguagesComponent, CreateGeneralRecord returns early without creating any station record. This could cause players to silently lose their records without any logging or fallback. Consider using an empty list as fallback instead of blocking record creation entirely.

🛡️ Proposed fix — use empty list fallback
         TryComp<LanguagesComponent>(player, out var languages);
         TryComp<FingerprintComponent>(player, out var fingerprintComponent);
         TryComp<DnaComponent>(player, out var dnaComponent);

-        if (languages == null)
-            return;
+        var speakingLanguages = languages?.Speaking ?? new List<ProtoId<LanguagePrototype>>();

         var jobName = _role.GetSubnameByEntity(player, jobId) ?? jobProto.LocalizedName; //WL-changes

-        CreateGeneralRecord(station, idUid.Value, profile.Name, profile.Age, profile.Species, profile.Gender, profile.MedicalRecord, profile.SecurityRecord, profile.EmploymentRecord, profile.FullName, profile.DateOfBirth, profile.Confederation, profile.Country, profile.Height, languages.Speaking, jobId, jobName, fingerprintComponent?.Fingerprint, dnaComponent?.DNA, profile, records); //WL-changes
+        CreateGeneralRecord(station, idUid.Value, profile.Name, profile.Age, profile.Species, profile.Gender, profile.MedicalRecord, profile.SecurityRecord, profile.EmploymentRecord, profile.FullName, profile.DateOfBirth, profile.Confederation, profile.Country, profile.Height, speakingLanguages, jobId, jobName, fingerprintComponent?.Fingerprint, dnaComponent?.DNA, profile, records); //WL-changes
📝 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.

Suggested change
TryComp<LanguagesComponent>(player, out var languages);
TryComp<FingerprintComponent>(player, out var fingerprintComponent);
TryComp<DnaComponent>(player, out var dnaComponent);
if (languages == null)
return;
TryComp<LanguagesComponent>(player, out var languages);
TryComp<FingerprintComponent>(player, out var fingerprintComponent);
TryComp<DnaComponent>(player, out var dnaComponent);
var speakingLanguages = languages?.Speaking ?? new List<ProtoId<LanguagePrototype>>();
var jobName = _role.GetSubnameByEntity(player, jobId) ?? jobProto.LocalizedName; //WL-changes
CreateGeneralRecord(station, idUid.Value, profile.Name, profile.Age, profile.Species, profile.Gender, profile.MedicalRecord, profile.SecurityRecord, profile.EmploymentRecord, profile.FullName, profile.DateOfBirth, profile.Confederation, profile.Country, profile.Height, speakingLanguages, jobId, jobName, fingerprintComponent?.Fingerprint, dnaComponent?.DNA, profile, records); //WL-changes
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Content.Server/StationRecords/Systems/StationRecordsSystem.cs` around lines
103 - 108, CreateGeneralRecord currently returns early when
TryComp<LanguagesComponent>(player, out var languages) yields null, which
silently blocks record creation; instead, if languages is null, set a safe empty
fallback (e.g., an empty list/collection of known languages or a newly
constructed LanguagesComponent with no languages) and continue so record
creation proceeds. Locate the TryComp<LanguagesComponent>(player, out var
languages) usage in CreateGeneralRecord and replace the early return with a
fallback assignment to an empty languages collection, ensuring subsequent code
that reads languages uses that fallback.

@Litogin Litogin merged commit d806e64 into corvax-team:master Mar 1, 2026
14 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants