Fix UI lobby profile editor#1056
Conversation
|
Всё я пошёл |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughИзменения добавляют функционал управления загружаемыми предметами (loadouts), включая отображение и удаление недоступных загружаемых элементов, отслеживание количества символов в тексте описания, проверку пригодности загружаемых элементов в зависимости от требований персонажа, и миграцию структуры данных предпочтений загружаемых элементов. Также включены локализация на русский язык и различные редакторские правки. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@Content.Client/Lobby/UI/HumanoidProfileEditor.xaml`:
- Around line 199-214: The LoadoutsRemoveUnusableButton currently has an English
placeholder ("You shouldn't see this"); change its initial Text to a neutral
value (e.g., empty string or a localized placeholder) in the XAML so the user
never sees the English message, and ensure the UpdateLoadoutsRemoveButton method
is responsible for populating the final label at runtime for
LoadoutsRemoveUnusableButton.
In `@Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs`:
- Around line 2406-2441: CheckAndHideTabs only hides tabs but never restores
them; change the logic so it sets visibility explicitly based on current content
(i.e., call container.SetTabVisible(tabId, !shouldHide) instead of only hiding),
ensure CheckNestedContainer still recurses into nested NeoTabContainer and
returns whether it has any visible child controls, and update HasVisibleTraits
to reflect current selector.Visible state; also wire a call to
CheckAndHideTabs(TraitsTabs) (or the same updater) whenever filters/state that
affect trait visibility (e.g., "show unusable") change so tabs are re-evaluated
and shown again when traits become visible.
- Around line 2482-2503: The UpdateLoadoutsRemoveButton method sets the wrong
style class on LoadoutsRemoveUnusableButton causing OpenRight to be applied
instead of OpenLeft; update both branches in UpdateLoadoutsRemoveButton to
RemoveStyleClass(StyleBase.ButtonOpenLeft) /
AddStyleClass(StyleBase.ButtonOpenLeft) as appropriate (replace references to
StyleBase.ButtonOpenRight with StyleBase.ButtonOpenLeft) so the button matches
the XAML and avoids duplicate/conflicting rounded-corner classes.
- Around line 2457-2479: После удаления лодаутов в TryRemoveUnusableLoadouts
Profile обновляется, но не вызываются методы, которые обновляют превью и флаг
"dirty"; исправьте это, либо вызвав существующую общую логику (вызвать
OnLoadoutsChange() после присвоения Profile), либо явно вызвать
ReloadProfilePreview() и ReloadClothes() и обновление состояния сохранения после
строки Profile = currentProfile; чтобы превью и кнопки сохранения
синхронизировались с новым Profile.
|
Там если что в процессе всё |
|
Сделаю-ка я драфт |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
beb5ad1 to
69432b7
Compare
…klen/MYwwdpublic into Fix-UI-lobby-profile-editor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Content.Client/_White/Loadouts/LoadoutEntry.xaml.cs (1)
108-113:⚠️ Potential issue | 🟡 MinorПосле изменения
CanWearнужно сразу пересчитыватьVisible.Сейчас
Visibleобновляется только вShowUnusablesetter, поэтому послеEnsureIsWearableэлемент может остаться в устаревшем состоянии видимости.🧩 Возможная правка
PreferenceButton.RemoveStyleClass(StyleBase.ButtonDanger); CanWear = true; + Visible = CanWear || _showUnusable; PreferenceButton.Disabled = false; HeirloomButton.Visible = true; HeadingButton.Visible = true; PreferenceButton.MouseFilter = MouseFilterMode.Pass; @@ PreferenceButton.AddStyleClass(StyleBase.ButtonDanger); CanWear = false; + Visible = CanWear || _showUnusable;Also applies to: 121-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Client/_White/Loadouts/LoadoutEntry.xaml.cs` around lines 108 - 113, После того как EnsureIsWearable изменяет CanWear, сразу пересчитывай и обновляй видимость элементов вместо ожидания сеттера ShowUnusable: в методе EnsureIsWearable (и в аналогичном блоке на участках, перечисленных как also applies) вызывай существующий метод обновления видимости или прямо обновляй свойства PreferenceButton.Visible / HeirloomButton.Visible / HeadingButton.Visible в соответствии с текущими CanWear/ShowUnusable состояниями; то есть после изменения CanWear вызвать тот же код, что выполняется в сеттере ShowUnusable, чтобы не допустить устаревшего состояния видимости.
🧹 Nitpick comments (5)
Content.Client/FlavorText/FlavorText.xaml.cs (1)
32-56: Рекомендуется кэшироватьILocalizationManager.
IoCManager.Resolve<ILocalizationManager>()вызывается при каждом изменении текста. Для лучшей производительности стоит закэшировать локализационный менеджер в поле класса, поскольку этот метод срабатывает на каждое нажатие клавиши.♻️ Предлагаемый рефакторинг
public sealed partial class FlavorText : Control { public Action<string>? OnFlavorTextChanged; + private readonly ILocalizationManager _loc; public FlavorText() { RobustXamlLoader.Load(this); IoCManager.InjectDependencies(this); - var loc = IoCManager.Resolve<ILocalizationManager>(); - CFlavorTextInput.Placeholder = new Rope.Leaf(loc.GetString("flavor-text-placeholder")); + _loc = IoCManager.Resolve<ILocalizationManager>(); + CFlavorTextInput.Placeholder = new Rope.Leaf(_loc.GetString("flavor-text-placeholder")); CFlavorTextInput.OnTextChanged += _ => FlavorTextChanged(); UpdateCharacterCount(); }public void UpdateCharacterCount() { - var loc = IoCManager.Resolve<ILocalizationManager>(); var currentLength = CFlavorTextInput.TextLength; var maxLength = HumanoidCharacterProfile.MaxDescLength; - CharacterCountLabel.Text = loc.GetString("flavor-text-character-count", + CharacterCountLabel.Text = _loc.GetString("flavor-text-character-count", ("current", currentLength), ("max", maxLength));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Client/FlavorText/FlavorText.xaml.cs` around lines 32 - 56, The method UpdateCharacterCount currently calls IoCManager.Resolve<ILocalizationManager>() on every keystroke; cache the resolved ILocalizationManager in a private class-level field (e.g., _loc or _localizationManager) and initialize it once (constructor or control initialization) instead of resolving inside UpdateCharacterCount; then replace the inline call in UpdateCharacterCount with the cached field while leaving the rest of the logic (CharacterCountLabel updates, CFlavorTextInput.TextLength, HumanoidCharacterProfile.MaxDescLength checks) unchanged.Content.Client/Humanoid/MarkingPicker.xaml.cs (1)
246-251: Рассмотрите использование более общего ключа локализации.Ключ
humanoid-profile-editor-loadouts-customizeсодержит "loadouts" в названии, но используется в контексте маркировок (markings). Если это осознанное повторное использование — всё в порядке, но для консистентности можно создать общий ключ типаhumanoid-profile-editor-customizeили использовать уже существующий ключ для кнопки "Настроить".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Client/Humanoid/MarkingPicker.xaml.cs` around lines 246 - 251, Ключ локализации, используемый при создании кнопки customize (переменная customize в MarkingPicker.xaml.cs) содержит "loadouts" и неконсистентен с контекстом маркировок; замените строковый ключ "humanoid-profile-editor-loadouts-customize" на более общий или существующий ключ (например "humanoid-profile-editor-customize" или уже существующий ключ для "Настроить"), и обновите/добавьте соответствующие записи в файлы локализации, чтобы отражать новую ключ-значение пару.Content.Client/_White/DatumContainer/LocalDatumContainer.cs (1)
100-104: Избегайте лишней записи в userdata при пустой очистке.
Clear()всегда вызываетDirty(). Здесь лучше добавить ранний выход, чтобы не писать файл, если словарь уже пуст.♻️ Возможная правка
public void Clear() { + if (_data.Count == 0) + return; + _data.Clear(); Dirty(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Client/_White/DatumContainer/LocalDatumContainer.cs` around lines 100 - 104, Метод Clear() всегда вызывает Dirty(), что приводит к ненужной записи в userdata при уже пустом словаре _data; добавьте в начало метода ранний выход (например проверку _data.Count == 0 или _data.Any()) и не вызывайте Dirty() если нет элементов для очистки, сохранив текущее поведение для случая фактической модификации данных.Content.Client/_White/Loadouts/LoadoutPicker.xaml.cs (1)
102-109: Логику временного spawn для метаданных лучше вынести в helper с гарантированным cleanup.Сейчас код дублируется и удаление temp-сущности не защищено
finally.♻️ Вариант рефактора
- if (displayName == $"loadout-name-{loadoutPrototype.ID}" && loadoutPrototype.Items.Any()) - { - var tempEntity = _entityManager.SpawnEntity(loadoutPrototype.Items.First(), MapCoordinates.Nullspace); - displayName = _entityManager.GetComponent<MetaDataComponent>(tempEntity).EntityName; - _entityManager.DeleteEntity(tempEntity); - } + if (displayName == $"loadout-name-{loadoutPrototype.ID}" && loadoutPrototype.Items.Any()) + displayName = GetFirstItemMeta(loadoutPrototype).name; @@ - if (loadoutPrototype.Items.Any()) - { - var tempEntity = _entityManager.SpawnEntity(loadoutPrototype.Items.First(), MapCoordinates.Nullspace); - var itemName = _entityManager.GetComponent<MetaDataComponent>(tempEntity).EntityName; - NameEdit.PlaceHolder = itemName; - _entityManager.DeleteEntity(tempEntity); - } + if (loadoutPrototype.Items.Any()) + NameEdit.PlaceHolder = GetFirstItemMeta(loadoutPrototype).name; @@ - if (loadoutPrototype.Items.Any()) - { - var tempEntity = _entityManager.SpawnEntity(loadoutPrototype.Items.First(), MapCoordinates.Nullspace); - var itemDesc = _entityManager.GetComponent<MetaDataComponent>(tempEntity).EntityDescription; - DescriptionEdit.Placeholder = new Rope.Leaf(itemDesc); - _entityManager.DeleteEntity(tempEntity); - } + if (loadoutPrototype.Items.Any()) + DescriptionEdit.Placeholder = new Rope.Leaf(GetFirstItemMeta(loadoutPrototype).description);private (string name, string description) GetFirstItemMeta(LoadoutPrototype prototype) { var tempEntity = _entityManager.SpawnEntity(prototype.Items.First(), MapCoordinates.Nullspace); try { var meta = _entityManager.GetComponent<MetaDataComponent>(tempEntity); return (meta.EntityName, meta.EntityDescription); } finally { _entityManager.DeleteEntity(tempEntity); } }Also applies to: 438-459
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Client/_White/Loadouts/LoadoutPicker.xaml.cs` around lines 102 - 109, Extract the temporary-spawn + metadata-read logic into a single helper (e.g., GetFirstItemMeta or similar) so the spawn and DeleteEntity are always paired inside a try/finally; replace the duplicated blocks in LoadoutPicker.xaml.cs (the current block using _entityManager.SpawnEntity(...) and the similar block around lines 438-459) with calls to that helper which return the entity name/description, ensuring DeleteEntity is invoked in the finally to guarantee cleanup.Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (1)
2465-2484: Удаление unusable loadouts лучше сделать за один проход.Сейчас список фильтруется заново на каждый
loadoutId(лишние аллокации и O(n²)).♻️ Вариант упрощения
- var unusableLoadouts = _loadouts.Where(l => !l.Value).Select(l => l.Key.ID).ToList(); - - var currentProfile = Profile; - foreach (var loadoutId in unusableLoadouts) - { - var loadout = currentProfile.LoadoutPreferencesList.FirstOrDefault(l => l.LoadoutName == loadoutId); - if (loadout != null) - { - var newList = currentProfile.LoadoutPreferencesList.Where(l => l.LoadoutName != loadoutId).ToList(); - currentProfile = currentProfile.WithLoadoutPreference(newList); - } - } - - Profile = currentProfile; + var unusableLoadouts = _loadouts + .Where(l => !l.Value) + .Select(l => l.Key.ID) + .ToHashSet(); + + var newList = Profile.LoadoutPreferencesList + .Where(l => !unusableLoadouts.Contains(l.LoadoutName)) + .ToList(); + + Profile = Profile.WithLoadoutPreference(newList);🤖 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 2465 - 2484, The current TryRemoveUnusableLoadouts iterates unusable IDs and rebuilds LoadoutPreferencesList for each ID causing O(n²) allocations; instead collect unusable loadout IDs from _loadouts (e.g., var unusableLoadoutIds = _loadouts.Where(...).Select(...).ToHashSet()), then build a single filtered list once from currentProfile.LoadoutPreferencesList where LoadoutName is not in unusableLoadoutIds, and finally call currentProfile = currentProfile.WithLoadoutPreference(filteredList); update only once to avoid repeated filtering and extra allocations (refer to TryRemoveUnusableLoadouts, _loadouts, Profile, LoadoutPreferencesList, WithLoadoutPreference).
🤖 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/Humanoid/MarkingPicker.xaml.cs`:
- Around line 258-266: Удалите дублирующий вызов OnMarkingAdded из блока после
вызова MarkingAdd: метод MarkingAdd уже вызывает
OnMarkingAdded?.Invoke(_currentMarkings) (см. MarkingAdd), поэтому удалите
вторичный вызов OnMarkingAdded?.Invoke(_currentMarkings) после MarkingAdd;
дополнительно устраните неиспользуемую переменную categoryMarkings, либо
используйте discard (var _) в TryGetCategory, либо уберите попытку получения
категории, оставив логику PointsLeft/Remove корректной (методы:
_currentMarkings.TryGetCategory, _currentMarkings.PointsLeft,
_currentMarkings.Remove, _selectedMarkingCategory, Populate and
CMarkingSearch.Text).
In `@Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs`:
- Line 978: SetProfile() currently calls Loadouts.ClearCustomValues() on every
UI refresh which causes unnecessary persisted I/O and Dirty() writes; change the
logic so ClearCustomValues() is only invoked when the active slot/profile
actually changes. Inside HumanoidProfileEditor.SetProfile(), compare the
incoming profile/slot identifier (e.g. profileId or slotIndex passed into
SetProfile) against the editor's current activeProfile/activeSlot state and only
call Loadouts.ClearCustomValues() when they differ (or expose a boolean
parameter like forceClear to trigger clearing explicitly); leave refresh-only
paths to update UI without touching Loadouts.ClearCustomValues().
---
Outside diff comments:
In `@Content.Client/_White/Loadouts/LoadoutEntry.xaml.cs`:
- Around line 108-113: После того как EnsureIsWearable изменяет CanWear, сразу
пересчитывай и обновляй видимость элементов вместо ожидания сеттера
ShowUnusable: в методе EnsureIsWearable (и в аналогичном блоке на участках,
перечисленных как also applies) вызывай существующий метод обновления видимости
или прямо обновляй свойства PreferenceButton.Visible / HeirloomButton.Visible /
HeadingButton.Visible в соответствии с текущими CanWear/ShowUnusable
состояниями; то есть после изменения CanWear вызвать тот же код, что выполняется
в сеттере ShowUnusable, чтобы не допустить устаревшего состояния видимости.
---
Nitpick comments:
In `@Content.Client/_White/DatumContainer/LocalDatumContainer.cs`:
- Around line 100-104: Метод Clear() всегда вызывает Dirty(), что приводит к
ненужной записи в userdata при уже пустом словаре _data; добавьте в начало
метода ранний выход (например проверку _data.Count == 0 или _data.Any()) и не
вызывайте Dirty() если нет элементов для очистки, сохранив текущее поведение для
случая фактической модификации данных.
In `@Content.Client/_White/Loadouts/LoadoutPicker.xaml.cs`:
- Around line 102-109: Extract the temporary-spawn + metadata-read logic into a
single helper (e.g., GetFirstItemMeta or similar) so the spawn and DeleteEntity
are always paired inside a try/finally; replace the duplicated blocks in
LoadoutPicker.xaml.cs (the current block using _entityManager.SpawnEntity(...)
and the similar block around lines 438-459) with calls to that helper which
return the entity name/description, ensuring DeleteEntity is invoked in the
finally to guarantee cleanup.
In `@Content.Client/FlavorText/FlavorText.xaml.cs`:
- Around line 32-56: The method UpdateCharacterCount currently calls
IoCManager.Resolve<ILocalizationManager>() on every keystroke; cache the
resolved ILocalizationManager in a private class-level field (e.g., _loc or
_localizationManager) and initialize it once (constructor or control
initialization) instead of resolving inside UpdateCharacterCount; then replace
the inline call in UpdateCharacterCount with the cached field while leaving the
rest of the logic (CharacterCountLabel updates, CFlavorTextInput.TextLength,
HumanoidCharacterProfile.MaxDescLength checks) unchanged.
In `@Content.Client/Humanoid/MarkingPicker.xaml.cs`:
- Around line 246-251: Ключ локализации, используемый при создании кнопки
customize (переменная customize в MarkingPicker.xaml.cs) содержит "loadouts" и
неконсистентен с контекстом маркировок; замените строковый ключ
"humanoid-profile-editor-loadouts-customize" на более общий или существующий
ключ (например "humanoid-profile-editor-customize" или уже существующий ключ для
"Настроить"), и обновите/добавьте соответствующие записи в файлы локализации,
чтобы отражать новую ключ-значение пару.
In `@Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs`:
- Around line 2465-2484: The current TryRemoveUnusableLoadouts iterates unusable
IDs and rebuilds LoadoutPreferencesList for each ID causing O(n²) allocations;
instead collect unusable loadout IDs from _loadouts (e.g., var
unusableLoadoutIds = _loadouts.Where(...).Select(...).ToHashSet()), then build a
single filtered list once from currentProfile.LoadoutPreferencesList where
LoadoutName is not in unusableLoadoutIds, and finally call currentProfile =
currentProfile.WithLoadoutPreference(filteredList); update only once to avoid
repeated filtering and extra allocations (refer to TryRemoveUnusableLoadouts,
_loadouts, Profile, LoadoutPreferencesList, WithLoadoutPreference).
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
Content.Client/FlavorText/FlavorText.xamlContent.Client/FlavorText/FlavorText.xaml.csContent.Client/Humanoid/MarkingPicker.xaml.csContent.Client/Humanoid/SingleMarkingPicker.xaml.csContent.Client/Lobby/UI/HumanoidProfileEditor.xamlContent.Client/Lobby/UI/HumanoidProfileEditor.xaml.csContent.Client/_White/DatumContainer/LocalDatumContainer.csContent.Client/_White/Loadouts/LoadoutCategory.csContent.Client/_White/Loadouts/LoadoutEntry.xaml.csContent.Client/_White/Loadouts/LoadoutPicker.Categories.csContent.Client/_White/Loadouts/LoadoutPicker.xaml.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Content.Client/Lobby/UI/HumanoidProfileEditor.xaml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Content.Client/Humanoid/MarkingPicker.xaml.cs (1)
256-256: Закомментированный код можно удалить.Если старая логика проверки очков больше не нужна (заменена на автоматическое удаление маркировки при нехватке очков), рекомендуется удалить закомментированную строку для чистоты кода.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Client/Humanoid/MarkingPicker.xaml.cs` at line 256, Remove the obsolete commented-out line starting with "item.Pressed = item.Pressed && !Forced ? _currentMarkings.PointsLeft(_selectedMarkingCategory) > 0 : item.Pressed;" in MarkingPicker.xaml.cs; the old points-check logic is no longer used (auto-removal handles insufficient points), so simply delete that commented line to clean up the code and keep the surrounding logic unchanged (references: item.Pressed, Forced, _currentMarkings.PointsLeft, _selectedMarkingCategory).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Content.Client/Humanoid/MarkingPicker.xaml.cs`:
- Line 256: Remove the obsolete commented-out line starting with "item.Pressed =
item.Pressed && !Forced ? _currentMarkings.PointsLeft(_selectedMarkingCategory)
> 0 : item.Pressed;" in MarkingPicker.xaml.cs; the old points-check logic is no
longer used (auto-removal handles insufficient points), so simply delete that
commented line to clean up the code and keep the surrounding logic unchanged
(references: item.Pressed, Forced, _currentMarkings.PointsLeft,
_selectedMarkingCategory).
|
ой |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Content.Shared/Preferences/HumanoidCharacterProfile.cs (1)
519-543:⚠️ Potential issue | 🟠 Major
LoadoutPreferences.SequenceEqual(...)здесь не дает корректного value-сравнения.Для словаря это сравнение зависит от порядка вставки, а для
Loadoutеще и от того, как сам тип реализует equality. После сериализации/миграции два логически одинаковых набора loadout-ов легко окажутся "неравны", что ломает dirty-state и сравнение профилей. Здесь лучше сравнивать по ключам и содержимому loadout-а, а не по последовательностиKeyValuePair.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/Preferences/HumanoidCharacterProfile.cs` around lines 519 - 543, В методе MemberwiseEquals сравнение LoadoutPreferences через LoadoutPreferences.SequenceEqual неправильно для словаря — порядок или реализация Equals у Loadout могут ломать сравнение; замените это на сравнение по ключам и по содержимому: проверить равенство размеров, убедиться, что множества ключей совпадают (Keys/KeySet), затем для каждого ключа сравнить соответствующие значения loadout-ов с их содержательным сравнением (например вызвать у типа loadout метод MemberwiseEquals/эквивалентный equals или рекурсивно сравнить поля). Обновите место со ссылкой на LoadoutPreferences в MemberwiseEquals (и используйте _jobPriorities/_antagPreferences/_traitPreferences аналогично только если они тоже словари) чтобы избежать зависимости от порядка вставки.
🧹 Nitpick comments (1)
Content.Shared/Preferences/HumanoidCharacterProfile.cs (1)
56-61: Не отдавайте наружу изменяемый_loadoutPreferences.
HumanoidCharacterProfileвыше позиционируется как почти immutable, а здесь наружу уходит живойDictionary<string, Loadout>иValues-коллекция. Любой caller сможет менять профиль in-place, обходяWith...,EnsureValidи логику сравнения/dirty-state. Лучше экспонироватьIReadOnlyDictionary<string, Loadout>/IEnumerable<Loadout>и оставлять запись только через явные методы.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/Preferences/HumanoidCharacterProfile.cs` around lines 56 - 61, The public properties currently expose the mutable backing field _loadoutPreferences via LoadoutPreferences and its Values via LoadoutPreferencesList; change LoadoutPreferences to return an IReadOnlyDictionary<string, Loadout> (or a defensive copy) and keep LoadoutPreferencesList as IEnumerable<Loadout> (enumerable over values or a snapshot) so callers cannot mutate the internal Dictionary directly; ensure mutations remain only via the class' explicit mutator methods (e.g. With... and any internal methods used by EnsureValid) and update any consumers to use the read-only view or provided mutators.
🤖 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.Shared/_White/Preferences/LoadoutPreferencesSerializer.cs`:
- Around line 19-33: The SequenceDataNode validation in
LoadoutPreferencesSerializer.Validate(SequenceDataNode) is missing and allows
invalid items that later cause InvalidCastException in Read(SequenceDataNode)
which does node.Cast<MappingDataNode>(); update Validate(SequenceDataNode) to
iterate the sequence items and validate each as a MappingDataNode with required
fields "selected" and "loadoutName" (return a ValidatedSequenceNode containing
the per-item ValidationNode results), or alternatively change
Read(SequenceDataNode) to avoid unsafe Cast<T>() by using pattern matching/Type
checks when enumerating items and handling/reporting malformed entries; target
the Validate(SequenceDataNode) and Read(SequenceDataNode) methods and ensure
consistency between validation and reading logic.
In `@Content.Shared/Humanoid/SharedHumanoidAppearanceSystem.cs`:
- Around line 119-163: The migration currently keeps items when the "selected"
field is missing or invalid; update MigrateLoadoutPreferences so it treats
absence/non-true values as not selected (matching LoadoutPreferencesSerializer
behavior): when examining itemMapping.TryGet<ValueDataNode>("selected", out var
selectedNode) only accept the item if selectedNode exists and its Value equals
"true" (case-insensitive), otherwise skip the item; adjust the conditional
around selectedNode accordingly to ensure only explicitly true selections are
migrated.
In `@Content.Shared/Preferences/HumanoidCharacterProfile.cs`:
- Around line 373-374: The profile's Height/Width are only set in one creation
path (RandomWithSpecies) causing other paths (DefaultWithSpecies, raw profiles)
to keep 0/0; move the defaulting and clamping logic into a common initializer
such as EnsureValid (or DefaultWithSpecies) on HumanoidCharacterProfile so every
construction path normalizes sizes. In EnsureValid (or DefaultWithSpecies) read
speciesPrototype?.DefaultHeight and DefaultWidth, fall back to 1f, and clamp to
a sensible positive range (>0) before assigning to Height/Width; remove the
ad-hoc Height/Width assignments from RandomWithSpecies to avoid duplication.
Ensure references to RandomWithSpecies, DefaultWithSpecies, EnsureValid and
HumanoidCharacterProfile are updated so all creation flows call the centralized
normalizer.
In `@Resources/Locale/ru-RU/traits/traits.ftl`:
- Around line 442-445: The trait display name trait-name-HealingWordPower
contains an ASCII 'c' in "Лечущее cлово" which breaks Russian text matching;
open the traits.ftl entry for trait-name-HealingWordPower and replace the ASCII
'c' with the correct Cyrillic 'с' in "Лечущее слово", then scan nearby entries
(e.g., trait-description-HealingWordPower) for any other Latin/ASCII letters
accidentally used in Russian text and correct them to the proper Cyrillic
characters.
- Line 224: In the translation string in
Resources/Locale/ru-RU/traits/traits.ftl replace the typo "дуафтер" with a clear
phrase so the line reads like "процесс потребления еды и напитков проходит в
[color=green]2[/color] раза быстрее" (or use "процесс потребления еды и
напитков" in place of the single word), updating the sentence that currently
contains "Ваш дуафтер потребления еды и напитков..." to restore correct meaning.
- Around line 512-522: Fix the misspelling of "Медициский"/"медициского" to
"Медицинский"/"медицинского" in the localization entries: update
trait-name-MedicalEyesModule, trait-description-MedicalEyesModule, and
trait-description-OmniEyesModule so the words use the correct forms
("Медицинский" in the trait name and "медицинского" inside descriptions) to
preserve search/consistency.
---
Outside diff comments:
In `@Content.Shared/Preferences/HumanoidCharacterProfile.cs`:
- Around line 519-543: В методе MemberwiseEquals сравнение LoadoutPreferences
через LoadoutPreferences.SequenceEqual неправильно для словаря — порядок или
реализация Equals у Loadout могут ломать сравнение; замените это на сравнение по
ключам и по содержимому: проверить равенство размеров, убедиться, что множества
ключей совпадают (Keys/KeySet), затем для каждого ключа сравнить соответствующие
значения loadout-ов с их содержательным сравнением (например вызвать у типа
loadout метод MemberwiseEquals/эквивалентный equals или рекурсивно сравнить
поля). Обновите место со ссылкой на LoadoutPreferences в MemberwiseEquals (и
используйте _jobPriorities/_antagPreferences/_traitPreferences аналогично только
если они тоже словари) чтобы избежать зависимости от порядка вставки.
---
Nitpick comments:
In `@Content.Shared/Preferences/HumanoidCharacterProfile.cs`:
- Around line 56-61: The public properties currently expose the mutable backing
field _loadoutPreferences via LoadoutPreferences and its Values via
LoadoutPreferencesList; change LoadoutPreferences to return an
IReadOnlyDictionary<string, Loadout> (or a defensive copy) and keep
LoadoutPreferencesList as IEnumerable<Loadout> (enumerable over values or a
snapshot) so callers cannot mutate the internal Dictionary directly; ensure
mutations remain only via the class' explicit mutator methods (e.g. With... and
any internal methods used by EnsureValid) and update any consumers to use the
read-only view or provided mutators.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d718ea8c-f210-48ca-abde-4263828f4438
📒 Files selected for processing (4)
Content.Shared/Humanoid/SharedHumanoidAppearanceSystem.csContent.Shared/Preferences/HumanoidCharacterProfile.csContent.Shared/_White/Preferences/LoadoutPreferencesSerializer.csResources/Locale/ru-RU/traits/traits.ftl
Content.Shared/_White/Preferences/LoadoutPreferencesSerializer.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (2)
Resources/Locale/ru-RU/traits/traits.ftl (2)
512-512:⚠️ Potential issue | 🟡 MinorПоправьте
Медицинскийв модуле и описании омнивизора.На Line 512 и Line 522 опечатки
Медициский/медицискоговыбиваются из UI и мешают консистентному поиску по локализованным названиям.💡 Вариант правки
-trait-name-MedicalEyesModule = Глазной модуль КПБ: Медициский визор +trait-name-MedicalEyesModule = Глазной модуль КПБ: Медицинский визор - Этот модуль обеспечивает комбинированные преимущества медициского, диагностического и СБ визоров, так что этот модуль считается [color=red]контрабандным[/color] для тех, кто не работает в службе безопасности станции, и может быть отключен вашим работодателем перед отправкой на станцию. + Этот модуль обеспечивает комбинированные преимущества медицинского, диагностического и СБ визоров, так что этот модуль считается [color=red]контрабандным[/color] для тех, кто не работает в службе безопасности станции, и может быть отключен вашим работодателем перед отправкой на станцию.Also applies to: 522-522
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Locale/ru-RU/traits/traits.ftl` at line 512, Fix the typo "Медициский" -> "Медицинский" in the localization entries: update trait-name-MedicalEyesModule to use "Глазной модуль КПБ: Медицинский визор" and also correct the matching omnivisor description entry referenced at the other occurrence (the entry around Line 522) so both the module name and its description use "Медицинский" for consistent UI display and searchability.
224-224:⚠️ Potential issue | 🟡 MinorИсправьте артефакт
дуафтер.На Line 224 в описании остался мусорный фрагмент, из-за которого строка читается сломанно.
💡 Вариант правки
- Ваш дуафтер потребления еды и напитков проходит в [color=green]2[/color] раза быстрее. + Процесс потребления еды и напитков проходит в [color=green]2[/color] раза быстрее.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Locale/ru-RU/traits/traits.ftl` at line 224, Строка содержит мусорный фрагмент "дуафтер"; замените это слово на корректный термин (рекомендую "показатель") в тексте "Ваш дуафтер потребления еды и напитков проходит в [color=green]2[/color] раза быстрее.", чтобы получить "Ваш показатель потребления еды и напитков проходит в [color=green]2[/color] раза быстрее."; проверьте итоговую фразу на грамматику и единообразие со смежными локализациями.
🤖 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.Shared/Preferences/HumanoidCharacterProfile.cs`:
- Around line 538-539: GetHashCode for HumanoidCharacterProfile currently omits
Height and Width while MemberwiseEquals compares them, breaking the
Equals/GetHashCode contract; update the GetHashCode implementation (method
GetHashCode within HumanoidCharacterProfile) to incorporate Height and Width
into the combined hash computation (alongside the existing fields) so profiles
that differ only by size produce different hash codes.
In `@Resources/Locale/ru-RU/traits/traits.ftl`:
- Line 196: Фраза "чиновники старались связать его общим языком" неясна —
замените конец предложения так, чтобы явно указать субъект и объект:
переформулируйте строку с фразой "Уходя своими корнями в мандаринский китайский,
общий язык стал официальным языком Солнечного Альянса, а чиновники старались
связать его общим языком" на вариант вроде "…стал официальным языком Солнечного
Альянса, и чиновники стремились утвердить его в качестве общего языка" или
другой эквивалент, явно связывающий Альянс, язык и действия чиновников; обновите
только эту финальную часть, сохранив смысл начальной части.
- Around line 201-203: The phrase "Говорящий в Новых Нидерландах" in the
traits.ftl fragment is a literal and grammatically mismatched construction;
replace it with a grammatically coherent modifier such as "распространённый в
Новых Нидерландах" (or "зародившийся в Новых Нидерландах") so the clause agrees
with the sentence about the language and flows correctly with the surrounding
text; update the fragment containing that exact string to the chosen wording and
verify the surrounding punctuation and case remain correct.
- Line 347: The trait name key trait-name-ValyrianStandard uses "Валириский"
while the description keys (e.g., trait-desc-ValyrianStandard) use "Валерийский"
— make them consistent by choosing one spelling and updating all occurrences
(both trait-name-ValyrianStandard and trait-desc-ValyrianStandard) to the same
form; ensure every localized string that references the Valyrian language/people
uses that chosen spelling so searches and display are consistent.
- Line 594: Замените орфографически неверное "В следствие поломки или
технической недоработки вашего корпуса вода наносит вам
[color=yellow]шоковый[/color] урон." на корректный вариант "Вследствие поломки
или технической недоработки вашего корпуса вода наносит вам
[color=yellow]шоковый[/color] урон." в файле
Resources/Locale/ru-RU/traits/traits.ftl — найдите и исправьте текстовую строку,
содержащую "В следствие поломки..." и объедините слово "вследствие".
- Line 526: The phrase "между стандартным прицелом и режимом
[color=#013320]«Ночного видения»[/color]" is incorrect; change "стандартным
прицелом" to "стандартным зрением" in the string inside
Resources/Locale/ru-RU/traits/traits.ftl (the line containing "Ваше зрение было
улучшено с помощью модуля ночного видения, позволяющего пользователю
переключаться между ..."). Ensure the updated sentence reads with "между
стандартным зрением и режимом [color=#013320]«Ночного видения»[/color]" to
preserve meaning and formatting.
- Around line 273-274: Тут опечатка в конце второго предложения: фраза "прирост
потенциал значительно увеличивается" некорректна; замените её на грамматически
правильную формулировку, например "прирост потенциала значительно увеличивается"
или просто "псионический потенциал значительно увеличивается" в той же строке,
чтобы восстановить читаемость и согласование слов (исправление нужно в строке
содержащей "прирост потенциал значительно увеличивается").
- Around line 321-322: Replace the awkward phrase "атаки безоружным оружием" in
traits.ftl with a natural Russian variant such as "безоружные атаки" (or "атаки
без оружия") so the sentence reads: "В результате ваши безоружные атаки наносят
[color=orange]тупой[/color] урон вместо стандартного для вашего вида." Locate
and update the string containing "Естественное оружие... было
[color=red]удалено[/color]..." and ensure only the phrase is changed, preserving
the surrounding markup and color tags.
- Line 259: Согласуйте падежи для описаний оружейных трейтов: замените
некорректные формы "режущим оружиями" и "тупыми оружием" на согласованные
инструментальные формы (например, "режущим оружием" и "тупым оружием") во
фрагменте строки (текущая строка с "Ваш урон ... увеличен на
[color=green]35%[/color], но урон ... уменьшен на [color=red]20%[/color]") и
аналогично исправьте соответствующий повтор (строка с тем же описанием на 264),
чтобы обе части использовали одинаковый падеж и число.
---
Duplicate comments:
In `@Resources/Locale/ru-RU/traits/traits.ftl`:
- Line 512: Fix the typo "Медициский" -> "Медицинский" in the localization
entries: update trait-name-MedicalEyesModule to use "Глазной модуль КПБ:
Медицинский визор" and also correct the matching omnivisor description entry
referenced at the other occurrence (the entry around Line 522) so both the
module name and its description use "Медицинский" for consistent UI display and
searchability.
- Line 224: Строка содержит мусорный фрагмент "дуафтер"; замените это слово на
корректный термин (рекомендую "показатель") в тексте "Ваш дуафтер потребления
еды и напитков проходит в [color=green]2[/color] раза быстрее.", чтобы получить
"Ваш показатель потребления еды и напитков проходит в [color=green]2[/color]
раза быстрее."; проверьте итоговую фразу на грамматику и единообразие со
смежными локализациями.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4355702a-88c8-4e67-b544-28dae5f70747
📒 Files selected for processing (3)
Content.Shared/Humanoid/SharedHumanoidAppearanceSystem.csContent.Shared/Preferences/HumanoidCharacterProfile.csResources/Locale/ru-RU/traits/traits.ftl
|
Готово, @Spatison. |
Описание PR
Небольшой фикс разного рода UI в редакторе персонажа:
Медиа
Список
Лоадауты («Customize» тоже переведено, старый скрин):

Трейты:

Флавор:

Требования:

Изменения
🆑 Myaflic