Skip to content

Customizable job titles - #214

Open
CrimeMoot wants to merge 9 commits into
masterfrom
AltJob
Open

Customizable job titles#214
CrimeMoot wants to merge 9 commits into
masterfrom
AltJob

Conversation

@CrimeMoot

@CrimeMoot CrimeMoot commented Jun 23, 2025

Copy link
Copy Markdown
Member

Описание PR

Добавлена поддержка альтернативных должностей для профессий в системе спавна персонажей и в системе работы с ролями (JobSystem). Теперь у персонажей на ID-картах и PDA может отображаться альтернативное название должности, заданное в профиле персонажа через JobAlternateTitlePrototype. Это позволяет гибко менять отображаемое название профессии, не меняя основного прототипа работы. Изначальный ресурс был взят с Gaby-station, но переделан, для нормальной работы и реализации. Часть кода использует систему кода с PDA должностей от Времени Приключений.

Также улучшено приветствие игрока при появлении роли (роль теперь отображается с альтернативным названием, если оно задано в ID-карте).

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

  • В StationSpawningSystem при спавне игрока теперь берётся альтернативное название должности из профиля персонажа (JobAlternateTitlePrototype), если оно есть, и устанавливается на ID-карту.
  • В JobSystem при отправке приветствия игроку вместо стандартного имени работы используется название должности с ID-карты, если оно там задано.
  • Добавлен новый прототип jobAlternateTitle для альтернативных должностей.
  • Исправлена логика отправки событий, работа с StartingGearEquippedEvent сделана корректно (через ref).
  • Обновлена логика загрузки профиля персонажа, чтобы учитывать альтернативные титулы.
  • Использование новых зависимостей и компонентов для работы с PDA и ID-картами.
  • Обеспечена обратная совместимость, если альтернативное название не задано, используется стандартное из JobPrototype.
  • Данную функцию можно просто отключить, используя CCvars, изначально включена. И нельзя будет менять должности.
[ic]
alternate_job_titles_enable  = false

Медиа

image
image
image
image

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

🆑 CrimeMoot,

  • add: Добавлена поддержка альтернативных названий должностей через JobAlternateTitlePrototype. Их можно выбрать в процессе персонализации персонажа. Важно отметить, что выбранная должность не сохраняется после выхода из игры — при следующем входе её нужно будет выбрать заново. (Пока что синхронизация с базой данных не реализована, поэтому выбранный вариант сохраняется только на текущую сессию.)
  • tweak: Приветствие игроков теперь использует альтернативное название должности, если оно задано на ID-карте.
  • tweak: При спавне персонажа на ID-карту и PDA устанавливается альтернативное название должности.

@github-actions github-actions Bot added S: Untriaged Status: Требуется сортировка по тегам size/L Changes: Localization Изменения затрагивают локализацию. Changes: UI labels Jun 23, 2025
@HyperB1 HyperB1 added S: Needs Review Status: Требуется рассмотрение A: Preferences Area: Menu and lobby customization settings which are saved in the database. labels Jun 23, 2025
@HyperB1

HyperB1 commented Jun 23, 2025

Copy link
Copy Markdown
Member
  • В JobSystem при отправке приветствия игроку вместо стандартного имени работы используется название должности с ID-карты, если оно там задано.

В AdventureTimeSS14/space_station_ADT#1416 было реализовано тоже самое, ID карта определяет название должности. Если этот код уже существует, то что ты сделал мне страшно даже думать.

Comment thread Resources/Prototypes/Roles/Jobs/Cargo/cargo_technician.yml Outdated
@HyperB1 HyperB1 changed the title AltJobs Customizable job titles Jun 23, 2025
Comment thread Resources/Locale/ru-RU/_Ganimed/job/job-alt-names.ftl
@RedSpyy

RedSpyy commented Jun 23, 2025

Copy link
Copy Markdown
Contributor

Вроде круто выглядит, прям сейчас не смогу протестить лично, но предварительно одобряю.
Если кто-то это уже затестил @HyperB1 @AltMapper отпишите сюда.

@CrimeMoot

Copy link
Copy Markdown
Member Author

В AdventureTimeSS14/space_station_ADT#1416 было реализовано тоже самое, ID карта определяет название должности. Если этот код уже существует, то что ты сделал мне страшно даже думать.

Отображение в манифесте с них и взята.

@Yuoko Yuoko added T: Refactor Type: Переработка систем и принципа работы кода P3: Standard Стандартный приоритет рассмотрения and removed S: Untriaged Status: Требуется сортировка по тегам labels Jul 4, 2025
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@CrimeMoot
CrimeMoot marked this pull request as ready for review September 4, 2025 10:41
@CrimeMoot
CrimeMoot requested a review from HyperB1 as a code owner September 4, 2025 10:41
@coderabbitai

coderabbitai Bot commented Sep 4, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Добавлена поддержка альтернативных титулов должностей: новый прототип JobAlternateTitlePrototype и поле AlternateTitles в JobPrototype; HumanoidCharacterProfile получает словарь JobAlternateTitles, сериализацию/валидацию и метод WithJobAltTitle. Серверная модель сохраняет данные в новую сущность DBJobAlternateTitle с миграциями (Postgres/SQLite); данные загружаются при логине и используются при спавне, заполнении ID/PDA, объявлениях и записях станции. Клиентский UI (RequirementsSelector, HumanoidProfileEditor) расширен для выбора альтернативных титулов и передачи выбора в профиль; поведение включается через CCVar ic.alternate_job_titles_enable. Добавлены локализация и пример прототипа для Cargo Technician.

Suggested reviewers

  • HyperB1
  • RedSpyy
  • AltMapper
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch AltJob

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 16

Caution

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

⚠️ Outside diff range comments (3)
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (1)

1042-1081: Исправить семантику отображения альтернативных титулов в RequirementsSelector
Сейчас в Setup пункты добавляются в порядке: сохранённый альтернативный титул → основной → остальные, и при выборе первого пункта (Id=0) вызывается OnSelectedTitle(null), что сбрасывает титул на базовый, вместо сохранённого.
Предложение: в методе Setup файла Content.Client/Lobby/UI/Roles/RequirementsSelector.xaml.cs

  • собрать List<ProtoId<JobAlternateTitlePrototype>> mappedAltTitles так, чтобы в нём первым элементом шёл defaultAltTitle (если не null), а затем остальные из altTitles;
  • добавлять в OptionButton сначала title (Id=0), потом все из mappedAltTitles (Id=1…);
  • в обработчике OnItemSelected делать
    var selected = args.Id == 0 
        ? null 
        : mappedAltTitles[args.Id - 1];
    OnSelectedTitle?.Invoke(selected);

Это гарантирует, что выбор сохранённого альтернативного титула не будет интерпретироваться как сброс.

Content.Server/Database/ServerDbBase.cs (2)

105-121: При сохранении слота стоит подгружать AltTitles у старого профиля.

Чтобы EF корректно отследил замену/очистку коллекции альтернативных титулов, добавьте Include аналогично Jobs/Traits.

             var oldProfile = db.DbContext.Profile
                 .Include(p => p.Preference)
                 .Where(p => p.Preference.UserId == userId.UserId)
                 .Include(p => p.Jobs)
                 .Include(p => p.Antags)
                 .Include(p => p.Traits)
+                .Include(p => p.AltTitles) // Ganimed-JobAlt
                 // ADT Start
                 .Include(p => p.Languages)

1-134: Реализовать миграции AltJobTitle или убрать работу с AltTitles
Up/Down в файлах миграций пустые – таблица DBJobAlternateTitle не создаётся. Либо добавить в Up создание таблицы с FK на Profile и уникальным индексом (ProfileId, RoleName) и включить Include для загрузки AltTitles, либо удалить чтение/запись AltTitles в ServerDbBase.

♻️ Duplicate comments (1)
Resources/Prototypes/Roles/Jobs/Cargo/cargo_technician.yml (1)

21-23: Нейминг и место хранения примера alt‑титула

  • ID CargoTechnitianTitleWtf: опечатка в “Technician” и сомнительный суффикс Wtf. Лучше привести к нейтральному ID и ключу локали.
  • Размещение примера в файле профессии перегружает YAML. Имеет смысл вынести jobAlternateTitle в отдельный файл (как уже предлагали).

Предлагаемый правящий дифф:

-  - CargoTechnitianTitleWtf
+  - CargoTechnicianAltJunior

- type: jobAlternateTitle
- id: CargoTechnitianTitleWtf
- name: job-name-alt-cargo-wtf
+type: jobAlternateTitle
+id: CargoTechnicianAltJunior
+name: job-name-alt-cargo-junior

И (по желанию) перенос блока jobAlternateTitle в, например, Resources/Prototypes/Roles/Jobs/Cargo/alt_titles.yml.

Also applies to: 35-37

🧹 Nitpick comments (13)
Content.Server.Database/Model.cs (1)

507-518: Именование полей — неточно; лучше хранить ProtoId в явных *Id

Чтобы было ясно, что сохраняются идентификаторы прототипов, а не локализованные строки, переименуйте:

-        public string RoleName { get; set; } = string.Empty;
-        public string AlternateTitle { get; set; } = string.Empty;
+        public string RoleId { get; set; } = string.Empty;
+        public string AlternateTitleId { get; set; } = string.Empty;

Не забудьте синхронизировать соответствующие места в ServerDbBase (конвертация профилей) и миграциях/индексах (RoleId вместо RoleName).

Content.Client/Lobby/UI/Roles/RequirementsSelector.xaml (1)

4-4: Контейнер вместо Label: проверьте совместимость с код-бихайндом и событиями мыши

Заменили Label на голый Control. Убедитесь, что в RequirementsSelector.xaml.cs вы действительно добавляете внутрь TitleContent либо Label, либо OptionButton, и что отсутствие MouseFilter="Stop" не вызывает «прокликивания» по контейнеру.

Рекомендация (по желанию): задать минимальный размер/выравнивание контейнера, чтобы OptionButton не прыгал при разных локалях:

-    <Control Name="TitleContent" Margin="5 0" />
+    <Control Name="TitleContent" Margin="5 0" MinSize="120 0" VerticalAlignment="Center" />
Content.Server/GameTicking/GameTicker.Spawning.cs (1)

234-245: Гард на CVAR (когда фича выключена)
По описанию PR, при alternate_job_titles_enable = false альтернативы не должны применяться. В этом месте CVAR явно не проверяется; надеюсь, профиль при загрузке не содержит alt‑титулы. Лучше добавить явный гард.

Вариант (если в классе уже есть IConfigurationManager):

+            if (_configurationManager.GetCVar(CCVars.ICAlternateJobTitlesEnable))
+            {
+                // блок применения alt-title (с валидацией как выше)
+            }

Если зависимости нет — ок, но убедитесь тестами, что при выключенной фиче jobName всегда берётся из jobPrototype.LocalizedName.

Content.Server/Access/Systems/IdCardSystem.cs (1)

97-116: API для смены должности на ID‑карте — ок; мелкие ниты

Логика корректна: обновляете ключ и локализованную строку, помечаете компонент грязным, возвращаете статус.

  • Приведите XML‑комментарии к единому языку проекта (в основном англ.), чтобы не смешивать RU/EN.
  • При необходимости аудит‑лога (если этот метод будет вызываться админками/консолями) — добавьте необязательный лог, но это уже за рамками PR.
-    /// Ganimed-JobAlt
-    /// Изменяет должность на карте, обновляя локализованный ключ и строку отображения.
+    /// Ganimed-JobAlt
+    /// Changes the job title on an ID card, updating both the loc key and the localized display string.
Content.Server/Access/Components/PresetIdCardComponent.cs (1)

15-17: Поле alternateTitle добавлено корректно; стоит упростить отладку через VV

Рекомендую пометить поле для просмотра в админ-панели/VV, чтобы быстро диагностировать несоответствия прототипов на сервере.

-    [DataField("alternateTitle")]
-    public ProtoId<JobAlternateTitlePrototype>? AlternateTitleId;
+    [ViewVariables(VVAccess.ReadOnly)]
+    [DataField("alternateTitle")]
+    public ProtoId<JobAlternateTitlePrototype>? AlternateTitleId;
Content.Server/StationRecords/Systems/StationRecordsSystem.cs (1)

161-179: Избежать дублирования логики с внешним репозиторием

С учётом замечания ревьюера о наличии аналогичной реализации в ADT, имеет смысл вынести согласованный порядок разрешения титула (ID‑карта → профиль.alt для job → дефолт из JobPrototype) в общий helper/систему, чтобы не расходились поведения и локализации в разных репозиториях.

Content.Server/Station/Systems/StationSpawningSystem.cs (1)

259-260: Исправьте форматирование региона.

Комментарии региона должны быть выровнены правильно для лучшей читаемости.

-    
-        #endregion Player spawning helpers
+    #endregion Player spawning helpers
Content.Client/Lobby/UI/Roles/RequirementsSelector.xaml.cs (1)

107-111: Избыточное копирование HashSet в List.

Создание нового List из HashSet кажется избыточным, так как можно работать напрямую с HashSet или преобразовать его в список при необходимости.

-            _altTitles = new List<ProtoId<JobAlternateTitlePrototype>>();
-            foreach (var entry in altTitles)
-            {
-                _altTitles.Add(entry);
-            }
+            _altTitles = altTitles.ToList();
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (1)

1091-1098: Подписываться на OnSelectedTitle только при включённой фиче.

Сейчас обработчик вешается всегда, но ранний if (!altJobTitlesEnable) return; в лямбде спасает. Чуть чище — подписывать внутри ветки if (altJobTitlesEnable).

-                    selector.OnSelectedTitle += selectedTitle =>
-                    {
-                        if (!altJobTitlesEnable)
-                            return;
-                        Profile = Profile?.WithJobAltTitle(job.ID, selectedTitle);
-                        SetDirty();
-                    };
+                    if (altJobTitlesEnable)
+                    {
+                        selector.OnSelectedTitle += selectedTitle =>
+                        {
+                            Profile = Profile?.WithJobAltTitle(job.ID, selectedTitle);
+                            SetDirty();
+                        };
+                    }
Content.Server/Database/ServerDbBase.cs (1)

239-249: Чтение AltTitles: устойчивость к дубликатам и мусорным данным.

Текущее altTitles.Add(...) кинет исключение при повторяющемся RoleName. Рекомендую безопасно перетирать либо предварительно фильтровать пустые значения.

-            foreach (var role in profile.AltTitles)
-            {
-                altTitles.Add(
-                    new ProtoId<JobPrototype>(role.RoleName),
-                    new ProtoId<JobAlternateTitlePrototype>(role.AlternateTitle)
-                );
-            }
+            foreach (var role in profile.AltTitles)
+            {
+                if (string.IsNullOrWhiteSpace(role.RoleName) || string.IsNullOrWhiteSpace(role.AlternateTitle))
+                    continue;
+                // перезаписываем последние значения без исключений
+                altTitles[new ProtoId<JobPrototype>(role.RoleName)] =
+                    new ProtoId<JobAlternateTitlePrototype>(role.AlternateTitle);
+            }

По желанию: дополнительно валидировать через TryIndex и отбрасывать несуществующие прототипы.

Content.Shared/Preferences/HumanoidCharacterProfile.cs (3)

49-55: Публичный изменяемый словарь нарушает «псевдо‑иммутабельность» профиля.

Сейчас JobAlternateTitlespublic и изменяемый. Рекомендую инкапсулировать поле и экспонировать IReadOnlyDictionary, изменения — только через WithJobAltTitle.

-        [DataField]
-        public Dictionary<ProtoId<JobPrototype>, ProtoId<JobAlternateTitlePrototype>> JobAlternateTitles = new();
+        [DataField]
+        private Dictionary<ProtoId<JobPrototype>, ProtoId<JobAlternateTitlePrototype>> _jobAlternateTitles = new();
+        public IReadOnlyDictionary<ProtoId<JobPrototype>, ProtoId<JobAlternateTitlePrototype>> JobAlternateTitles => _jobAlternateTitles;

И далее скорректировать конструктор/копирование/EnsureValid/WithJobAltTitle на работу с _jobAlternateTitles.


56-58: Поле AlternateJobTitle не используется. Удалить либо пояснить назначение.

Есть риск путаницы из‑за параллельного механизма. Если цель — только пер‑job‑карта, строковое поле лучше убрать.

-        [DataField("alternateJobTitle")]
-        public string? AlternateJobTitle { get; set; }
+        // Удалено: пер‑job карта JobAlternateTitles заменяет это поле.

585-587: SequenceEqual на словаре может флапать.

Dictionary не гарантирует порядок перечисления, так что SequenceEqual потенциально даст ложное неравенство. Для устойчивости сравнивайте по ключам/значениям независимо от порядка.

if (!JobAlternateTitles.Count.Equals(other.JobAlternateTitles.Count) ||
    JobAlternateTitles.Any(kv => !other.JobAlternateTitles.TryGetValue(kv.Key, out var v) || v != kv.Value))
    return false;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 8753382 and 2a548a4.

📒 Files selected for processing (23)
  • Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (3 hunks)
  • Content.Client/Lobby/UI/Roles/RequirementsSelector.xaml (1 hunks)
  • Content.Client/Lobby/UI/Roles/RequirementsSelector.xaml.cs (4 hunks)
  • Content.Server.Database/Migrations/Postgres/20241212015038_AltJobTitle.Designer.cs (1 hunks)
  • Content.Server.Database/Migrations/Postgres/20241212015038_AltJobTitle.cs (1 hunks)
  • Content.Server.Database/Migrations/Sqlite/20241212015023_AltJobTitle.Designer.cs (1 hunks)
  • Content.Server.Database/Migrations/Sqlite/20241212015023_AltJobTitle.cs (1 hunks)
  • Content.Server.Database/Model.cs (2 hunks)
  • Content.Server/Access/Components/PresetIdCardComponent.cs (1 hunks)
  • Content.Server/Access/Systems/IdCardSystem.cs (1 hunks)
  • Content.Server/Access/Systems/PresetIdCardSystem.cs (1 hunks)
  • Content.Server/Database/ServerDbBase.cs (3 hunks)
  • Content.Server/GameTicking/GameTicker.Spawning.cs (2 hunks)
  • Content.Server/Mind/MindSystem.cs (1 hunks)
  • Content.Server/Station/Systems/StationSpawningSystem.cs (4 hunks)
  • Content.Server/StationRecords/Systems/StationRecordsSystem.cs (2 hunks)
  • Content.Shared/CCVar/CCVars.cs (1 hunks)
  • Content.Shared/Preferences/HumanoidCharacterProfile.cs (9 hunks)
  • Content.Shared/Roles/JobAlternateTitlePrototype.cs (1 hunks)
  • Content.Shared/Roles/JobPrototype.cs (1 hunks)
  • Content.Shared/Roles/Jobs/JobRoleComponent.cs (0 hunks)
  • Resources/Locale/ru-RU/_Ganimed/job/job-alt-names.ftl (1 hunks)
  • Resources/Prototypes/Roles/Jobs/Cargo/cargo_technician.yml (2 hunks)
💤 Files with no reviewable changes (1)
  • Content.Shared/Roles/Jobs/JobRoleComponent.cs
🧰 Additional context used
🧬 Code graph analysis (12)
Content.Server.Database/Migrations/Postgres/20241212015038_AltJobTitle.cs (1)
Content.Server.Database/Migrations/Sqlite/20241212015023_AltJobTitle.cs (3)
  • AltJobTitle (8-21)
  • Up (11-14)
  • Down (17-20)
Content.Shared/Roles/JobAlternateTitlePrototype.cs (1)
Content.Shared/Roles/JobPrototype.cs (1)
  • Prototype (13-170)
Content.Server.Database/Model.cs (1)
Content.Server/Database/ServerDbBase.cs (1)
  • Profile (313-425)
Content.Server.Database/Migrations/Sqlite/20241212015023_AltJobTitle.cs (1)
Content.Server.Database/Migrations/Postgres/20241212015038_AltJobTitle.cs (3)
  • AltJobTitle (8-21)
  • Up (11-14)
  • Down (17-20)
Content.Server/Access/Systems/IdCardSystem.cs (1)
Content.Server/Station/Systems/StationSpawningSystem.cs (2)
  • EntityUid (70-81)
  • EntityUid (96-202)
Content.Server.Database/Migrations/Postgres/20241212015038_AltJobTitle.Designer.cs (2)
Content.Server.Database/Model.cs (1)
  • Server (783-795)
Content.Server.Database/Migrations/Postgres/20241212015038_AltJobTitle.cs (1)
  • AltJobTitle (8-21)
Content.Server/Station/Systems/StationSpawningSystem.cs (1)
Content.Server/Access/Systems/IdCardSystem.cs (1)
  • TryChangeJobTitle (105-116)
Content.Server.Database/Migrations/Sqlite/20241212015023_AltJobTitle.Designer.cs (2)
Content.Server.Database/Model.cs (1)
  • Server (783-795)
Content.Server.Database/Migrations/Sqlite/20241212015023_AltJobTitle.cs (1)
  • AltJobTitle (8-21)
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (2)
Content.Server/Database/ServerDbBase.cs (1)
  • Profile (313-425)
Content.Client/Lobby/UI/Roles/RequirementsSelector.xaml.cs (1)
  • Setup (81-164)
Content.Shared/Preferences/HumanoidCharacterProfile.cs (1)
Content.Server/Database/ServerDbBase.cs (1)
  • HumanoidCharacterProfile (200-311)
Content.Server/Access/Systems/PresetIdCardSystem.cs (2)
Content.Server/Station/Systems/StationSpawningSystem.cs (2)
  • EntityUid (70-81)
  • EntityUid (96-202)
Content.Server/Access/Systems/IdCardSystem.cs (1)
  • TryChangeJobTitle (105-116)
Content.Server/Database/ServerDbBase.cs (1)
Content.Server.Database/Model.cs (1)
  • DBJobAlternateTitle (508-517)
🔇 Additional comments (19)
Content.Shared/CCVar/CCVars.cs (1)

48-54: CVar для фичефлага — ок; семантика и флаги корректные

SERVER | REPLICATED подходит: сервер — источник истины, клиенту нужен визуальный флаг. Название и описание понятны.

Content.Server/GameTicking/GameTicker.Spawning.cs (1)

254-254: Добавление роли майнду — ок
Вызов с передачей jobPrototype: jobId оставлен без изменений по смыслу. Совместимо с логикой выше (имя для анонсов/логов берётся из jobName).

Content.Server/StationRecords/Systems/StationRecordsSystem.cs (1)

149-153: Проверка существующей записи — ок

Лаконичная проверка через pattern matching; поведение без изменений.

Content.Server/Access/Systems/PresetIdCardSystem.cs (2)

65-66: Имя на карте задаётся условно — ок

Безопасно избегает лишних вызовов при отсутствии предустановленного имени.


74-78: Валидация прототипа работы — ок

Ранняя проверка и логирование ошибки оставляют карту в согласованном состоянии.

Content.Server/Station/Systems/StationSpawningSystem.cs (3)

180-184: Корректная реализация загрузки альтернативных должностей.

Логика получения альтернативной должности из профиля персонажа реализована правильно. Код проверяет наличие альтернативной должности в словаре JobAlternateTitles и корректно индексирует прототип.


189-189: Убедитесь в корректной передаче параметра altTitle.

Изменение вызова SetPdaAndIdCardData с добавлением параметра altTitle выглядит корректно и соответствует обновленной сигнатуре метода.


222-241: Корректная обработка альтернативных должностей в ID-карте.

Логика установки названия должности в ID-карте правильно учитывает приоритет: если задана альтернативная должность, используется её локализованное название, иначе - стандартное из прототипа работы.

Content.Client/Lobby/UI/Roles/RequirementsSelector.xaml.cs (3)

24-25: Добавлены необходимые поля для поддержки альтернативных должностей.

Добавление поля _altTitles для хранения списка альтернативных должностей корректно.


27-27: Корректное добавление события для выбора альтернативной должности.

Событие OnSelectedTitle правильно определено для передачи выбранной альтернативной должности.


137-144: Проверить корректность вычисления индекса для _altTitles[args.Id - 1]. При отсутствии defaultAltTitle смещение позиций может измениться и привести к выходу за пределы списка или неверному элементу.

Content.Server.Database/Migrations/Sqlite/20241212015023_AltJobTitle.Designer.cs (2)

1-1903: Designer-файл миграции выглядит корректно.

Это автоматически сгенерированный файл Entity Framework для SQLite миграции. Он содержит полное определение модели базы данных после применения миграции AltJobTitle. Структура соответствует аналогичному файлу для PostgreSQL.


12-14: Миграции AltJobTitle для SQLite и PostgreSQL идентичны. Up/Down методы в обеих миграциях пустые и совпадают.

Content.Server/Database/ServerDbBase.cs (2)

361-373: Сериализация AltTitles в БД — ок.

Очистка и пересборка коллекции понятна и консистентна с остальными частями профиля.


48-66: Добавить Include для AltTitles в GetPlayerPreferencesAsync

Navigation-свойство Profile.AltTitles не загружается, из-за чего ConvertProfiles всегда получает пустой список. Вставьте сразу после Include(h => h.Traits):

 .Include(p => p.Profiles).ThenInclude(h => h.Traits)
+.Include(p => p.Profiles).ThenInclude(h => h.AltTitles)

Уточнить продуктовое ожидание и учесть возможные изменения миграций.

Content.Shared/Preferences/HumanoidCharacterProfile.cs (2)

720-734: Валидация alt‑титулов — хорошо.

Фильтрация по существующим прототипам и принадлежности к job.AlternateTitles устраняет мусор из БД/клиента.


885-904: GetHashCode: ок, но учитывайте инкапсуляцию.

Добавление словаря в хэш корректно; если перейдёте на приватное поле, обновите ссылку на _jobAlternateTitles.

Content.Server.Database/Migrations/Postgres/20241212015038_AltJobTitle.Designer.cs (2)

125-129: Индекс GIN по tsvector на admin_log.message — всё корректно.

Настройка Npgsql:TsVectorConfig = english + метод GIN выглядит верной и даёт быстрый полнотекстовый поиск по сообщениям.

Also applies to: 133-134


662-665: Фильтрованный уникальный индекс IX_job_one_high_priority — принято.

Ограничение уникальности только для priority = 3 соответствует ожидаемой инварианте «ровно один high-priority на профиль».

Comment thread Content.Client/Lobby/UI/Roles/RequirementsSelector.xaml.cs
Comment thread Content.Server.Database/Migrations/Postgres/20241212015038_AltJobTitle.cs Outdated
Comment on lines +25 to +26
.HasAnnotation("ProductVersion", "8.0.0")
.HasAnnotation("Relational:MaxIdentifierLength", 63);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Проверьте согласованность версий EF Core/Npgsql (ProductVersion = 8.0.0).

Снимок зафиксирован под EF Core 8.0.0. Убедитесь, что пакеты Microsoft.EntityFrameworkCore и Npgsql.EntityFrameworkCore.PostgreSQL в решении закреплены на совместимых 8.x и одинаковы для всех проектов, иначе возможны расхождения снимков/миграций.

Запустить для проверки:


🏁 Script executed:

#!/bin/bash
fd -a -g '**/*.csproj' -x rg -nP 'PackageReference Include="(Microsoft\.EntityFrameworkCore[^"]*|Npgsql\.EntityFrameworkCore\.PostgreSQL)"\s+Version="([^"]+)"' {}

Length of output: 166


🏁 Script executed:

#!/bin/bash
# Поиск версий EF Core и Npgsql в файлах .csproj
rg -nP '<PackageReference Include="(Microsoft\.EntityFrameworkCore[^"]*|Npgsql\.EntityFrameworkCore\.PostgreSQL)"\s*Version="([^"]+)"' -g '*.csproj'

Length of output: 150


🏁 Script executed:

#!/bin/bash
# Ищем упоминания EF Core и Npgsql в .csproj без указания версии
rg -n '<PackageReference Include="Microsoft.EntityFrameworkCore' -g '*.csproj' || true
rg -n '<PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL' -g '*.csproj' || true

# Ищем центральный файл управления версиями
files=$(fd -g 'Directory.Packages.props')
for file in $files; do
  echo "=== $file ==="
  rg -n '<PackageVersion Include="Microsoft.EntityFrameworkCore' "$file" || true
  rg -n '<PackageVersion Include="Npgsql.EntityFrameworkCore.PostgreSQL' "$file" || true
done

Length of output: 1185


Пересгенерируйте миграцию под EF Core 9.x и синхронизируйте версии пакетов
В Directory.Packages.props указаны Microsoft.EntityFrameworkCore.Design/Sqlite.Core @ 9.0.1 и Npgsql.EntityFrameworkCore.PostgreSQL @ 9.0.3, тогда как в файле миграции стоит ProductVersion = 8.0.0. Обновите аннотации или пересоздайте миграцию (Content.Server.Database/Migrations/Postgres/20241212015038_AltJobTitle.Designer.cs:25-26) под актуальные версии.

Comment thread Content.Server/StationRecords/Systems/StationRecordsSystem.cs
Comment thread Content.Shared/Preferences/HumanoidCharacterProfile.cs
Comment thread Content.Shared/Roles/JobAlternateTitlePrototype.cs
Comment thread Content.Shared/Roles/JobPrototype.cs
Comment thread Resources/Locale/ru-RU/_Ganimed/job/job-alt-names.ftl
…ьтернативных должностей в профль пользователя
@github-actions github-actions Bot added size/XL and removed size/L labels Sep 4, 2025

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 6

♻️ Duplicate comments (1)
Content.Server.Database/Model.cs (1)

133-143: Ужесточить уникальный индекс на (ProfileId, RoleName)

В ConvertProfiles (ServerDbBase.cs строки 247–250) бизнес-логика группирует записи по RoleName и выбирает единственный AlternateTitle, значит на уровне БД должно быть не более одного титула на роль для одного профиля. В Model.cs (строка 141) замените индекс:

- modelBuilder.Entity<DBJobAlternateTitle>()
-     .HasIndex(p => new { p.ProfileId, p.RoleName, p.AlternateTitle })
-     .IsUnique();
+ modelBuilder.Entity<DBJobAlternateTitle>()
+     .HasIndex(p => new { p.ProfileId, p.RoleName })
+     .IsUnique();

Перегенерируйте или вручную обновите миграции для PostgreSQL и SQLite.

🧹 Nitpick comments (5)
Content.Server.Database/Migrations/Sqlite/20250904114556_DBJobAlternateTitle.cs (1)

17-22: Небольшая придирка: явная семантика полей-идентификаторов

Поля role_name/alternate_title фактически хранят ProtoId. Чтобы снизить риск путаницы и регистрозависимости, можно в будущем переименовать колонки в job_id/alt_title_id и договориться о нормализации регистра на уровне приложения. Не blocker.

Content.Server/Database/ServerDbBase.cs (1)

363-375: Сохранение через Clear()+Add — корректно, но шумно по БД

Текущий подход приведёт к DELETE/INSERT даже при отсутствии изменений. Можно микрооптимизировать: вычислять дельту (удалять/добавлять только изменившиеся пары). Это снизит churn и потенциальные блокировки на горячих профилях. Не blocker.

Пример дельты:

-            profile.AltTitles.Clear();
-            foreach (var (role, title) in humanoid.JobAlternateTitles)
-            {
-                var newTitle = new DBJobAlternateTitle { RoleName = role.Id, AlternateTitle = title.Id };
-                profile.AltTitles.Add(newTitle);
-            }
+            var desired = humanoid.JobAlternateTitles.ToDictionary(kv => kv.Key.Id, kv => kv.Value.Id);
+            var existing = profile.AltTitles.ToDictionary(a => a.RoleName, a => a);
+
+            // удаляем лишние
+            foreach (var a in profile.AltTitles.Where(a => !desired.ContainsKey(a.RoleName)).ToArray())
+                profile.AltTitles.Remove(a);
+
+            // добавляем/обновляем
+            foreach (var (roleId, altId) in desired)
+            {
+                if (existing.TryGetValue(roleId, out var e))
+                    e.AlternateTitle = altId;
+                else
+                    profile.AltTitles.Add(new DBJobAlternateTitle { RoleName = roleId, AlternateTitle = altId });
+            }
Content.Server.Database/Model.cs (1)

519-530: Нейминг и ограничения длин — необязательные, но улучшающие читаемость/надёжность

  • Рассмотрите переименование AlternateTitle → AlternateTitleId (мы храним именно ID прототипа, а не локализованное имя).
  • Необязательно, но полезно задать MaxLength для RoleName/AlternateTitle, чтобы защититься от «случайно длинных» значений.
     public class DBJobAlternateTitle
     {
         public int Id { get; set; }
         public Profile Profile { get; set; } = null!;
         public int ProfileId { get; set; }
 
-        public string RoleName { get; set; } = string.Empty;
+        [MaxLength(64)]
+        public string RoleName { get; set; } = string.Empty;
 
-        public string AlternateTitle { get; set; } = string.Empty;
+        [MaxLength(64)]
+        public string AlternateTitle { get; set; } = string.Empty; // или AlternateTitleId
     }
Content.Server.Database/Migrations/Sqlite/SqliteServerDbContextModelSnapshot.cs (1)

636-665: Новая таблица dbjob_alternate_title — уточнить семантику и ввести валидацию.

  • Подтвердите, что AlternateTitle хранит ProtoId JobAlternateTitlePrototype, а не локализованный текст.
  • Опционально: переименовать колонку в alt_title_proto_id для ясности; добавить CHECK на непустое значение.

Пример CHECK (SQLite):

migrationBuilder.Sql("""
CREATE TEMP TRIGGER IF NOT EXISTS validate_dbjob_alt_title_ins
BEFORE INSERT ON dbjob_alternate_title
FOR EACH ROW BEGIN
  SELECT CASE WHEN length(trim(NEW.alternate_title))=0 THEN RAISE(ABORT,'alternate_title empty') END;
END;
""");

Уникальный индекс (ProfileId, RoleName, AlternateTitle) выглядит корректно и покрывает выборки по ProfileId.

Content.Server.Database/Migrations/Postgres/PostgresServerDbContextModelSnapshot.cs (1)

674-705: DBJobAlternateTitle (Postgres) — уточнения по данным и ограничениям.

  • Подтвердите хранение ProtoId в AlternateTitle; если так — можно переименовать в alt_title_proto_id.
  • Рассмотрите CHECK на непустые значения; при необходимости — citext/коллации, если ожидается регистронезависимость.

Пример CHECK (Postgres):

migrationBuilder.Sql("""
ALTER TABLE dbjob_alternate_title
  ADD CONSTRAINT ck_dbjob_alt_title_nonempty
  CHECK (length(btrim(alternate_title)) > 0 AND length(btrim(role_name)) > 0);
""");

Индекс (ProfileId, RoleName, AlternateTitle) — ОК.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2a548a4 and e8ed1c7.

📒 Files selected for processing (8)
  • Content.Server.Database/Migrations/Postgres/20250904114605_DBJobAlternateTitle.Designer.cs (1 hunks)
  • Content.Server.Database/Migrations/Postgres/20250904114605_DBJobAlternateTitle.cs (1 hunks)
  • Content.Server.Database/Migrations/Postgres/PostgresServerDbContextModelSnapshot.cs (6 hunks)
  • Content.Server.Database/Migrations/Sqlite/20250904114556_DBJobAlternateTitle.Designer.cs (1 hunks)
  • Content.Server.Database/Migrations/Sqlite/20250904114556_DBJobAlternateTitle.cs (1 hunks)
  • Content.Server.Database/Migrations/Sqlite/SqliteServerDbContextModelSnapshot.cs (6 hunks)
  • Content.Server.Database/Model.cs (4 hunks)
  • Content.Server/Database/ServerDbBase.cs (5 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
Content.Server.Database/Migrations/Sqlite/20250904114556_DBJobAlternateTitle.cs (2)
Content.Server.Database/Model.cs (3)
  • Server (795-807)
  • DBJobAlternateTitle (520-529)
  • PrimaryKey (1345-1354)
Content.Server.Database/Migrations/Postgres/20250904114605_DBJobAlternateTitle.cs (3)
  • DBJobAlternateTitle (9-48)
  • Up (12-40)
  • Down (43-47)
Content.Server.Database/Migrations/Postgres/20250904114605_DBJobAlternateTitle.cs (2)
Content.Server.Database/Model.cs (3)
  • Server (795-807)
  • DBJobAlternateTitle (520-529)
  • PrimaryKey (1345-1354)
Content.Server.Database/Migrations/Sqlite/20250904114556_DBJobAlternateTitle.cs (3)
  • DBJobAlternateTitle (8-47)
  • Up (11-39)
  • Down (42-46)
Content.Server.Database/Model.cs (2)
Content.Server.Database/Migrations/Postgres/20250904114605_DBJobAlternateTitle.cs (1)
  • DBJobAlternateTitle (9-48)
Content.Server.Database/Migrations/Sqlite/20250904114556_DBJobAlternateTitle.cs (1)
  • DBJobAlternateTitle (8-47)
Content.Server.Database/Migrations/Sqlite/20250904114556_DBJobAlternateTitle.Designer.cs (3)
Content.Server.Database/Model.cs (2)
  • Server (795-807)
  • DBJobAlternateTitle (520-529)
Content.Server.Database/Migrations/Postgres/20250904114605_DBJobAlternateTitle.cs (1)
  • DBJobAlternateTitle (9-48)
Content.Server.Database/Migrations/Sqlite/20250904114556_DBJobAlternateTitle.cs (1)
  • DBJobAlternateTitle (8-47)
Content.Server.Database/Migrations/Postgres/20250904114605_DBJobAlternateTitle.Designer.cs (2)
Content.Server.Database/Model.cs (2)
  • Server (795-807)
  • DBJobAlternateTitle (520-529)
Content.Server.Database/Migrations/Postgres/20250904114605_DBJobAlternateTitle.cs (1)
  • DBJobAlternateTitle (9-48)
Content.Server/Database/ServerDbBase.cs (3)
Content.Server.Database/Migrations/Postgres/20250904114605_DBJobAlternateTitle.cs (1)
  • DBJobAlternateTitle (9-48)
Content.Server.Database/Migrations/Sqlite/20250904114556_DBJobAlternateTitle.cs (1)
  • DBJobAlternateTitle (8-47)
Content.Server.Database/Model.cs (1)
  • DBJobAlternateTitle (520-529)
🔇 Additional comments (17)
Content.Server.Database/Migrations/Sqlite/20250904114556_DBJobAlternateTitle.cs (2)

35-38: Уточнить уникальность: одна запись на роль для профиля

Сейчас уникальный индекс включает alternate_title, тем самым позволяя хранить несколько альтернативных титулов на одну и ту же роль в одном профиле. В рантайме вы затем вынуждены выбирать «последнюю по Id», что делает поведение зависящим от порядка вставок и может флапать при гонках/миграциях.

Рекомендую зафиксировать инвариант «ровно одно выбранное альтернативное название на роль и профиль» индексом (profile_id, role_name) и упростить загрузку (без OrderByDescending(Id)).

[ suggest_essential_refactor ]

Пример правки миграции:

-            migrationBuilder.CreateIndex(
-                name: "IX_dbjob_alternate_title_profile_id_role_name_alternate_title",
-                table: "dbjob_alternate_title",
-                columns: new[] { "profile_id", "role_name", "alternate_title" },
-                unique: true);
+            migrationBuilder.CreateIndex(
+                name: "IX_dbjob_alternate_title_profile_id_role_name",
+                table: "dbjob_alternate_title",
+                columns: new[] { "profile_id", "role_name" },
+                unique: true);

25-32: FK с каскадным удалением — ок

Связь с profile(profile_id) и каскадное удаление корректны для «дочерних» записей альтернативных титулов. Замечаний нет.

Content.Server/Database/ServerDbBase.cs (4)

53-54: Загрузка AltTitles при логине — ок

Подключение AltTitles в Include гарантирует наличие данных в профиле. Так и нужно.


112-113: Трекинг AltTitles при сохранении — ок

Include(p => p.AltTitles) обязателен, чтобы Clear() действительно привёл к DELETE существующих строк. Исполнено корректно.


283-312: Актуализировать описание PR: сейчас альтернативные титулы сохраняются в БД

В описании PR сказано, что выбор не сохраняется между сессиями. По коду — сохраняется (миграции + сериализация/десериализация). Чтобы не вводить тестирующих в заблуждение, обновите комментарий в PR.


303-305: Все вызовы конструктора HumanoidCharacterProfile синхронизированы с новой сигнатурой

Content.Server.Database/Migrations/Postgres/20250904114605_DBJobAlternateTitle.cs (2)

35-39: Схема уникальности аналогично SQLite: закрепить одну запись на роль

Та же рекомендация: сделать уникальный индекс по (profile_id, role_name) вместо тройного, чтобы не допускать дублирования и убрать потребность выбирать «последнюю по Id».

[ suggest_essential_refactor ]

Правка миграции:

-            migrationBuilder.CreateIndex(
-                name: "IX_dbjob_alternate_title_profile_id_role_name_alternate_title",
-                table: "dbjob_alternate_title",
-                columns: new[] { "profile_id", "role_name", "alternate_title" },
-                unique: true);
+            migrationBuilder.CreateIndex(
+                name: "IX_dbjob_alternate_title_profile_id_role_name",
+                table: "dbjob_alternate_title",
+                columns: new[] { "profile_id", "role_name" },
+                unique: true);

26-33: FK и стратегия идентичности — ок

Первичный ключ и IdentityByDefault для PostgreSQL выставлены правильно, FK на profile с каскадом — ожидаемо. Замечаний нет.

Content.Server.Database/Model.cs (2)

51-51: DbSet зарегистрирован корректно

DbSet для DBJobAlternateTitle добавлен в контекст; соответствующие миграции присутствуют. Замечаний нет.


471-471: Навигация ок, согласована со стилем файла

Коллекция AltTitles оформлена как навигация с get-only и инициализацией по умолчанию — это соответствует остальным навигациям (Jobs, Traits и т.д.) и корректно трекается EF Core.

Content.Server.Database/Migrations/Sqlite/20250904114556_DBJobAlternateTitle.Designer.cs (1)

639-667: The requested verification is in progress.

Content.Server.Database/Migrations/Sqlite/SqliteServerDbContextModelSnapshot.cs (3)

1864-1874: Каскадное удаление AltTitles при удалении Profile — ОК.


2283-2284: Навигация Profile.AltTitles — ОК, соответствует новой связи.


18-18: Проверьте таргет-фреймворки
EF Core-пакеты централизованно заданы в Directory.Packages.props (Design 9.0.1, Sqlite.Core 9.0.1, Npgsql 9.0.3) — все 9.x; убедитесь, что TargetFramework (используемый через $(TargetFramework)) при сборке выставлен в net8.0 или net9.0.

Content.Server.Database/Migrations/Postgres/PostgresServerDbContextModelSnapshot.cs (3)

23-23: EF Core 9.0.1 (Postgres) — проверьте выравнивание версий.

Убедитесь, что Npgsql.EntityFrameworkCore.PostgreSQL = 9.x и остальные EF-пакеты/TFM согласованы во всех проектах.

Используйте тот же скрипт из комментария к SQLite-снапшоту.


1952-1962: Связь DBJobAlternateTitle → Profile с каскадом — корректно.


2371-2372: Навигация Profile.AltTitles — соответствует модели, без замечаний.

Comment thread Content.Server/Database/ServerDbBase.cs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
Content.Client/Lobby/UI/Roles/RequirementsSelector.xaml.cs (2)

122-133: Сообщайте исходный выбор через событие.

Сейчас OnSelectedTitle вызывается только при клике. Стоит один раз уведомить подписчиков после установки дефолтного выбора.

-            if (defaultAltTitle != null)
-            {
-                var index = _altTitles.FindIndex(x => x == defaultAltTitle);
-                if (index >= 0)
-                    titleOptions.SelectId(index + 1);
-                else
-                    titleOptions.SelectId(0);
-            }
-            else
-            {
-                titleOptions.SelectId(0);
-            }
+            var selectedId = 0;
+            if (defaultAltTitle != null)
+            {
+                var index = _altTitles.FindIndex(x => x.Equals(defaultAltTitle));
+                if (index >= 0)
+                    selectedId = index + 1; // +1, т.к. 0 — основной титул
+            }
+            titleOptions.SelectId(selectedId);
+            OnSelectedTitle?.Invoke(selectedId == 0 ? null : _altTitles[selectedId - 1]);

144-152: Фолбэк без прототипов: тоже генерируйте событие по умолчанию.

Когда нет protoMan/altTitles, имеет смысл явно сообщить null, чтобы подписчики не зависели от факта клика.

             var titleLabel = new Label
             {
                 ToolTip = description,
                 Text = title
             };
             TitleContent.AddChild(titleLabel);
+            OnSelectedTitle?.Invoke(null);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e8ed1c7 and 3d9b00b.

📒 Files selected for processing (1)
  • Content.Client/Lobby/UI/Roles/RequirementsSelector.xaml.cs (5 hunks)
🔇 Additional comments (2)
Content.Client/Lobby/UI/Roles/RequirementsSelector.xaml.cs (2)

135-142: Маппинг индексов OptionButton к _altTitles корректен.

Смещение на 1 относительно базового титула верное, обработка Id == 0 как null — ожидаемое поведение.


113-114: Проверьте локализацию аргумента title при вызове Setup в Content.Client/Lobby/UI/Roles/RequirementsSelector.xaml.cs:113–114. Убедитесь, что вызывающая сторона передаёт уже локализованную строку или оберните её в Loc.GetString(...).

Comment thread Content.Client/Lobby/UI/Roles/RequirementsSelector.xaml.cs
@ss14-ganimed ss14-ganimed deleted a comment from coderabbitai Bot Sep 5, 2025
@ss14-ganimed ss14-ganimed deleted a comment from coderabbitai Bot Sep 5, 2025

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
Content.Server/Access/Systems/PresetIdCardSystem.cs (1)

82-118: Не перезаписываем тайтл, уважаем CVAR и передаём LocId, а не локализованную строку

Сейчас:

  • титул на ID-карте перезаписывается даже если уже выставлен раньше (например, StationSpawningSystem), из‑за чего теряется выбранный пользователем альт‑титул;
  • в TryChangeJobTitle передаётся LocalizedName (строка), тогда как API ожидает LocId (ключ локализации) — это приведёт к некорректной локализации/поиску ключа;
  • нет учёта флага CCVar, отключающего фичу.

Предлагаю:

  • не трогать титул, если card.LocalizedJobTitle уже непустой;
  • оборачивать выбор альтернативных тайтлов в проверку CCVars.ICAlternateJobTitlesEnable;
  • передавать в TryChangeJobTitle altTitle.Name / job.Name (LocId), а не LocalizedName.

Применить внутри данного диапазона:

-        // Ganimed-JobAlt-start
-        if (!TryComp<IdCardComponent>(uid, out var card))
-        {
-            Log.Warning($"Entity {uid} does not have IdCardComponent, skipping title setup.");
-            return;
-        }
-
-        string? titleToSet = null;
-
-        if (id.AlternateTitleId != null &&
-            _prototypeManager.TryIndex(id.AlternateTitleId.Value, out JobAlternateTitlePrototype? altTitle))
-        {
-            titleToSet = altTitle.LocalizedName;
-        }
-        else if (job.AlternateTitles != null && job.AlternateTitles.Count > 0)
-        {
-            JobAlternateTitlePrototype? altFromJob = null;
-            foreach (var altId in job.AlternateTitles)
-            {
-                if (_prototypeManager.TryIndex(altId, out var proto))
-                {
-                    altFromJob = proto;
-                    break;
-                }
-            }
-
-            titleToSet = altFromJob?.LocalizedName ?? job.LocalizedName;
-        }
-        else
-        {
-            titleToSet = job.LocalizedName;
-        }
-
-        if (!string.IsNullOrEmpty(titleToSet))
-            _cardSystem.TryChangeJobTitle(uid, titleToSet);
-        // Ganimed-JobAlt-end
+        // Ganimed-JobAlt-start
+        if (!TryComp<IdCardComponent>(uid, out var card))
+        {
+            Log.Warning($"Entity {uid} does not have IdCardComponent, skipping title setup.");
+            return;
+        }
+
+        // Не перезаписываем уже заданный (например, персонализацией/спавном) титул
+        if (string.IsNullOrEmpty(card.LocalizedJobTitle))
+        {
+            var useAltTitles = _cfg.GetCVar(CCVars.ICAlternateJobTitlesEnable);
+            var titleSet = false;
+
+            if (useAltTitles
+                && id.AlternateTitleId != null
+                && _prototypeManager.TryIndex(id.AlternateTitleId.Value, out JobAlternateTitlePrototype? altTitle))
+            {
+                _cardSystem.TryChangeJobTitle(uid, altTitle.Name);
+                titleSet = true;
+            }
+            else if (useAltTitles && job.AlternateTitles != null && job.AlternateTitles.Count > 0)
+            {
+                foreach (var altId in job.AlternateTitles)
+                {
+                    if (_prototypeManager.TryIndex(altId, out JobAlternateTitlePrototype? proto))
+                    {
+                        _cardSystem.TryChangeJobTitle(uid, proto.Name);
+                        titleSet = true;
+                        break;
+                    }
+                }
+            }
+
+            if (!titleSet)
+                _cardSystem.TryChangeJobTitle(uid, job.Name);
+        }
+        // Ganimed-JobAlt-end

Дополнительно (вне диапазона) добавьте зависимости и using:

// using-и вверху файла
using Content.Shared.CCVar;
using Robust.Shared.Configuration;

// поле в классе PresetIdCardSystem
[Dependency] private readonly IConfigurationManager _cfg = default!;

Проверьте также YAML прототипы ID-карт: если вы рассчитываете на preset-альт‑титулы, у PresetIdCardComponent должны быть alternateTitle (AlternateTitleId).

Для самопроверки, найдите все неправильные вызовы TryChangeJobTitle с LocalizedName:

#!/bin/bash
rg -n -C2 -P 'TryChangeJobTitle\([^,]+,\s*[A-Za-z_][A-Za-z0-9_]*\.LocalizedName' --type cs
🧹 Nitpick comments (2)
Content.Server/Access/Systems/PresetIdCardSystem.cs (2)

65-67: Не перезаписывать пустое имя и избегать лишних вызовов

Лучше проверять на пустые/пробельные строки, чтобы не дёргать систему карт без необходимости.

-        if (id.IdName != null) // Ganimed-JobAlt
+        if (!string.IsNullOrWhiteSpace(id.IdName)) // Ganimed-JobAlt
             _cardSystem.TryChangeFullName(uid, id.IdName);

121-122: Проверка и индексация иконки должности

Если job.Icon — ProtoId?, проверка через string.IsNullOrEmpty некорректна. Лучше явная проверка на null.

-        if (!string.IsNullOrEmpty(job.Icon) && _prototypeManager.TryIndex(job.Icon, out var jobIcon)) // Ganimed-JobAlt
+        if (job.Icon != null && _prototypeManager.TryIndex(job.Icon, out var jobIcon)) // Ganimed-JobAlt
             _cardSystem.TryChangeJobIcon(uid, jobIcon);

Если job.Icon — всё-таки string, оставьте текущую проверку. Уточните тип и поправьте соответствующим образом.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3d9b00b and 99942a7.

📒 Files selected for processing (1)
  • Content.Server/Access/Systems/PresetIdCardSystem.cs (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CrimeMoot
PR: CrimeMoot/Ganimed14#0
File: :0-0
Timestamp: 2025-09-05T09:59:35.975Z
Learning: В системе альтернативных названий должностей Ganimed, если альтернативные титулы работают в манифестах и объявлениях при спавне, но не отображаются на ID картах, проблема в том, что PresetIdCardSystem перезаписывает титулы после StationSpawningSystem. Нужно добавлять поле alternateTitle в PresetIdCard компоненты прототипов ID карт.
Learnt from: CrimeMoot
PR: CrimeMoot/Ganimed14#0
File: :0-0
Timestamp: 2025-09-05T10:11:18.316Z
Learning: В системе альтернативных названий должностей Ganimed, проблема с отображением альтернативных титулов на ID картах решается добавлением поля alternateTitle в PresetIdCard компонент прототипа ID карты. Например, для CargoIDCard нужно добавить alternateTitle: CargoTechnitianTitleWtf в PresetIdCard компонент в файле identification_cards.yml.
📚 Learning: 2025-09-05T09:59:35.975Z
Learnt from: CrimeMoot
PR: CrimeMoot/Ganimed14#0
File: :0-0
Timestamp: 2025-09-05T09:59:35.975Z
Learning: В системе альтернативных названий должностей Ganimed, если альтернативные титулы работают в манифестах и объявлениях при спавне, но не отображаются на ID картах, проблема в том, что PresetIdCardSystem перезаписывает титулы после StationSpawningSystem. Нужно добавлять поле alternateTitle в PresetIdCard компоненты прототипов ID карт.

Applied to files:

  • Content.Server/Access/Systems/PresetIdCardSystem.cs
📚 Learning: 2025-09-05T10:11:18.316Z
Learnt from: CrimeMoot
PR: CrimeMoot/Ganimed14#0
File: :0-0
Timestamp: 2025-09-05T10:11:18.316Z
Learning: В системе альтернативных названий должностей Ganimed, проблема с отображением альтернативных титулов на ID картах решается добавлением поля alternateTitle в PresetIdCard компонент прототипа ID карты. Например, для CargoIDCard нужно добавить alternateTitle: CargoTechnitianTitleWtf в PresetIdCard компонент в файле identification_cards.yml.

Applied to files:

  • Content.Server/Access/Systems/PresetIdCardSystem.cs
🧬 Code graph analysis (1)
Content.Server/Access/Systems/PresetIdCardSystem.cs (2)
Content.Server/Station/Systems/StationSpawningSystem.cs (2)
  • EntityUid (70-81)
  • EntityUid (96-202)
Content.Server/Access/Systems/IdCardSystem.cs (1)
  • TryChangeJobTitle (105-116)
🔇 Additional comments (1)
Content.Server/Access/Systems/PresetIdCardSystem.cs (1)

74-79: Индексация JobPrototype — ок

Получение прототипа должности через TryIndex и обработка ошибки выглядят корректно.

Signed-off-by: CrimeMoot <wakeafa@gmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (1)

1045-1101: Учесть alternateTitle в PresetIdCardSystem и добавить его в прототипы ID-карт

  • В методе SetupIdName (Content.Server/Access/Systems/PresetIdCardSystem.cs) добавить проверку id.AlternateTitleId и при наличии применять альтернативный титул вместо (или до) перезаписи через id.IdName.
  • Во всех YAML-прототипах ID-карт (Resources/Prototypes/**/identification_cards.yml) в блоках - type: PresetIdCard для нужных ролей добавить поле
      alternateTitle: <defaultAltTitle>
    чтобы при спавне карты система подтягивала выбранный alt-титул.
🧹 Nitpick comments (2)
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (2)

1045-1045: Фича-флаг читается в нужном месте; можно чуть подсократить накладные расходы.

Опционально: вынести altJobTitlesEnable из внутреннего цикла по вакансиям (вычислять один раз на категорию или до обоих циклов), т.к. значение CVar редко меняется на лету.


1094-1101: Лишний guard в обработчике выбора титула.

Событие OnSelectedTitle возникает только когда alt‑титулы реально отрисованы (см. RequirementsSelector.Setup), поэтому проверка if (!altJobTitlesEnable) return; избыточна.

-                    selector.OnSelectedTitle += selectedTitle =>
-                    {
-                        if (!altJobTitlesEnable)
-                            return;
-                        Profile = Profile?.WithJobAltTitle(job.ID, selectedTitle);
-                        SetDirty();
-                    };
+                    selector.OnSelectedTitle += selectedTitle =>
+                    {
+                        Profile = Profile?.WithJobAltTitle(job.ID, selectedTitle);
+                        SetDirty();
+                    };
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 99942a7 and 30dce91.

📒 Files selected for processing (1)
  • Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CrimeMoot
PR: CrimeMoot/Ganimed14#0
File: :0-0
Timestamp: 2025-09-05T09:59:35.975Z
Learning: В системе альтернативных названий должностей Ganimed, если альтернативные титулы работают в манифестах и объявлениях при спавне, но не отображаются на ID картах, проблема в том, что PresetIdCardSystem перезаписывает титулы после StationSpawningSystem. Нужно добавлять поле alternateTitle в PresetIdCard компоненты прототипов ID карт.
Learnt from: CrimeMoot
PR: CrimeMoot/Ganimed14#0
File: :0-0
Timestamp: 2025-09-05T10:11:18.316Z
Learning: В системе альтернативных названий должностей Ganimed, проблема с отображением альтернативных титулов на ID картах решается добавлением поля alternateTitle в PresetIdCard компонент прототипа ID карты. Например, для CargoIDCard нужно добавить alternateTitle: CargoTechnitianTitleWtf в PresetIdCard компонент в файле identification_cards.yml.
🧬 Code graph analysis (1)
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (3)
Content.Server.Database/Model.cs (1)
  • Profile (444-480)
Content.Server/Database/ServerDbBase.cs (1)
  • Profile (315-427)
Content.Client/Lobby/UI/Roles/RequirementsSelector.xaml.cs (1)
  • Setup (81-163)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: build (ubuntu-latest)
  • GitHub Check: YAML map schema validator
  • GitHub Check: YAML Linter
  • GitHub Check: Test Packaging
🔇 Additional comments (1)
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (1)

1067-1067: Отрисовка иконки через Frame0() — ок.

Замена на jobIcon.Icon.Frame0() выглядит корректно и упрощает код.

Comment thread Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs
@Yuoko Yuoko added S: Conceptual Approval Статус: Концепция PR'а одобрена S: Requires testing Status: Требуется дополнительное разностороннее тестирование, чтобы выявить потенциальные проблемы. labels Sep 15, 2025
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Signed-off-by: CrimeMoot <wakeafa@gmail.com>
@HyperB1

HyperB1 commented Nov 1, 2025

Copy link
Copy Markdown
Member

Ожидаю возвращения к работе над PRом и реализации запрошенных мною изменений.

@CrimeMoot

Copy link
Copy Markdown
Member Author

@HyperB1 каких

@HyperB1

HyperB1 commented Dec 16, 2025

Copy link
Copy Markdown
Member

@HyperB1 каких

  1. Подвязка ограничений (как на профессии), то есть возможность установить requirement, например, на время роли/департамента или проверку на спонсора, расу или пол (последнее особенно нужно для локализации, если нельзя сделать локализацию, которая будет меняться от пола, хотя вроде бы можно).
  2. Добавить аналогичный эффект/проверку (LoadoutEffect), но уже для лодаута, на установленный альтернейм, чтобы можно было ограничивать пункты лодаута в зависимости от установленного названия профессии.

ПРИМЕЧАНИЕ: не обязательно делать именно так же отдельными эффектами как у лодаутов и ролей, но всё-же этот способ реализации не просто так используется в других местах.

@HyperB1 HyperB1 added the T: Enhancement Type: Новый контент, QoL и улучшения label Dec 24, 2025
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A: Preferences Area: Menu and lobby customization settings which are saved in the database. Changes: Localization Изменения затрагивают локализацию. Changes: UI P3: Standard Стандартный приоритет рассмотрения S: Conceptual Approval Статус: Концепция PR'а одобрена S: Merge Conflict S: Needs Review Status: Требуется рассмотрение S: Requires testing Status: Требуется дополнительное разностороннее тестирование, чтобы выявить потенциальные проблемы. size/XL T: Enhancement Type: Новый контент, QoL и улучшения T: Refactor Type: Переработка систем и принципа работы кода

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants