[PORT] Nanochat #1009
[PORT] Nanochat #1009
Conversation
WalkthroughИзменения представляют собой масштабное добавление функциональности NanoChat в игру. Новая система позволяет игрокам обмениваться сообщениями через специальный картридж в КПК. Функционал включает создание чатов, отправку и получение сообщений, управление контактами, а также дополнительные возможности, такие как сканирование истории сообщений и взаимодействие с картами NanoChat. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
S: Untriaged
Changes: Sprites
|
RSI Diff Bot; head commit 16d3f4d merging into 6e9f070 Resources/Textures/_CorvaxNext/Misc/program_icons.rsi
Resources/Textures/_CorvaxNext/cartridge/cartridge.rsi
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (51)
Resources/Locale/ru-RU/ss14-ru/prototypes/_corvaxnext/entities/objects/devices/cartriges.ftl (1)
3-4: Рекомендуется использовать более формальный стиль описанияТекущее описание использует разговорный стиль ("чилловых", "початиться"), что может не соответствовать общему тону игры.
Предлагаемый вариант:
ent-NanoChatCartridge = картридж NanoChat - .desc = Для просто чилловых парней, которые любят початиться. + .desc = Картридж для обмена сообщениями между пользователями.Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatLogEntry.xaml.cs (2)
10-16: Предлагаю улучшить обработку входных параметровКонструктор не проверяет входные параметры на null, что может привести к NullReferenceException.
Рекомендуемые изменения:
public NanoChatLogEntry(int number, string time, string message) { RobustXamlLoader.Load(this); NumberLabel.Text = number.ToString(); - TimeLabel.Text = time; - MessageLabel.Text = message; + TimeLabel.Text = time ?? string.Empty; + MessageLabel.Text = message ?? string.Empty; }
1-17: Предложение по документацииРекомендуется добавить XML-документацию для публичного класса и его конструктора, чтобы улучшить понимание кода другими разработчиками.
Пример добавления документации:
+/// <summary> +/// Представляет элемент лога в интерфейсе NanoChat. +/// </summary> public sealed partial class NanoChatLogEntry : BoxContainer { + /// <summary> + /// Инициализирует новый экземпляр элемента лога NanoChat. + /// </summary> + /// <param name="number">Порядковый номер сообщения</param> + /// <param name="time">Время отправки сообщения</param> + /// <param name="message">Текст сообщения</param> public NanoChatLogEntry(int number, string time, string message)Content.Server/CartridgeLoader/Cartridges/LogProbeCartridgeComponent.cs (1)
23-27: Добавьте валидацию данных.Поле
ScannedNanoChatDataможет быть null, что потенциально может привести к ошибкам при использовании. Рекомендуется:
- Добавить проверки на null в местах использования
- Рассмотреть возможность инициализации пустым объектом вместо null
- Документировать в комментарии условия, при которых поле может быть null
Также рекомендуется добавить атрибут
[ViewVariables]для отладки:[DataField] +[ViewVariables(VVAccess.ReadWrite)] public NanoChatData? ScannedNanoChatData;Content.Shared/_CorvaxNext/NanoChat/SharedNanoChatSystem.cs (1)
39-45: Рассмотрите возможность устранения дублирования кода при использовании метода ResolveВо многих методах повторяется проверка с помощью
Resolve. Это приводит к дублированию кода и может усложнять его поддержку. Предлагаю создать вспомогательный метод для этой проверки или использовать более обобщенный подход, чтобы повысить читаемость и снизить вероятность ошибок.Content.Server/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatCartridgeSystem.cs (4)
31-31: Подумайте о конфигурации максимальной длины уведомленияЗначение
NotificationMaxLengthжестко задано как 64. Рассмотрите возможность вынести его в конфигурационный файл или сделать настраиваемым параметром, чтобы упростить его изменение без необходимости правки кода.
353-353: Удалите неформальный комментарий из кодаКомментарий
// I have no idea why this isn't public in the RadioSystemможет быть воспринят как непрофессиональный. Рекомендуется удалить или перефразировать его для поддержания делового стиля кода.
293-303: Оптимизируйте поиск карточек по номеруМетод
AttemptMessageDeliveryперебирает все компонентыNanoChatCardComponentдля поиска карточек с нужным номером, что может негативно сказаться на производительности при большом количестве пользователей. Рассмотрите возможность использования словаря или другой структуры данных для быстрого доступа к карточкам по их номеру.
447-466: Улучшите эффективность метода GetCardInfoМетод
GetCardInfoвыполняет полное перебирание всех компонентовNanoChatCardComponent, что может быть неэффективно. Предлагается использовать централизованный реестр или словарь для быстрого поиска информации о карточке по ее номеру.Content.Client/CartridgeLoader/Cartridges/LogProbeUiFragment.xaml.cs (1)
75-118: Предложение по рефакторингу: устранить дублирование кода при обработке сообщенийВ методе
DisplayNanoChatDataимеется повторяющийся код при обработке входящих и исходящих сообщений. Рекомендуется объединить общую логику в отдельный метод для повышения читаемости и удобства поддержки кода.Content.Client/CartridgeLoader/Cartridges/LogProbeUi.cs (1)
26-26: Улучшить стиль комментарияТекущий комментарий недостаточно информативен. Рекомендуется описать причину изменения более подробно.
- _fragment?.UpdateState(logProbeUiState); // Corvax-Next-PDAChat - just take the state + _fragment?.UpdateState(logProbeUiState); // Corvax-Next-PDAChat: Передаём полное состояние для поддержки данных NanoChatContent.Client/Access/UI/AgentIDCardWindow.xaml (1)
9-12: Привести стиль комментариев к единому форматуРекомендуется использовать более краткий формат комментариев без излишних Start/End маркеров.
- <!-- Corvax-Next-PDAChat-Start - Add NanoChat number field --> <Label Name="CurrentNumber" Text="{Loc 'agent-id-card-current-number'}" /> <LineEdit Name="NumberLineEdit" PlaceHolder="#0000" /> - <!-- Corvax-Next-PDAChat-End- --> + <!-- Corvax-Next-PDAChat: Поле для номера NanoChat -->Content.Shared/CartridgeLoader/Cartridges/LogProbeUiState.cs (2)
22-22: Исправить отступВ строке используется табуляция вместо пробелов, что не соответствует стилю остального кода.
- NanoChatData = nanoChatData; // Corvax-Next-PDAChat + NanoChatData = nanoChatData; // Corvax-Next-PDAChat
14-17: Дополнить документациюXML-документация свойства NanoChatData может быть более информативной.
- /// <summary> - /// Corvax-Next-PDAChat: The NanoChat data if a card was scanned, null otherwise - /// </summary> + /// <summary> + /// Содержит данные NanoChat при сканировании карты. + /// Значение null указывает на отсутствие отсканированной карты NanoChat. + /// </summary>Content.Shared/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatUiState.cs (2)
8-13: Рекомендуется добавить документацию к публичным свойствамДля улучшения поддержки кода, добавьте XML-документацию к публичным свойствам класса, описывающую их назначение и формат данных.
15-29: Рассмотрите использование init-only свойствТекущая реализация использует readonly поля. Рекомендуется рассмотреть использование init-only свойств (C# 9.0+) для более современного подхода к обеспечению неизменяемости:
- public readonly Dictionary<uint, NanoChatRecipient> Recipients = new(); + public Dictionary<uint, NanoChatRecipient> Recipients { get; init; } = new();Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatEntry.xaml.cs (2)
11-13: Рекомендуется использовать слабые событияТекущая реализация может привести к утечкам памяти. Рассмотрите использование слабых событий:
- public event Action<uint>? OnPressed; + public event EventHandler<uint>? OnPressed;
37-37: Вынесите цветовые константы в отдельный классРекомендуется выделить цветовые константы в отдельный класс стилей для улучшения поддержки и переиспользования.
Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatUi.cs (1)
21-24: Используйте именованные методы вместо лямбда-выраженийДля улучшения читаемости и отладки кода, рекомендуется выделить обработчик события в отдельный метод:
- _fragment.OnMessageSent += (type, number, content, job) => - { - SendNanoChatUiMessage(type, number, content, job, userInterface); - }; + _fragment.OnMessageSent += HandleMessageSent; + + private void HandleMessageSent(NanoChatUiMessageType type, uint? number, string? content, string? job) + { + SendNanoChatUiMessage(type, number, content, job, userInterface); + }Resources/Locale/en-US/_corvaxnext/cartridge-loader/nanochat.ftl (2)
12-12: Добавьте единицы измерения в сообщение о длинеРекомендуется уточнить единицы измерения в сообщении о превышении длины:
-nano-chat-message-too-long = Message too long ({$current}/{$max} characters) +nano-chat-message-too-long = Message too long ({$current}/{$max} symbols)
36-36: Улучшите формат сообщения в логахТекущий формат может быть неудобен для чтения. Рекомендуется добавить временную метку и улучшить разделители:
-log-probe-message-format = {$sender} → {$recipient}: {$content} +log-probe-message-format = [{$timestamp}] {$sender} ▶ {$recipient}: {$content}Resources/Locale/ru-RU/_corvaxnext/cartridge-loader/nanochat.ftl (2)
12-12: Улучшить формат сообщения о длине текстаТекущий формат может быть непонятен пользователю. Предлагаю изменить на более понятный вариант.
-nano-chat-message-too-long = Сообщение содержит ({$current}/{$max} символов) +nano-chat-message-too-long = Превышен лимит символов: {$current} из {$max}
25-25: Уточнить текст подсказки для поля должностиТекущий текст может быть не совсем понятен. Предлагаю сделать его более информативным.
-nano-chat-job-placeholder = Введите должность (необязательно) +nano-chat-job-placeholder = Укажите должность контакта (при наличии)Content.Client/CartridgeLoader/Cartridges/LogProbeUiFragment.xaml (1)
23-26: Вынести цвета в константы или темыЖестко закодированные цвета усложняют поддержку и изменение тем оформления.
Рекомендуется определить цвета в отдельном ресурсе тем:
-BackgroundColor="#000000FF" -BorderColor="#5a5a5a" +BackgroundColor="{ThemeResource BackgroundColorDefault}" +BorderColor="{ThemeResource BorderColorDefault}"Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatEntry.xaml (2)
24-25: Вынести цвета индикатора в ресурсы темЖестко закодированные цвета индикатора непрочитанных сообщений усложняют поддержку разных тем.
-BackgroundColor="#17c622" -BorderColor="#0f7a15" +BackgroundColor="{ThemeResource UnreadIndicatorBackground}" +BorderColor="{ThemeResource UnreadIndicatorBorder}"
8-8: Пересмотреть максимальный размер кнопки чатаФиксированный максимальный размер может ограничивать использование на разных устройствах.
-MaxSize="137 64" +MaxSize="200 80"Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatMessageBubble.xaml (2)
29-30: Улучшите документацию стилей.Комментарий "Colors set in code based on message sender" следует заменить на более информативный, указывающий конкретные цвета и их назначение.
15-35: Рассмотрите добавление поддержки тем оформления.Текущая реализация панели сообщений использует жестко заданные стили. Рекомендуется вынести стили в отдельный ресурс для поддержки различных тем оформления.
Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NewChatPopup.xaml (2)
11-13: Добавьте подсказки для валидации ввода.Для поля ввода номера следует добавить атрибуты, указывающие допустимый формат и ограничения (например, ToolTip с форматом номера).
36-49: Рассмотрите добавление клавиатурной навигации.Для улучшения доступности интерфейса рекомендуется добавить поддержку клавиши Enter для кнопки Create и Escape для кнопки Cancel.
Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatMessageBubble.xaml.cs (2)
46-47: Удалите непрофессиональный комментарий.Комментарий "fuuuuuck" следует заменить на конструктивное объяснение необходимости удаления дочерних элементов.
12-16: Добавьте документацию для цветовых констант.Рекомендуется добавить XML-документацию для цветовых констант, объясняющую их назначение и контекст использования.
Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NewChatPopup.xaml.cs (2)
31-32: Удалить дублирующий комментарий!Комментарий "Input validation" дублируется с строкой 27.
59-66: Улучшить валидацию входных данных!Рекомендуется:
- Добавить проверку на минимальное значение для number
- Вынести логику валидации в отдельный метод для каждого поля
private void ValidateInputs() { + private bool IsNumberValid() => + !string.IsNullOrWhiteSpace(NumberInput.Text) && + uint.TryParse(NumberInput.Text, out var num) && + num > 0; + + private bool IsNameValid() => + !string.IsNullOrWhiteSpace(NameInput.Text); + var isValid = !string.IsNullOrWhiteSpace(NumberInput.Text) && !string.IsNullOrWhiteSpace(NameInput.Text) && uint.TryParse(NumberInput.Text, out _); CreateButton.Disabled = !isValid; }Content.Server/_CorvaxNext/NanoChat/NanoChatSystem.cs (2)
48-51: Удалить или восстановить закомментированный код!Закомментированный код для показа всплывающих сообщений следует либо удалить, либо восстановить с соответствующей локализацией.
73-102: Оптимизировать логику перемешивания сообщений!Текущая реализация может быть оптимизирована:
- Использовать LINQ для фильтрации сообщений
- Применить параллельную обработку для больших наборов данных
private void ScrambleMessages(NanoChatCardComponent component) { + var messagesToScramble = component.Messages + .SelectMany(kvp => kvp.Value.Select(msg => (kvp.Key, msg))) + .Where(_ => _random.Prob(0.5f)) + .ToList(); + + Parallel.ForEach(messagesToScramble, item => + { + var (recipientNumber, message) = item; + message.Content = ScrambleText(message.Content); + });Content.Client/Access/UI/AgentIDCardWindow.xaml.cs (2)
65-71: Добавить XML документацию!Для нового метода
OnNumberEnteredследует добавить XML документацию, описывающую его назначение и параметры.+ /// <summary> + /// Обрабатывает событие ввода номера, проверяет его корректность и вызывает событие изменения. + /// </summary> + /// <param name="args">Аргументы события редактирования строки</param> private void OnNumberEntered(LineEdit.LineEditEventArgs args)
72-77: Улучшить форматирование номера!Метод
SetCurrentNumberиспользует простое форматирование. Рекомендуется добавить дополнительную валидацию и обработку ошибок.public void SetCurrentNumber(uint? number) { + if (number.HasValue && number.Value == 0) + { + NumberLineEdit.Text = string.Empty; + return; + } NumberLineEdit.Text = number?.ToString("D4") ?? ""; }Content.Shared/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatUiMessageEvent.cs (3)
59-59: Удалите неформальный комментарий.Комментарий "putting this here because i can" является неуместным и непрофессиональным.
105-105: Рассмотрите использование DateTimeOffset вместо TimeSpan.TimeSpan для временной метки сообщения может быть проблематичным при синхронизации между клиентами в разных часовых поясах.
- public TimeSpan Timestamp; + public DateTimeOffset Timestamp;Also applies to: 121-121
144-154: Добавьте ограничение на размер коллекций.Неограниченные словари
RecipientsиMessagesмогут привести к проблемам с памятью. Рекомендуется добавить максимальные лимиты.Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatUiFragment.xaml (2)
149-152: Добавьте ограничение длины ввода в UI.LineEdit для ввода сообщения должен иметь явное ограничение длины, соответствующее
MaxMessageLengthиз code-behind.<LineEdit Name="MessageInput" PlaceHolder="{Loc nano-chat-message-placeholder}" HorizontalExpand="True" + MaxLength="256" StyleClasses="OpenRight" />
125-133: Оптимизируйте производительность списка сообщений.Для улучшения производительности при большом количестве сообщений:
- Добавьте виртуализацию списка
- Реализуйте пагинацию или подгрузку по мере прокрутки
Resources/Prototypes/_Backmen/name_identifier_groups.yml (1)
22-25: Рекомендуется добавить минимальное значение для идентификатора.Для группы идентификаторов NanoChat указано только максимальное значение (9999). Рекомендуется также добавить
minValueдля предотвращения использования некорректных значений.- type: nameIdentifierGroup id: NanoChat maxValue: 9999 + type: nameIdentifierGroup + id: NanoChat + minValue: 1000 + maxValue: 9999Resources/Prototypes/_CorvaxNext/Entities/Objects/Devices/cartridges.yml (1)
1-20: Рекомендуется добавить контроль доступа для картриджа.Сущность
NanoChatCartridgeкорректно определена, но рекомендуется добавить компонентAccessReaderдля ограничения использования картриджа определенными ролями.- type: ActiveRadio channels: - Common + - type: AccessReader + access: [["NanoChat"]]Resources/Prototypes/Entities/Objects/Misc/identification_cards.yml (1)
847-848: Рекомендуется добавить комментарии к настройкам компонента.Добавление компонента
NanoChatCardкорректно, но рекомендуется документировать причину, по которой уведомления отключены по умолчанию.- - type: NanoChatCard # Corvax-Next-PDAChat - notificationsMuted: true + # Уведомления отключены по умолчанию для предотвращения спама + - type: NanoChatCard # Corvax-Next-PDAChat + notificationsMuted: trueResources/Prototypes/Entities/Objects/Devices/pda.yml (1)
87-87: Рекомендуется вынести общие картриджи в базовый прототип.Картридж
NanoChatCartridgeдобавляется в несколько типов PDA. Рекомендуется вынести его в списокpreinstalledбазового прототипаBasePDAдля уменьшения дублирования кода.- type: CartridgeLoader uiKey: enum.PdaUiKey.Key preinstalled: - CrewManifestCartridge - NotekeeperCartridge - BankCartridge - GlimmerMonitorCartridge - NewsReaderCartridge + - NanoChatCartridge # Общий картридж для всех PDAAlso applies to: 146-146, 160-160, 859-859, 930-930, 955-955, 1104-1104, 1273-1273, 1301-1301, 1446-1446
Content.Server/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatCartridgeComponent.cs (2)
21-25: Рассмотреть возможность валидации RadioChannelХотя значение по умолчанию установлено как "Common", рекомендуется добавить проверку существования указанного канала в прототипах.
Предлагаю добавить валидацию в конструкторе или при инициализации:
+ [DataField, ValidatePrototypeId<RadioChannelPrototype>] public ProtoId<RadioChannelPrototype> RadioChannel = "Common";
1-26: Рекомендации по архитектуреКомпонент является частью большей системы чата. Рекомендуется:
- Добавить интерфейс ICartridge для стандартизации взаимодействия с картриджами
- Рассмотреть добавление событий для коммуникации между компонентами
- Документировать взаимодействие с другими системами
Content.Server/_CorvaxNext/CartridgeLoader/Cartridges/LogProbeCartridgeSystem.NanoChat.cs (2)
8-14: Добавьте документацию для улучшения читаемости кодаРекомендуется добавить XML-документацию для частичного класса и метода инициализации, описывающую:
- Назначение системы NanoChat
- Обрабатываемые события
- Взаимодействие с другими системами
+/// <summary> +/// Система для обработки картриджей LogProbe с поддержкой функционала NanoChat. +/// </summary> public sealed partial class LogProbeCartridgeSystem { + /// <summary> + /// Инициализирует обработчики событий NanoChat. + /// </summary> private void InitializeNanoChat()
38-58: Оптимизируйте создание словарейВ методе
OnMessageReceivedпроисходит избыточное копирование словарей. Рекомендуется использовать существующие коллекции, где это возможно.private void OnMessageReceived(ref NanoChatMessageReceivedEvent args) { var query = EntityQueryEnumerator<LogProbeCartridgeComponent, CartridgeComponent>(); while (query.MoveNext(out var uid, out var probe, out var cartridge)) { if (probe.ScannedNanoChatData == null || GetEntity(probe.ScannedNanoChatData.Value.Card) != args.CardUid) continue; if (!TryComp<NanoChatCardComponent>(args.CardUid, out var card)) continue; + if (card.Messages == null) + continue; probe.ScannedNanoChatData = new NanoChatData( probe.ScannedNanoChatData.Value.Recipients, - new Dictionary<uint, List<NanoChatMessage>>(card.Messages), + card.Messages,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
Resources/Textures/Backmen/Objects/Weapons/Gunsx64/Flamethrowers/tank.rsi/cart-chat.pngis excluded by!**/*.pngResources/Textures/_CorvaxNext/Interface/VerbIcons/bell.svgis excluded by!**/*.svgResources/Textures/_CorvaxNext/Interface/VerbIcons/bell.svg.pngis excluded by!**/*.pngResources/Textures/_CorvaxNext/Interface/VerbIcons/bell_muted.pngis excluded by!**/*.pngResources/Textures/_CorvaxNext/Misc/program_icons.rsi/nanochat.pngis excluded by!**/*.png
📒 Files selected for processing (45)
Content.Client/Access/UI/AgentIDCardBoundUserInterface.cs(2 hunks)Content.Client/Access/UI/AgentIDCardWindow.xaml(1 hunks)Content.Client/Access/UI/AgentIDCardWindow.xaml.cs(2 hunks)Content.Client/CartridgeLoader/Cartridges/LogProbeUi.cs(1 hunks)Content.Client/CartridgeLoader/Cartridges/LogProbeUiFragment.xaml(1 hunks)Content.Client/CartridgeLoader/Cartridges/LogProbeUiFragment.xaml.cs(2 hunks)Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatEntry.xaml(1 hunks)Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatEntry.xaml.cs(1 hunks)Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatLogEntry.xaml(1 hunks)Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatLogEntry.xaml.cs(1 hunks)Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatMessageBubble.xaml(1 hunks)Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatMessageBubble.xaml.cs(1 hunks)Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatUi.cs(1 hunks)Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatUiFragment.xaml(1 hunks)Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatUiFragment.xaml.cs(1 hunks)Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NewChatPopup.xaml(1 hunks)Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NewChatPopup.xaml.cs(1 hunks)Content.Client/_CorvaxNext/NanoChat/NanoChatSystem.cs(1 hunks)Content.Server/Access/Systems/AgentIDCardSystem.cs(5 hunks)Content.Server/CartridgeLoader/Cartridges/LogProbeCartridgeComponent.cs(2 hunks)Content.Server/CartridgeLoader/Cartridges/LogProbeCartridgeSystem.cs(5 hunks)Content.Server/_CorvaxNext/CartridgeLoader/Cartridges/LogProbeCartridgeSystem.NanoChat.cs(1 hunks)Content.Server/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatCartridgeComponent.cs(1 hunks)Content.Server/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatCartridgeSystem.cs(1 hunks)Content.Server/_CorvaxNext/NanoChat/NanoChatSystem.cs(1 hunks)Content.Shared/Access/SharedAgentIDCardSystem.cs(1 hunks)Content.Shared/CartridgeLoader/Cartridges/LogProbeUiState.cs(2 hunks)Content.Shared/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatUiMessageEvent.cs(1 hunks)Content.Shared/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatUiState.cs(1 hunks)Content.Shared/_CorvaxNext/NanoChat/NanoChatCardComponent.cs(1 hunks)Content.Shared/_CorvaxNext/NanoChat/SharedNanoChatSystem.cs(1 hunks)Resources/Locale/en-US/_corvaxnext/access/components/agent-id-card-component.ftl(1 hunks)Resources/Locale/en-US/_corvaxnext/cartridge-loader/nanochat.ftl(1 hunks)Resources/Locale/en-US/_corvaxnext/nanochat/components/nanochat-card-component.ftl(1 hunks)Resources/Locale/ru-RU/_corvaxnext/access/components/agent-id-card-component.ftl(1 hunks)Resources/Locale/ru-RU/_corvaxnext/cartridge-loader/nanochat.ftl(1 hunks)Resources/Locale/ru-RU/_corvaxnext/nanochat/components/nanochat-card-component.ftl(1 hunks)Resources/Locale/ru-RU/ss14-ru/prototypes/_corvaxnext/entities/objects/devices/cartriges.ftl(1 hunks)Resources/Prototypes/Entities/Objects/Devices/pda.yml(11 hunks)Resources/Prototypes/Entities/Objects/Misc/identification_cards.yml(2 hunks)Resources/Prototypes/_Backmen/name_identifier_groups.yml(2 hunks)Resources/Prototypes/_CorvaxNext/Entities/Objects/Devices/cartridges.yml(1 hunks)Resources/Textures/Backmen/Objects/Weapons/Gunsx64/Flamethrowers/tank.rsi/meta.json(1 hunks)Resources/Textures/_CorvaxNext/Interface/VerbIcons/ATTRIBUTION.txt(1 hunks)Resources/Textures/_CorvaxNext/Misc/program_icons.rsi/meta.json(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- Resources/Textures/_CorvaxNext/Interface/VerbIcons/ATTRIBUTION.txt
- Resources/Locale/en-US/_corvaxnext/access/components/agent-id-card-component.ftl
- Resources/Textures/_CorvaxNext/Misc/program_icons.rsi/meta.json
- Resources/Locale/ru-RU/_corvaxnext/access/components/agent-id-card-component.ftl
- Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatLogEntry.xaml
⏰ Context from checks skipped due to timeout of 300000ms (2)
- GitHub Check: Test Packaging
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (24)
Resources/Locale/ru-RU/ss14-ru/prototypes/_corvaxnext/entities/objects/devices/cartriges.ftl (1)
1-2: Локализация картриджа СБ выполнена корректно!Название и описание точно отражают функциональность устройства для отслеживания разыскиваемых лиц.
Content.Client/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatLogEntry.xaml.cs (2)
1-5: Импорты выглядят корректноВсе необходимые пространства имен для работы с UI и XAML присутствуют.
7-9: Хорошая практика использования атрибутов и модификаторов
- Атрибут
GenerateTypedNameReferencesобеспечивает типобезопасный доступ к элементам XAML- Модификатор
sealedпредотвращает нежелательное наследование- Наследование от
BoxContainerлогично для компонента UIContent.Server/CartridgeLoader/Cartridges/LogProbeCartridgeComponent.cs (1)
2-2: Проверьте необходимость импорта.Убедитесь, что новое пространство имен
Content.Shared._CorvaxNext.CartridgeLoader.Cartridgesдействительно используется в этом компоненте. Префикс с подчеркиванием и суффикс "Next" в пути предполагают, что это может быть временное или экспериментальное решение.✅ Verification successful
Импорт подтверждён как необходимый.
Пространство имен
Content.Shared._CorvaxNext.CartridgeLoader.Cartridgesиспользуется для типаNanoChatDataв полеScannedNanoChatData. Префикс с подчеркиванием является частью согласованного именования для этой функциональности.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the imported namespace is used in the component rg -l "Content.Shared._CorvaxNext.CartridgeLoader.Cartridges" "Content.Server/CartridgeLoader/Cartridges/LogProbeCartridgeComponent.cs" # Check for other usages of this namespace rg "Content.Shared._CorvaxNext.CartridgeLoader.Cartridges"Length of output: 2136
Script:
#!/bin/bash # Check what types from this namespace are used in the component rg -A 5 "Content.Shared._CorvaxNext.CartridgeLoader.Cartridges" "Content.Server/CartridgeLoader/Cartridges/LogProbeCartridgeComponent.cs" # Get the full content to see type usage cat "Content.Server/CartridgeLoader/Cartridges/LogProbeCartridgeComponent.cs"Length of output: 1283
Content.Shared/_CorvaxNext/NanoChat/SharedNanoChatSystem.cs (2)
118-132: Проверьте возможность возникновения проблем с параллельным доступом к коллекциямМетод
AddMessageизменяет коллекцииMessagesиRecipients. Если к данным коллекциям может быть параллельный доступ из разных потоков или систем, существует риск возникновения условий гонки. Рекомендуется проверить и, при необходимости, добавить механизмы синхронизации для обеспечения потокобезопасности.
1-273: Код в целом выглядит хорошо и соответствует поставленным целямОбщий обзор кода не выявил критических ошибок. Структура класса логична, методы имеют понятные названия, следуют принципам хорошего проектирования и обеспечивают необходимую функциональность NanoChat.
Content.Client/CartridgeLoader/Cartridges/LogProbeUiFragment.xaml.cs (1)
20-35: Логика метода UpdateState реализована корректноОбработка данных
NanoChatDataиPulledLogsвыполнена правильно и соответствует требованиям функциональности.Resources/Locale/en-US/_corvaxnext/nanochat/components/nanochat-card-component.ftl (1)
1-5: Добавленные строки локализации выглядят корректноНовые строки локализации правильно сформированы и соответствуют форматированию файла
.ftl.Resources/Locale/ru-RU/_corvaxnext/nanochat/components/nanochat-card-component.ftl (1)
1-5: Локализация соответствует требованиям!Переводы корректны, сохраняют естественность русского языка и правильно используют плейсхолдеры.
Content.Shared/_CorvaxNext/NanoChat/NanoChatCardComponent.cs (1)
44-45:⚠️ Potential issueРеализовать ограничение частоты отправки сообщений
TODO комментарий указывает на необходимость реализации rate limiting. Это критично для предотвращения спама.
Предлагаю добавить константу для минимального интервала между сообщениями и метод проверки:
+ private const float MIN_MESSAGE_INTERVAL = 1.0f; // seconds + + public bool CanSendMessage(TimeSpan currentTime) + { + return (currentTime - LastMessageTime).TotalSeconds >= MIN_MESSAGE_INTERVAL; + }Content.Client/Access/UI/AgentIDCardBoundUserInterface.cs (2)
29-29: Корректная интеграция обработчика номера NanoChat.Реализация соответствует существующему паттерну обработки событий в классе.
Also applies to: 32-37
67-67: Проверьте обработку null значений.При установке номера через
SetCurrentNumberстоит добавить проверку на null, чтобы избежать потенциальных исключений.Content.Shared/Access/SharedAgentIDCardSystem.cs (2)
31-40: Корректное добавление поля CurrentNumber!Изменения хорошо интегрированы в существующий код. Использование опционального параметра в конструкторе обеспечивает обратную совместимость.
42-50: Правильная реализация класса сообщения!Класс
AgentIDCardNumberChangedMessageследует установленному паттерну других классов сообщений в файле.Content.Server/Access/Systems/AgentIDCardSystem.cs (1)
36-43: Добавьте валидацию номера.В методе
OnNumberChangedотсутствует проверка валидности номера перед его установкой.private void OnNumberChanged(Entity<AgentIDCardComponent> ent, ref AgentIDCardNumberChangedMessage args) { if (!TryComp<NanoChatCardComponent>(ent, out var comp)) return; + + if (args.Number == 0 || args.Number > 9999) + return; _nanoChat.SetNumber((ent, comp), args.Number); Dirty(ent, comp); }✅ Verification successful
Проверка валидации номера завершена
Предложенное изменение корректно и необходимо:
- Добавляет серверную валидацию, соответствующую клиентским ограничениям
- Предотвращает установку некорректных значений номера (0 или >9999)
- Обеспечивает согласованность с ограничением MaxNumberLength=4 в UI
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Search for related components and number validations rg -l "NanoChatCard|AgentIDCard" --type cs # Look for any number validation patterns rg "SetNumber|NumberChanged" -A 5 -B 5 --type cs # Search for any constants related to card numbers rg "const.*(number|Number)" --type csLength of output: 12953
Resources/Textures/Backmen/Objects/Weapons/Gunsx64/Flamethrowers/tank.rsi/meta.json (1)
12-15: Корректное добавление нового состояния спрайта!Новое состояние
cart-chatдобавлено с соблюдением структуры JSON.Content.Server/_CorvaxNext/CartridgeLoader/Cartridges/NanoChatCartridgeComponent.cs (3)
6-7: Корректная регистрация компонента!Компонент правильно зарегистрирован и имеет ограничение доступа через атрибут Access, что обеспечивает инкапсуляцию.
9-13: Рекомендуется добавить валидацию StationСвойство Station может быть null, что может привести к NullReferenceException при использовании. Рекомендуется добавить проверку на null в системе.
15-19: Проверить инициализацию CardСвойство Card также допускает null. Необходимо убедиться, что в системе корректно обрабатываются случаи отсутствия карты.
✅ Verification successful
Проверка пройдена успешно: корректная обработка null для Card
Анализ кода показывает, что система корректно обрабатывает случаи отсутствия карты:
- Проверки на null реализованы в критических местах
- Используются безопасные паттерны доступа к компоненту
- Валидация карты производится перед выполнением операций
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Поиск использования Card без проверки на null ast-grep --pattern 'component.Card.$$$'Length of output: 41
Script:
#!/bin/bash # Поиск файлов, содержащих NanoChatCartridgeComponent rg -l "NanoChatCartridgeComponent" # Поиск использования компонента в системах rg -A 5 "NanoChatCartridgeComponent" # Поиск прямых обращений к Card ast-grep --pattern 'Card ='Length of output: 7809
Content.Server/_CorvaxNext/CartridgeLoader/Cartridges/LogProbeCartridgeSystem.NanoChat.cs (1)
60-81: Добавьте обработку ошибокВ методе
ScanNanoChatCardрекомендуется:
- Добавить проверку входных параметров
- Обработать возможные ошибки при воспроизведении звука
- Добавить проверку успешности обновления UI
private void ScanNanoChatCard(Entity<LogProbeCartridgeComponent> ent, CartridgeAfterInteractEvent args, EntityUid target, NanoChatCardComponent card) { + if (args?.InteractEvent?.User == null) + return; + + if (card?.Recipients == null || card.Messages == null) + return; try { _audioSystem.PlayEntity(ent.Comp.SoundScan, args.InteractEvent.User, target, AudioHelpers.WithVariation(0.25f, _random)); } catch (Exception ex) { _logger.Warning($"Failed to play scan sound: {ex}"); }Content.Server/CartridgeLoader/Cartridges/LogProbeCartridgeSystem.cs (4)
6-6: Корректное добавление зависимостей!Добавление пространства имен NanoChat и изменение класса на partial логично соответствует требованиям новой функциональности.
Also applies to: 12-12
55-55: Правильная очистка данных!Очистка предыдущих данных NanoChat перед сканированием - хорошая практика для предотвращения смешивания данных.
38-45: Проверить реализацию сканирования NanoChat картыЛогика обработки NanoChat карты добавлена корректно, но необходимо проверить реализацию метода
ScanNanoChatCard. Также рекомендуется добавить комментарии к методу для улучшения документации.✅ Verification successful
Реализация сканирования NanoChat карты проверена
Метод
ScanNanoChatCardреализован корректно и включает все необходимые компоненты:
- Звуковое и визуальное оповещение пользователя
- Корректное сохранение данных карты
- Обновление состояния интерфейса
- Обработку обновлений получателей и сообщений
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for ScanNanoChatCard implementation ast-grep --pattern 'partial class LogProbeCartridgeSystem { $$$ ScanNanoChatCard($$$) { $$$ } $$$ }'Length of output: 114
Script:
#!/bin/bash # Find all files containing LogProbeCartridgeSystem echo "=== Files containing LogProbeCartridgeSystem ===" rg -l "LogProbeCartridgeSystem" echo -e "\n=== Search for ScanNanoChatCard method ===" rg "ScanNanoChatCard" -A 5 echo -e "\n=== Files related to NanoChat ===" rg -l "NanoChat"Length of output: 4323
Script:
#!/bin/bash # Get the complete implementation of ScanNanoChatCard echo "=== ScanNanoChatCard Implementation ===" cat "Content.Server/_CorvaxNext/CartridgeLoader/Cartridges/LogProbeCartridgeSystem.NanoChat.cs"Length of output: 3328
22-22: Проверить реализацию InitializeNanoChat()Метод
InitializeNanoChat()вызывается в Initialize, но его реализация не видна в текущем файле. Убедитесь, что метод корректно реализован в другой части partial класса.✅ Verification successful
Реализация InitializeNanoChat() подтверждена
Метод корректно реализован в файле LogProbeCartridgeSystem.NanoChat.cs и содержит необходимую инициализацию подписок на события NanoChat.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for InitializeNanoChat implementation ast-grep --pattern 'partial class LogProbeCartridgeSystem { $$$ InitializeNanoChat() { $$$ } $$$ }'Length of output: 113
Script:
#!/bin/bash # Search for any mention of InitializeNanoChat rg -l "InitializeNanoChat" # Search for files containing LogProbeCartridgeSystem rg -l "LogProbeCartridgeSystem" # Search for the method with any access modifier ast-grep --pattern '(private|protected|public|internal)? InitializeNanoChat'Length of output: 527
Script:
#!/bin/bash # View the content of the NanoChat implementation file cat "Content.Server/_CorvaxNext/CartridgeLoader/Cartridges/LogProbeCartridgeSystem.NanoChat.cs"Length of output: 3239
| _adminLogger.Add(LogType.Chat, | ||
| LogImpact.Low, | ||
| $"{ToPrettyString(card):user} sent NanoChat message to {recipientsText}: {msg.Content}{(deliveryFailed ? " [DELIVERY FAILED]" : "")}"); |
There was a problem hiding this comment.
Избегайте логирования содержимого пользовательских сообщений
Логирование содержимого сообщений пользователей может привести к утечке конфиденциальной информации и нарушению приватности. Рекомендуется не сохранять текст сообщений в логах или обезличивать их перед логированием.
| : $"#{message.SenderId:D4}"; | ||
|
|
||
| if (!recipient.Comp.Recipients[message.SenderId].HasUnread && !recipient.Comp.NotificationsMuted) | ||
| { |
There was a problem hiding this comment.
Проверьте наличие ключа в словаре перед обращением
При доступе к recipient.Comp.Recipients[message.SenderId] возможно исключение, если ключ message.SenderId отсутствует в словаре Recipients. Рекомендуется использовать метод TryGetValue для безопасного доступа и предотвращения возможных ошибок.
|
|
||
| namespace Content.Client._CorvaxNext.NanoChat; | ||
|
|
||
| public sealed class NanoChatSystem : SharedNanoChatSystem; |
There was a problem hiding this comment.
Синтаксическая ошибка: отсутствует тело класса NanoChatSystem
В определении класса NanoChatSystem отсутствует тело класса. В языке C# необходимо использовать фигурные скобки {} даже для пустых классов.
Примените следующий дифф для исправления ошибки:
-public sealed class NanoChatSystem : SharedNanoChatSystem;
+public sealed class NanoChatSystem : SharedNanoChatSystem
+{
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public sealed class NanoChatSystem : SharedNanoChatSystem; | |
| public sealed class NanoChatSystem : SharedNanoChatSystem | |
| { | |
| } |
| // Remove old handler if it exists | ||
| if (_pressHandler != null) | ||
| ChatButton.OnPressed -= _pressHandler; | ||
|
|
||
| _number = number; | ||
|
|
||
| // Create and store new handler | ||
| _pressHandler = _ => OnPressed?.Invoke(_number); | ||
| ChatButton.OnPressed += _pressHandler; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Улучшите управление обработчиками событий
Текущий подход к управлению обработчиками событий может быть улучшен. Рекомендуется:
- Использовать паттерн IDisposable для очистки обработчиков
- Проверять null перед созданием нового обработчика
+ private bool _disposed;
+
+ public void Dispose()
+ {
+ if (_disposed) return;
+ if (_pressHandler != null)
+ ChatButton.OnPressed -= _pressHandler;
+ _disposed = true;
+ }Committable suggestion skipped: line range outside the PR's diff.
|
|
||
| public override Control GetUIFragmentRoot() | ||
| { | ||
| return _fragment!; |
There was a problem hiding this comment.
Добавьте проверку на null
Использование оператора ! может привести к NullReferenceException. Рекомендуется добавить явную проверку:
- return _fragment!;
+ if (_fragment == null)
+ throw new InvalidOperationException("NanoChatUi не был корректно инициализирован");
+ return _fragment;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return _fragment!; | |
| if (_fragment == null) | |
| throw new InvalidOperationException("NanoChatUi не был корректно инициализирован"); | |
| return _fragment; |
| // Corvax-Next-PDAChat-Start - Copy NanoChat data if available | ||
| if (TryComp<NanoChatCardComponent>(args.Target, out var targetNanoChat) && | ||
| TryComp<NanoChatCardComponent>(uid, out var agentNanoChat)) | ||
| { | ||
| // First clear existing data | ||
| _nanoChat.Clear((uid, agentNanoChat)); | ||
|
|
||
| // Copy the number | ||
| if (_nanoChat.GetNumber((args.Target.Value, targetNanoChat)) is { } number) | ||
| _nanoChat.SetNumber((uid, agentNanoChat), number); | ||
|
|
||
| // Copy all recipients and their messages | ||
| foreach (var (recipientNumber, recipient) in _nanoChat.GetRecipients((args.Target.Value, targetNanoChat))) | ||
| { | ||
| _nanoChat.SetRecipient((uid, agentNanoChat), recipientNumber, recipient); | ||
|
|
||
| if (_nanoChat.GetMessagesForRecipient((args.Target.Value, targetNanoChat), recipientNumber) is not | ||
| { } messages) | ||
| continue; | ||
|
|
||
| foreach (var message in messages) | ||
| { | ||
| _nanoChat.AddMessage((uid, agentNanoChat), recipientNumber, message); | ||
| } | ||
| } | ||
| } | ||
| // Corvax-Next-PDAChat-End |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Оптимизируйте копирование данных чата.
Текущая реализация копирует все сообщения, что может быть неэффективно для больших чатов. Рекомендации:
- Добавьте ограничение на количество копируемых сообщений
- Добавьте обработку ошибок при копировании
- Рассмотрите возможность асинхронного копирования для больших объемов данных
| private void UpdateMessages(Dictionary<uint, List<NanoChatMessage>> messages) | ||
| { | ||
| _messages = messages; | ||
| MessageList.RemoveAllChildren(); | ||
|
|
||
| var activeChat = _pendingChat ?? _currentChat; | ||
| if (activeChat == null || !messages.TryGetValue(activeChat.Value, out var chatMessages)) | ||
| return; | ||
|
|
||
| foreach (var message in chatMessages) | ||
| { | ||
| var messageBubble = new NanoChatMessageBubble(); | ||
| messageBubble.SetMessage(message, message.SenderId == _ownNumber); | ||
| MessageList.AddChild(messageBubble); | ||
|
|
||
| // Add spacing between messages | ||
| MessageList.AddChild(new Control { MinSize = new Vector2(0, 4) }); | ||
| } | ||
|
|
||
| MessageList.InvalidateMeasure(); | ||
| MessagesScroll.InvalidateMeasure(); | ||
|
|
||
| // Scroll to bottom after messages are added | ||
| if (MessageList.Parent is ScrollContainer scroll) | ||
| scroll.SetScrollValue(new Vector2(0, float.MaxValue)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Оптимизируйте обновление списка сообщений.
В методе UpdateMessages:
- Происходит полная очистка и перестроение списка при каждом обновлении
- Отсутствует механизм дедупликации сообщений
- Возможны проблемы с производительностью при большом количестве сообщений
| private void SendMessage() | ||
| { | ||
| var activeChat = _pendingChat ?? _currentChat; | ||
| if (activeChat == null || string.IsNullOrWhiteSpace(MessageInput.Text)) | ||
| return; | ||
|
|
||
| var messageContent = MessageInput.Text; | ||
|
|
||
| // Add predicted message | ||
| var predictedMessage = new NanoChatMessage( | ||
| _timing.CurTime, | ||
| messageContent, | ||
| _ownNumber | ||
| ); | ||
|
|
||
| if (!_messages.TryGetValue(activeChat.Value, out var value)) | ||
| { | ||
| value = new List<NanoChatMessage>(); | ||
| _messages[activeChat.Value] = value; | ||
| } | ||
|
|
||
| value.Add(predictedMessage); | ||
|
|
||
| // Update UI with predicted message | ||
| UpdateMessages(_messages); | ||
|
|
||
| // Send message event | ||
| OnMessageSent?.Invoke(NanoChatUiMessageType.SendMessage, activeChat, messageContent, null); | ||
|
|
||
| // Clear input | ||
| MessageInput.Text = string.Empty; | ||
| SendButton.Disabled = true; | ||
| } |
There was a problem hiding this comment.
Улучшите обработку отправки сообщений.
В методе SendMessage:
- Отсутствует проверка длины сообщения перед отправкой
- Возможна гонка состояний при добавлении предсказанного сообщения
- Нет обработки ошибок при неудачной отправке
private void SendMessage()
{
var activeChat = _pendingChat ?? _currentChat;
- if (activeChat == null || string.IsNullOrWhiteSpace(MessageInput.Text))
+ if (activeChat == null || string.IsNullOrWhiteSpace(MessageInput.Text) ||
+ MessageInput.Text.Length > MaxMessageLength)
return;
var messageContent = MessageInput.Text;
// Add predicted message
var predictedMessage = new NanoChatMessage(
_timing.CurTime,
messageContent,
_ownNumber,
+ deliveryFailed: false
);
if (!_messages.TryGetValue(activeChat.Value, out var value))
{
value = new List<NanoChatMessage>();
_messages[activeChat.Value] = value;
}
+ try
+ {
value.Add(predictedMessage);
UpdateMessages(_messages);
OnMessageSent?.Invoke(NanoChatUiMessageType.SendMessage, activeChat, messageContent, null);
+ }
+ catch (Exception ex)
+ {
+ Logger.ErrorS("nanochat", $"Failed to send message: {ex}");
+ predictedMessage.DeliveryFailed = true;
+ UpdateMessages(_messages);
+ }
MessageInput.Text = string.Empty;
SendButton.Disabled = true;
}Committable suggestion skipped: line range outside the PR's diff.
| private void OnRecipientUpdated(ref NanoChatRecipientUpdatedEvent args) | ||
| { | ||
| var query = EntityQueryEnumerator<LogProbeCartridgeComponent, CartridgeComponent>(); | ||
| while (query.MoveNext(out var uid, out var probe, out var cartridge)) | ||
| { | ||
| if (probe.ScannedNanoChatData == null || GetEntity(probe.ScannedNanoChatData.Value.Card) != args.CardUid) | ||
| continue; | ||
|
|
||
| if (!TryComp<NanoChatCardComponent>(args.CardUid, out var card)) | ||
| continue; | ||
|
|
||
| probe.ScannedNanoChatData = new NanoChatData( | ||
| new Dictionary<uint, NanoChatRecipient>(card.Recipients), | ||
| probe.ScannedNanoChatData.Value.Messages, | ||
| card.Number, | ||
| GetNetEntity(args.CardUid)); | ||
|
|
||
| if (cartridge.LoaderUid != null) | ||
| UpdateUiState((uid, probe), cartridge.LoaderUid.Value); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Добавьте проверку на null для словарей
В методе OnRecipientUpdated отсутствует проверка card.Recipients на null. Также рекомендуется вынести повторяющуюся логику в отдельный метод.
private void OnRecipientUpdated(ref NanoChatRecipientUpdatedEvent args)
{
var query = EntityQueryEnumerator<LogProbeCartridgeComponent, CartridgeComponent>();
while (query.MoveNext(out var uid, out var probe, out var cartridge))
{
if (probe.ScannedNanoChatData == null || GetEntity(probe.ScannedNanoChatData.Value.Card) != args.CardUid)
continue;
if (!TryComp<NanoChatCardComponent>(args.CardUid, out var card))
continue;
+ if (card.Recipients == null)
+ continue;
probe.ScannedNanoChatData = new NanoChatData(
new Dictionary<uint, NanoChatRecipient>(card.Recipients),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private void OnRecipientUpdated(ref NanoChatRecipientUpdatedEvent args) | |
| { | |
| var query = EntityQueryEnumerator<LogProbeCartridgeComponent, CartridgeComponent>(); | |
| while (query.MoveNext(out var uid, out var probe, out var cartridge)) | |
| { | |
| if (probe.ScannedNanoChatData == null || GetEntity(probe.ScannedNanoChatData.Value.Card) != args.CardUid) | |
| continue; | |
| if (!TryComp<NanoChatCardComponent>(args.CardUid, out var card)) | |
| continue; | |
| probe.ScannedNanoChatData = new NanoChatData( | |
| new Dictionary<uint, NanoChatRecipient>(card.Recipients), | |
| probe.ScannedNanoChatData.Value.Messages, | |
| card.Number, | |
| GetNetEntity(args.CardUid)); | |
| if (cartridge.LoaderUid != null) | |
| UpdateUiState((uid, probe), cartridge.LoaderUid.Value); | |
| } | |
| } | |
| private void OnRecipientUpdated(ref NanoChatRecipientUpdatedEvent args) | |
| { | |
| var query = EntityQueryEnumerator<LogProbeCartridgeComponent, CartridgeComponent>(); | |
| while (query.MoveNext(out var uid, out var probe, out var cartridge)) | |
| { | |
| if (probe.ScannedNanoChatData == null || GetEntity(probe.ScannedNanoChatData.Value.Card) != args.CardUid) | |
| continue; | |
| if (!TryComp<NanoChatCardComponent>(args.CardUid, out var card)) | |
| continue; | |
| if (card.Recipients == null) | |
| continue; | |
| probe.ScannedNanoChatData = new NanoChatData( | |
| new Dictionary<uint, NanoChatRecipient>(card.Recipients), | |
| probe.ScannedNanoChatData.Value.Messages, | |
| card.Number, | |
| GetNetEntity(args.CardUid)); | |
| if (cartridge.LoaderUid != null) | |
| UpdateUiState((uid, probe), cartridge.LoaderUid.Value); | |
| } | |
| } |
| @@ -65,7 +77,7 @@ private void OnUiReady(Entity<LogProbeCartridgeComponent> ent, ref CartridgeUiRe | |||
|
|
|||
| private void UpdateUiState(Entity<LogProbeCartridgeComponent> ent, EntityUid loaderUid) | |||
| { | |||
| var state = new LogProbeUiState(ent.Comp.PulledAccessLogs); | |||
| var state = new LogProbeUiState(ent.Comp.PulledAccessLogs, ent.Comp.ScannedNanoChatData); // Corvax-Next-PDAChat - NanoChat support | |||
There was a problem hiding this comment.
💡 Codebase verification
Обнаружено несоответствие в определении UI состояния!
В серверной части добавлено свойство ScannedNanoChatData, но оно отсутствует в определении LogProbeUiState в shared-коде. Необходимо обновить класс LogProbeUiState в shared-коде, добавив соответствующее свойство.
🔗 Analysis chain
Корректное обновление UI состояния!
Добавление ScannedNanoChatData в UI состояние логично интегрировано с существующей системой. Убедитесь, что соответствующие изменения внесены в клиентский код UI.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for LogProbeUiState usage in client code
rg "LogProbeUiState.*ScannedNanoChatData"
Length of output: 252
Script:
#!/bin/bash
# Find LogProbeUiState class definition
ast-grep --pattern 'class LogProbeUiState'
# Search for any client-side implementations
rg -l "LogProbeUiState" Content.Client/
Length of output: 65784
Script:
#!/bin/bash
# Find LogProbeUiState class definition
rg -A 5 "class LogProbeUiState"
# Check LogProbeUiFragment usage of ScannedNanoChatData
rg -A 5 "ScannedNanoChatData" Content.Client/CartridgeLoader/Cartridges/LogProbeUiFragment.xaml.cs
Length of output: 687
|
завтра доделаю |
поддержка языков? |
|
Переделывай. @trest100 |
в чате ПДА? может нахуй... |
|
ready2merge |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (24)
Content.Server/Backmen/CartridgeLoader/Cartridges/NanoChatCartridgeSystem.cs (2)
351-352: Нитпик: Удалить или уточнить комментарийКомментарий на строке 351:
// I have no idea why this isn't public in the RadioSystemМожет быть воспринят как неформальный или неуместный в профессиональном коде. Рекомендуется удалить его или заменить на более информативный комментарий, объясняющий необходимость создания собственного метода
HasActiveServer.
218-259: Проверить обработку доставки сообщений и связь с серверомВ методе
HandleSendMessageвозможно улучшить обработку ошибок при доставке сообщений. Если доставка не удалась из-за отсутствия активных серверов связи, стоит информировать пользователя об этом, чтобы он понимал причину сбоя.Content.Client/Backmen/NanoChat/NanoChatSystem.cs (1)
1-5: Проверить необходимость отдельного классаNanoChatSystemна клиентеВ текущей реализации класс
NanoChatSystemна стороне клиента не содержит дополнительной логики и просто наследуется отSharedNanoChatSystem. Если нет планов по добавлению клиент-специфичной функциональности, можно рассмотреть использованиеSharedNanoChatSystemнапрямую без создания пустого наследника.Content.Server/Backmen/CartridgeLoader/Cartridges/NanoChatCartridgeComponent.cs (1)
6-26: Рекомендуется добавить валидацию состояния компонентаХотя базовая структура компонента реализована правильно, рекомендуется добавить метод для валидации состояния компонента при инициализации. Это поможет избежать потенциальных проблем с null-ссылками и некорректными значениями.
Пример реализации:
public sealed partial class NanoChatCartridgeComponent : Component { + public override void Initialize() + { + base.Initialize(); + ValidateState(); + } + + private void ValidateState() + { + if (string.IsNullOrEmpty(RadioChannel)) + RadioChannel = "Common"; + } [DataField] public EntityUid? Station;Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatLogEntry.xaml (1)
8-18: Рекомендуется улучшить адаптивность интерфейсаФиксированные значения ширины (SetWidth) для меток NumberLabel и TimeLabel могут вызвать проблемы при разных разрешениях экрана или размерах шрифта.
Предлагаемые изменения:
- SetWidth="26" + MinWidth="26" + HorizontalExpand="False" - SetWidth="100" + MinWidth="100" + HorizontalExpand="False"Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatEntry.xaml.cs (2)
11-13: Рассмотрите использование слабых событийДля предотвращения потенциальных утечек памяти в долгоживущих объектах, рекомендуется использовать
WeakEventвместо стандартного события.- public event Action<uint>? OnPressed; + public WeakEvent<uint> OnPressed = new();
20-38: Оптимизируйте управление обработчиком событийТекущая реализация создает новый обработчик при каждом вызове
SetRecipient. Рекомендуется создать единый обработчик в конструкторе.public NanoChatEntry() { RobustXamlLoader.Load(this); + _pressHandler = _ => OnPressed?.Invoke(_number); + ChatButton.OnPressed += _pressHandler; } public void SetRecipient(NanoChatRecipient recipient, uint number, bool isSelected) { - // Remove old handler if it exists - if (_pressHandler != null) - ChatButton.OnPressed -= _pressHandler; - _number = number; - - // Create and store new handler - _pressHandler = _ => OnPressed?.Invoke(_number); - ChatButton.OnPressed += _pressHandler;Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatUi.cs (1)
21-24: Используйте более лаконичный синтаксис для обработчика событийТекущая реализация может быть упрощена с использованием более современного синтаксиса C#.
- _fragment.OnMessageSent += (type, number, content, job) => - { - SendNanoChatUiMessage(type, number, content, job, userInterface); - }; + _fragment.OnMessageSent += (type, number, content, job) => + SendNanoChatUiMessage(type, number, content, job, userInterface);Content.Shared/Backmen/NanoChat/NanoChatCardComponent.cs (1)
21-27: Инициализируйте словари с начальной емкостьюДля оптимизации производительности рекомендуется указать начальную емкость словарей на основе
MaxRecipients.- public Dictionary<uint, NanoChatRecipient> Recipients = new(); - public Dictionary<uint, List<NanoChatMessage>> Messages = new(); + public Dictionary<uint, NanoChatRecipient> Recipients = new(50); + public Dictionary<uint, List<NanoChatMessage>> Messages = new(50);Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatEntry.xaml (1)
24-25: Вынесите цвета в ресурсыЖестко закодированные цвета затрудняют поддержку и изменение темы. Рекомендуется определить их в ресурсах.
- BackgroundColor="#17c622" - BorderColor="#0f7a15" /> + BackgroundColor="{StaticResource UnreadIndicatorBackground}" + BorderColor="{StaticResource UnreadIndicatorBorder}" />Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatMessageBubble.xaml (2)
17-18: Рассмотрите использование относительных значений для MaxWidthЖестко заданное значение
MaxWidth="320"может вызвать проблемы на экранах разного размера. Рекомендуется использовать относительные значения или привязки для более гибкого макета.Also applies to: 20-21
38-44: Добавьте подсказку для метки о неудачной доставкеРекомендуется добавить всплывающую подсказку (tooltip) к
DeliveryFailedLabelдля лучшего UX:<Label Name="DeliveryFailedLabel" Text="{Loc nano-chat-delivery-failed}" StyleClasses="LabelSmall" HorizontalExpand="True" HorizontalAlignment="Right" Margin="10 2 10 0" + ToolTip="{Loc nano-chat-delivery-failed-tooltip}" Visible="False" />Content.Client/Backmen/CartridgeLoader/Cartridges/NewChatPopup.xaml (1)
11-13: Улучшите доступность полей вводаРекомендуется добавить:
- Подсказки о максимальной длине ввода
- Сообщения об ошибках валидации
- Всплывающие подсказки с требованиями к вводу
<LineEdit Name="NumberInput" - PlaceHolder="{Loc nano-chat-number-placeholder}" /> + PlaceHolder="{Loc nano-chat-number-placeholder}" + ToolTip="{Loc nano-chat-number-tooltip}" + MaxLength="4" />Also applies to: 21-23, 31-33
Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatMessageBubble.xaml.cs (1)
46-47: Удалите непрофессиональный комментарийКомментарий "fuuuuuck" следует заменить на конструктивное объяснение проблемы или удалить.
- // fuuuuuck + // Temporary fix: Remove child from previous parent before adding to new container MessageBox.Parent?.RemoveChild(MessageBox);Content.Client/Backmen/CartridgeLoader/Cartridges/NewChatPopup.xaml.cs (1)
11-12: Сделайте константы конфигурируемымиЖестко закодированные значения длины следует вынести в конфигурацию:
- private const int MaxInputLength = 16; - private const int MaxNumberLength = 4; // i hardcoded it to be 4 so suffer + private static readonly IConfigurationValue<int> MaxInputLength = + ConfigurationValue.Create("nanochat.maxInputLength", 16); + private static readonly IConfigurationValue<int> MaxNumberLength = + ConfigurationValue.Create("nanochat.maxNumberLength", 4);Content.Server/Backmen/CartridgeLoader/Cartridges/LogProbeCartridgeSystem.NanoChat.cs (1)
60-81: Добавьте обработку ошибок при сканировании!Рекомендуется добавить try-catch блок для обработки потенциальных ошибок при сканировании карты и обновлении UI.
Предлагаемые изменения:
private void ScanNanoChatCard(Entity<LogProbeCartridgeComponent> ent, CartridgeAfterInteractEvent args, EntityUid target, NanoChatCardComponent card) { + try + { _audioSystem.PlayEntity(ent.Comp.SoundScan, args.InteractEvent.User, target, AudioHelpers.WithVariation(0.25f, _random)); _popupSystem.PopupCursor(Loc.GetString("log-probe-scan-nanochat", ("card", target)), args.InteractEvent.User); ent.Comp.PulledAccessLogs.Clear(); ent.Comp.ScannedNanoChatData = new NanoChatData( new Dictionary<uint, NanoChatRecipient>(card.Recipients ?? new Dictionary<uint, NanoChatRecipient>()), new Dictionary<uint, List<NanoChatMessage>>(card.Messages ?? new Dictionary<uint, List<NanoChatMessage>>()), card.Number, GetNetEntity(target) ); UpdateUiState(ent, args.Loader); + } + catch (Exception ex) + { + _popupSystem.PopupCursor(Loc.GetString("log-probe-scan-error"), args.InteractEvent.User); + Logger.Error($"Error scanning NanoChat card: {ex}"); + } }Content.Server/Backmen/NanoChat/NanoChatSystem.cs (3)
73-102: Улучшите читаемость метода ScrambleMessages!Рекомендуется разделить логику на более мелкие методы для улучшения читаемости и поддерживаемости кода.
Предлагаемые изменения:
- private void ScrambleMessages(NanoChatCardComponent component) + private void ScrambleMessages(NanoChatCardComponent component) + { + foreach (var (recipientNumber, messages) in component.Messages) + { + ScrambleMessageContent(messages); + TryReassignConversation(component, recipientNumber, messages); + } + } + + private void ScrambleMessageContent(List<NanoChatMessage> messages) + { + for (var i = 0; i < messages.Count; i++) + { + if (!_random.Prob(0.5f)) + continue; + + var message = messages[i]; + message.Content = ScrambleText(message.Content); + messages[i] = message; + } + } + + private void TryReassignConversation(NanoChatCardComponent component, uint recipientNumber, List<NanoChatMessage> messages) { - foreach (var (recipientNumber, messages) in component.Messages) - { - for (var i = 0; i < messages.Count; i++) - { - if (!_random.Prob(0.5f)) - continue; - - var message = messages[i]; - message.Content = ScrambleText(message.Content); - messages[i] = message; - } - - if (_random.Prob(0.25f) && component.Recipients.Count > 0) - { - var newRecipient = _random.Pick(component.Recipients.Keys.ToList()); - if (newRecipient == recipientNumber) - continue; - - if (!component.Messages.ContainsKey(newRecipient)) - component.Messages[newRecipient] = new List<NanoChatMessage>(); - - component.Messages[newRecipient].AddRange(messages); - component.Messages[recipientNumber].Clear(); - } - } + if (!_random.Prob(0.25f) || component.Recipients.Count <= 0) + return; + + var newRecipient = _random.Pick(component.Recipients.Keys.ToList()); + if (newRecipient == recipientNumber) + return; + + if (!component.Messages.ContainsKey(newRecipient)) + component.Messages[newRecipient] = new List<NanoChatMessage>(); + + component.Messages[newRecipient].AddRange(messages); + component.Messages[recipientNumber].Clear(); }
104-118: Добавьте документацию к алгоритму перемешивания!Рекомендуется добавить XML-документацию, объясняющую реализацию алгоритма Fisher-Yates.
Предлагаемые изменения:
+ /// <summary> + /// Перемешивает текст, используя алгоритм Fisher-Yates. + /// </summary> + /// <param name="text">Исходный текст для перемешивания</param> + /// <returns>Перемешанный текст</returns> + /// <remarks> + /// Реализует алгоритм Fisher-Yates для случайного перемешивания символов. + /// Сложность алгоритма: O(n), где n - длина текста. + /// </remarks> private string ScrambleText(string text)
120-129: Добавьте обработку ошибок при генерации номера!Рекомендуется добавить обработку ошибок при генерации уникального номера карты.
Предлагаемые изменения:
private void OnCardInit(Entity<NanoChatCardComponent> ent, ref MapInitEvent args) { if (ent.Comp.Number != null) return; + try + { // Assign a random number _name.GenerateUniqueName(ent, _nameIdentifierGroup, out var number); ent.Comp.Number = (uint)number; Dirty(ent); + } + catch (Exception ex) + { + Logger.Error($"Failed to generate unique number for NanoChat card: {ex}"); + ent.Comp.Number = 0; + } }Content.Shared/Backmen/CartridgeLoader/Cartridges/NanoChatUiMessageEvent.cs (2)
59-60: Удалите неуместный комментарий!Комментарий "putting this here because i can" не соответствует профессиональному стилю кодирования и должен быть удален.
99-137: Добавьте валидацию данных сообщения!Рекомендуется добавить проверку входных данных в конструкторе.
Предлагаемые изменения:
public NanoChatMessage(TimeSpan timestamp, string content, uint senderId, bool deliveryFailed = false) { + if (string.IsNullOrEmpty(content)) + throw new ArgumentException("Message content cannot be empty", nameof(content)); + Timestamp = timestamp; Content = content; SenderId = senderId; DeliveryFailed = deliveryFailed; }Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatUiFragment.xaml (2)
15-75: Улучшите доступность интерфейса!Рекомендуется добавить атрибуты доступности для элементов управления.
Предлагаемые изменения:
<Button Name="DeleteChatButton" MaxSize="32 32" Visible="False" StyleClasses="OpenBoth" Margin="0 0 4 0" + AccessKey="D" ToolTip="{Loc nano-chat-delete}"><Button Name="MuteButton" MaxSize="32 32" StyleClasses="OpenBoth" Margin="0 0 4 0" + AccessKey="M" ToolTip="{Loc nano-chat-toggle-mute}"><Button Name="NewChatButton" Text="+" MinSize="32 32" MaxSize="32 32" Margin="0 0 4 0" + AccessKey="N" StyleClasses="OpenBoth" ToolTip="{Loc nano-chat-new-chat}" />
136-162: Добавьте поддержку горячих клавиш!Рекомендуется добавить поддержку клавиши Enter для отправки сообщения.
Предлагаемые изменения:
<LineEdit Name="MessageInput" PlaceHolder="{Loc nano-chat-message-placeholder}" HorizontalExpand="True" + AcceptReturn="True" StyleClasses="OpenRight" />Resources/Prototypes/_Backmen/Entities/Objects/Devices/cartridges.yml (1)
44-64: Корректное определение сущности NanoChatCartridgeКонфигурация сущности реализована правильно. Рекомендации:
- Добавить метаданные о версии протокола связи
- Рассмотреть добавление дополнительных каналов связи помимо Common
- type: ActiveRadio channels: - Common + - Emergency + - Supply + metadata: + protocolVersion: "1.0"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Resources/Textures/_CorvaxNext/cartridge/cartridge.rsi/cart-chat.pngis excluded by!**/*.png
📒 Files selected for processing (23)
Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatEntry.xaml(1 hunks)Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatEntry.xaml.cs(1 hunks)Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatLogEntry.xaml(1 hunks)Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatLogEntry.xaml.cs(1 hunks)Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatMessageBubble.xaml(1 hunks)Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatMessageBubble.xaml.cs(1 hunks)Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatUi.cs(1 hunks)Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatUiFragment.xaml(1 hunks)Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatUiFragment.xaml.cs(1 hunks)Content.Client/Backmen/CartridgeLoader/Cartridges/NewChatPopup.xaml(1 hunks)Content.Client/Backmen/CartridgeLoader/Cartridges/NewChatPopup.xaml.cs(1 hunks)Content.Client/Backmen/NanoChat/NanoChatSystem.cs(1 hunks)Content.Server/Backmen/CartridgeLoader/Cartridges/LogProbeCartridgeSystem.NanoChat.cs(1 hunks)Content.Server/Backmen/CartridgeLoader/Cartridges/NanoChatCartridgeComponent.cs(1 hunks)Content.Server/Backmen/CartridgeLoader/Cartridges/NanoChatCartridgeSystem.cs(1 hunks)Content.Server/Backmen/NanoChat/NanoChatSystem.cs(1 hunks)Content.Shared/Backmen/CartridgeLoader/Cartridges/NanoChatUiMessageEvent.cs(1 hunks)Content.Shared/Backmen/CartridgeLoader/Cartridges/NanoChatUiState.cs(1 hunks)Content.Shared/Backmen/NanoChat/NanoChatCardComponent.cs(1 hunks)Content.Shared/Backmen/NanoChat/SharedNanoChatSystem.cs(1 hunks)Resources/Prototypes/_Backmen/Entities/Objects/Devices/cartridges.yml(1 hunks)Resources/Textures/Backmen/Objects/Weapons/Gunsx64/Flamethrowers/tank.rsi/meta.json(1 hunks)Resources/Textures/_CorvaxNext/cartridge/cartridge.rsi/meta.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- Resources/Textures/Backmen/Objects/Weapons/Gunsx64/Flamethrowers/tank.rsi/meta.json
- Resources/Textures/_CorvaxNext/cartridge/cartridge.rsi/meta.json
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: Test Packaging
🔇 Additional comments (12)
Content.Shared/Backmen/NanoChat/SharedNanoChatSystem.cs (1)
10-273: Отличная реализация базового классаSharedNanoChatSystemКод предоставляет широкий набор методов для управления компонентом
NanoChatCardComponent, включая работу с получателями и сообщениями. Используется корректная проверка компонентов с помощьюResolve, а также вызовыDirty(card)после изменения состояния, что обеспечивает актуальность данных. Публичный API охватывает все необходимые функции для работы NanoChat. Хорошая работа!Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatLogEntry.xaml.cs (1)
7-17: Реализация выглядит корректной!Класс реализован правильно, с использованием частичного класса и загрузки XAML. Структура компонента логична и соответствует требованиям пользовательского интерфейса.
Content.Shared/Backmen/NanoChat/NanoChatCardComponent.cs (1)
45-45: Необходимо реализовать ограничение частоты сообщенийTODO комментарий указывает на необходимость реализации ограничения частоты отправки сообщений. Это важно для предотвращения спама.
Хотите, чтобы я помог с реализацией системы ограничения частоты сообщений или создал issue для отслеживания этой задачи?
Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatEntry.xaml (1)
8-8: Пересмотрите использование фиксированных размеровЖестко заданный максимальный размер может создать проблемы при различных разрешениях экрана или масштабировании интерфейса.
Рекомендуется протестировать компонент при различных разрешениях экрана и масштабах интерфейса.
Content.Client/Backmen/CartridgeLoader/Cartridges/NewChatPopup.xaml.cs (1)
73-74: Улучшите санитизацию входных данныхТекущая обработка входных данных может быть недостаточной. Рекомендуется:
- Добавить проверку на специальные символы
- Реализовать защиту от инъекций
- Нормализовать пробельные символы
Content.Server/Backmen/CartridgeLoader/Cartridges/LogProbeCartridgeSystem.NanoChat.cs (1)
10-14: Корректная инициализация!Метод правильно подписывается на необходимые события для работы с NanoChat.
Content.Server/Backmen/NanoChat/NanoChatSystem.cs (2)
26-31: Корректная инициализация системы!Метод правильно инициализирует систему и подписывается на необходимые события.
33-71: Необходимо решить вопрос с TODO и закомментированным кодом!В методе есть несколько проблем:
- TODO комментарий о всплывающих подсказках требует внимания
- Закомментированный код для PopupEntity следует либо удалить, либо реализовать
Хотите, чтобы я помог реализовать логику всплывающих подсказок или создать issue для отслеживания этой задачи?
Content.Shared/Backmen/CartridgeLoader/Cartridges/NanoChatUiMessageEvent.cs (1)
61-97: Отличная реализация структуры данных!Структура хорошо документирована и правильно реализована.
Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatUiFragment.xaml (1)
82-103: Корректная реализация списка чатов!Реализация списка чатов с прокруткой выполнена правильно и соответствует лучшим практикам UI разработки.
Content.Client/Backmen/CartridgeLoader/Cartridges/NanoChatUiFragment.xaml.cs (2)
24-25: Рассмотрите использование ограничения размера коллекцийСловари
_recipientsи_messagesне имеют ограничений по размеру. Для предотвращения утечек памяти рекомендуется добавить максимальное количество элементов и реализовать их очистку.
116-134: Отличная реализация выбора чата!Логика выбора чата реализована правильно:
- Оптимизация для предотвращения повторного выбора
- Предиктивное обновление статуса прочтения
- Корректное обновление UI
| public readonly Dictionary<uint, NanoChatRecipient> Recipients = new(); | ||
| public readonly Dictionary<uint, List<NanoChatMessage>> Messages = new(); | ||
| public readonly uint? CurrentChat; | ||
| public readonly uint OwnNumber; | ||
| public readonly int MaxRecipients; | ||
| public readonly bool NotificationsMuted; |
There was a problem hiding this comment.
Необходимо обеспечить неизменяемость коллекций
Публичные коллекции Recipients и Messages могут быть модифицированы извне, что может привести к непредсказуемому поведению.
Рекомендуемые изменения:
- public readonly Dictionary<uint, NanoChatRecipient> Recipients = new();
- public readonly Dictionary<uint, List<NanoChatMessage>> Messages = new();
+ public IReadOnlyDictionary<uint, NanoChatRecipient> Recipients { get; }
+ public IReadOnlyDictionary<uint, IReadOnlyList<NanoChatMessage>> Messages { get; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public readonly Dictionary<uint, NanoChatRecipient> Recipients = new(); | |
| public readonly Dictionary<uint, List<NanoChatMessage>> Messages = new(); | |
| public readonly uint? CurrentChat; | |
| public readonly uint OwnNumber; | |
| public readonly int MaxRecipients; | |
| public readonly bool NotificationsMuted; | |
| public IReadOnlyDictionary<uint, NanoChatRecipient> Recipients { get; } | |
| public IReadOnlyDictionary<uint, IReadOnlyList<NanoChatMessage>> Messages { get; } | |
| public readonly uint? CurrentChat; | |
| public readonly uint OwnNumber; | |
| public readonly int MaxRecipients; | |
| public readonly bool NotificationsMuted; |
| public NanoChatUiState( | ||
| Dictionary<uint, NanoChatRecipient> recipients, | ||
| Dictionary<uint, List<NanoChatMessage>> messages, | ||
| uint? currentChat, | ||
| uint ownNumber, | ||
| int maxRecipients, | ||
| bool notificationsMuted) | ||
| { | ||
| Recipients = recipients; | ||
| Messages = messages; | ||
| CurrentChat = currentChat; | ||
| OwnNumber = ownNumber; | ||
| MaxRecipients = maxRecipients; | ||
| NotificationsMuted = notificationsMuted; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Добавить валидацию параметров в конструкторе
Отсутствует проверка входных параметров, что может привести к некорректному состоянию объекта.
Пример реализации:
public NanoChatUiState(
Dictionary<uint, NanoChatRecipient> recipients,
Dictionary<uint, List<NanoChatMessage>> messages,
uint? currentChat,
uint ownNumber,
int maxRecipients,
bool notificationsMuted)
{
+ if (maxRecipients <= 0)
+ throw new ArgumentException("MaxRecipients must be positive", nameof(maxRecipients));
+
+ if (recipients == null)
+ throw new ArgumentNullException(nameof(recipients));
+
+ if (messages == null)
+ throw new ArgumentNullException(nameof(messages));
+
Recipients = recipients;
Messages = messages;
CurrentChat = currentChat;
OwnNumber = ownNumber;
MaxRecipients = maxRecipients;
NotificationsMuted = notificationsMuted;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public NanoChatUiState( | |
| Dictionary<uint, NanoChatRecipient> recipients, | |
| Dictionary<uint, List<NanoChatMessage>> messages, | |
| uint? currentChat, | |
| uint ownNumber, | |
| int maxRecipients, | |
| bool notificationsMuted) | |
| { | |
| Recipients = recipients; | |
| Messages = messages; | |
| CurrentChat = currentChat; | |
| OwnNumber = ownNumber; | |
| MaxRecipients = maxRecipients; | |
| NotificationsMuted = notificationsMuted; | |
| } | |
| public NanoChatUiState( | |
| Dictionary<uint, NanoChatRecipient> recipients, | |
| Dictionary<uint, List<NanoChatMessage>> messages, | |
| uint? currentChat, | |
| uint ownNumber, | |
| int maxRecipients, | |
| bool notificationsMuted) | |
| { | |
| if (maxRecipients <= 0) | |
| throw new ArgumentException("MaxRecipients must be positive", nameof(maxRecipients)); | |
| if (recipients == null) | |
| throw new ArgumentNullException(nameof(recipients)); | |
| if (messages == null) | |
| throw new ArgumentNullException(nameof(messages)); | |
| Recipients = recipients; | |
| Messages = messages; | |
| CurrentChat = currentChat; | |
| OwnNumber = ownNumber; | |
| MaxRecipients = maxRecipients; | |
| NotificationsMuted = notificationsMuted; | |
| } |
|
|
||
| public override Control GetUIFragmentRoot() | ||
| { | ||
| return _fragment!; |
There was a problem hiding this comment.
Добавьте проверку на null
Использование оператора ! может привести к исключению NullReferenceException. Рекомендуется добавить явную проверку.
- return _fragment!;
+ if (_fragment == null)
+ throw new InvalidOperationException("NanoChatUi fragment not initialized");
+ return _fragment;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return _fragment!; | |
| if (_fragment == null) | |
| throw new InvalidOperationException("NanoChatUi fragment not initialized"); | |
| return _fragment; |
| public static readonly Color OwnMessageColor = Color.FromHex("#173717d9"); // Dark green | ||
| public static readonly Color OtherMessageColor = Color.FromHex("#252525d9"); // Dark gray | ||
| public static readonly Color BorderColor = Color.FromHex("#40404066"); // Subtle border | ||
| public static readonly Color TextColor = Color.FromHex("#dcdcdc"); // Slightly softened white | ||
| public static readonly Color ErrorColor = Color.FromHex("#cc3333"); // Red |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Переместите цвета в систему тем
Жестко закодированные значения цветов следует перенести в систему тем для обеспечения единообразия и поддержки различных тем оформления.
-public static readonly Color OwnMessageColor = Color.FromHex("#173717d9");
-public static readonly Color OtherMessageColor = Color.FromHex("#252525d9");
-public static readonly Color BorderColor = Color.FromHex("#40404066");
-public static readonly Color TextColor = Color.FromHex("#dcdcdc");
-public static readonly Color ErrorColor = Color.FromHex("#cc3333");
+public static readonly string ThemeVarOwnMessage = "nanochat-own-message";
+public static readonly string ThemeVarOtherMessage = "nanochat-other-message";
// ... и так далее для остальных цветовCommittable suggestion skipped: line range outside the PR's diff.
| NumberInput.OnTextChanged += _ => ValidateInputs(); | ||
| NameInput.OnTextChanged += _ => ValidateInputs(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Объедините логику валидации ввода
Дублирование кода валидации в обработчиках событий можно упростить, создав единый метод:
+ private void ValidateAndTrimInput(LineEdit input, int maxLength)
+ {
+ if (input.Text.Length > maxLength)
+ input.Text = input.Text[..maxLength];
+ ValidateInputs();
+ }Also applies to: 45-50
| private void OnRecipientUpdated(ref NanoChatRecipientUpdatedEvent args) | ||
| { | ||
| var query = EntityQueryEnumerator<LogProbeCartridgeComponent, CartridgeComponent>(); | ||
| while (query.MoveNext(out var uid, out var probe, out var cartridge)) | ||
| { | ||
| if (probe.ScannedNanoChatData == null || GetEntity(probe.ScannedNanoChatData.Value.Card) != args.CardUid) | ||
| continue; | ||
|
|
||
| if (!TryComp<NanoChatCardComponent>(args.CardUid, out var card)) | ||
| continue; | ||
|
|
||
| probe.ScannedNanoChatData = new NanoChatData( | ||
| new Dictionary<uint, NanoChatRecipient>(card.Recipients), | ||
| probe.ScannedNanoChatData.Value.Messages, | ||
| card.Number, | ||
| GetNetEntity(args.CardUid)); | ||
|
|
||
| if (cartridge.LoaderUid != null) | ||
| UpdateUiState((uid, probe), cartridge.LoaderUid.Value); | ||
| } | ||
| } |
There was a problem hiding this comment.
Добавьте проверку на null для Recipients!
В строке 28 создается новый словарь из card.Recipients без проверки на null. Если Recipients окажется null, это приведет к NullReferenceException.
Примените следующие изменения:
- probe.ScannedNanoChatData = new NanoChatData(
- new Dictionary<uint, NanoChatRecipient>(card.Recipients),
- probe.ScannedNanoChatData.Value.Messages,
- card.Number,
- GetNetEntity(args.CardUid));
+ probe.ScannedNanoChatData = new NanoChatData(
+ new Dictionary<uint, NanoChatRecipient>(card.Recipients ?? new Dictionary<uint, NanoChatRecipient>()),
+ probe.ScannedNanoChatData.Value.Messages,
+ card.Number,
+ GetNetEntity(args.CardUid));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private void OnRecipientUpdated(ref NanoChatRecipientUpdatedEvent args) | |
| { | |
| var query = EntityQueryEnumerator<LogProbeCartridgeComponent, CartridgeComponent>(); | |
| while (query.MoveNext(out var uid, out var probe, out var cartridge)) | |
| { | |
| if (probe.ScannedNanoChatData == null || GetEntity(probe.ScannedNanoChatData.Value.Card) != args.CardUid) | |
| continue; | |
| if (!TryComp<NanoChatCardComponent>(args.CardUid, out var card)) | |
| continue; | |
| probe.ScannedNanoChatData = new NanoChatData( | |
| new Dictionary<uint, NanoChatRecipient>(card.Recipients), | |
| probe.ScannedNanoChatData.Value.Messages, | |
| card.Number, | |
| GetNetEntity(args.CardUid)); | |
| if (cartridge.LoaderUid != null) | |
| UpdateUiState((uid, probe), cartridge.LoaderUid.Value); | |
| } | |
| } | |
| private void OnRecipientUpdated(ref NanoChatRecipientUpdatedEvent args) | |
| { | |
| var query = EntityQueryEnumerator<LogProbeCartridgeComponent, CartridgeComponent>(); | |
| while (query.MoveNext(out var uid, out var probe, out var cartridge)) | |
| { | |
| if (probe.ScannedNanoChatData == null || GetEntity(probe.ScannedNanoChatData.Value.Card) != args.CardUid) | |
| continue; | |
| if (!TryComp<NanoChatCardComponent>(args.CardUid, out var card)) | |
| continue; | |
| probe.ScannedNanoChatData = new NanoChatData( | |
| new Dictionary<uint, NanoChatRecipient>(card.Recipients ?? new Dictionary<uint, NanoChatRecipient>()), | |
| probe.ScannedNanoChatData.Value.Messages, | |
| card.Number, | |
| GetNetEntity(args.CardUid)); | |
| if (cartridge.LoaderUid != null) | |
| UpdateUiState((uid, probe), cartridge.LoaderUid.Value); | |
| } | |
| } |
| private void OnMessageReceived(ref NanoChatMessageReceivedEvent args) | ||
| { | ||
| var query = EntityQueryEnumerator<LogProbeCartridgeComponent, CartridgeComponent>(); | ||
| while (query.MoveNext(out var uid, out var probe, out var cartridge)) | ||
| { | ||
| if (probe.ScannedNanoChatData == null || GetEntity(probe.ScannedNanoChatData.Value.Card) != args.CardUid) | ||
| continue; | ||
|
|
||
| if (!TryComp<NanoChatCardComponent>(args.CardUid, out var card)) | ||
| continue; | ||
|
|
||
| probe.ScannedNanoChatData = new NanoChatData( | ||
| probe.ScannedNanoChatData.Value.Recipients, | ||
| new Dictionary<uint, List<NanoChatMessage>>(card.Messages), | ||
| card.Number, | ||
| GetNetEntity(args.CardUid)); | ||
|
|
||
| if (cartridge.LoaderUid != null) | ||
| UpdateUiState((uid, probe), cartridge.LoaderUid.Value); | ||
| } | ||
| } |
There was a problem hiding this comment.
Добавьте проверку на null для Messages!
Аналогичная проблема с потенциальным NullReferenceException при создании словаря из card.Messages.
Примените следующие изменения:
- probe.ScannedNanoChatData = new NanoChatData(
- probe.ScannedNanoChatData.Value.Recipients,
- new Dictionary<uint, List<NanoChatMessage>>(card.Messages),
- card.Number,
- GetNetEntity(args.CardUid));
+ probe.ScannedNanoChatData = new NanoChatData(
+ probe.ScannedNanoChatData.Value.Recipients,
+ new Dictionary<uint, List<NanoChatMessage>>(card.Messages ?? new Dictionary<uint, List<NanoChatMessage>>()),
+ card.Number,
+ GetNetEntity(args.CardUid));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private void OnMessageReceived(ref NanoChatMessageReceivedEvent args) | |
| { | |
| var query = EntityQueryEnumerator<LogProbeCartridgeComponent, CartridgeComponent>(); | |
| while (query.MoveNext(out var uid, out var probe, out var cartridge)) | |
| { | |
| if (probe.ScannedNanoChatData == null || GetEntity(probe.ScannedNanoChatData.Value.Card) != args.CardUid) | |
| continue; | |
| if (!TryComp<NanoChatCardComponent>(args.CardUid, out var card)) | |
| continue; | |
| probe.ScannedNanoChatData = new NanoChatData( | |
| probe.ScannedNanoChatData.Value.Recipients, | |
| new Dictionary<uint, List<NanoChatMessage>>(card.Messages), | |
| card.Number, | |
| GetNetEntity(args.CardUid)); | |
| if (cartridge.LoaderUid != null) | |
| UpdateUiState((uid, probe), cartridge.LoaderUid.Value); | |
| } | |
| } | |
| private void OnMessageReceived(ref NanoChatMessageReceivedEvent args) | |
| { | |
| var query = EntityQueryEnumerator<LogProbeCartridgeComponent, CartridgeComponent>(); | |
| while (query.MoveNext(out var uid, out var probe, out var cartridge)) | |
| { | |
| if (probe.ScannedNanoChatData == null || GetEntity(probe.ScannedNanoChatData.Value.Card) != args.CardUid) | |
| continue; | |
| if (!TryComp<NanoChatCardComponent>(args.CardUid, out var card)) | |
| continue; | |
| probe.ScannedNanoChatData = new NanoChatData( | |
| probe.ScannedNanoChatData.Value.Recipients, | |
| new Dictionary<uint, List<NanoChatMessage>>(card.Messages ?? new Dictionary<uint, List<NanoChatMessage>>()), | |
| card.Number, | |
| GetNetEntity(args.CardUid)); | |
| if (cartridge.LoaderUid != null) | |
| UpdateUiState((uid, probe), cartridge.LoaderUid.Value); | |
| } | |
| } |
| private void UpdateMessages(Dictionary<uint, List<NanoChatMessage>> messages) | ||
| { | ||
| _messages = messages; | ||
| MessageList.RemoveAllChildren(); | ||
|
|
||
| var activeChat = _pendingChat ?? _currentChat; | ||
| if (activeChat == null || !messages.TryGetValue(activeChat.Value, out var chatMessages)) | ||
| return; | ||
|
|
||
| foreach (var message in chatMessages) | ||
| { | ||
| var messageBubble = new NanoChatMessageBubble(); | ||
| messageBubble.SetMessage(message, message.SenderId == _ownNumber); | ||
| MessageList.AddChild(messageBubble); | ||
|
|
||
| // Add spacing between messages | ||
| MessageList.AddChild(new Control { MinSize = new Vector2(0, 4) }); | ||
| } | ||
|
|
||
| MessageList.InvalidateMeasure(); | ||
| MessagesScroll.InvalidateMeasure(); | ||
|
|
||
| // Scroll to bottom after messages are added | ||
| if (MessageList.Parent is ScrollContainer scroll) | ||
| scroll.SetScrollValue(new Vector2(0, float.MaxValue)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Оптимизируйте обновление списка сообщений
Метод UpdateMessages выполняет полную очистку и перестроение списка сообщений при каждом обновлении. Рекомендуется:
- Использовать виртуализацию списка для больших чатов
- Реализовать инкрементальные обновления
- Добавить обработку ошибок UI операций
| private void SendMessage() | ||
| { | ||
| var activeChat = _pendingChat ?? _currentChat; | ||
| if (activeChat == null || string.IsNullOrWhiteSpace(MessageInput.Text)) | ||
| return; | ||
|
|
||
| var messageContent = MessageInput.Text; | ||
|
|
||
| // Add predicted message | ||
| var predictedMessage = new NanoChatMessage( | ||
| _timing.CurTime, | ||
| messageContent, | ||
| _ownNumber | ||
| ); | ||
|
|
||
| if (!_messages.TryGetValue(activeChat.Value, out var value)) | ||
| { | ||
| value = new List<NanoChatMessage>(); | ||
| _messages[activeChat.Value] = value; | ||
| } | ||
|
|
||
| value.Add(predictedMessage); | ||
|
|
||
| // Update UI with predicted message | ||
| UpdateMessages(_messages); | ||
|
|
||
| // Send message event | ||
| OnMessageSent?.Invoke(NanoChatUiMessageType.SendMessage, activeChat, messageContent, null); | ||
|
|
||
| // Clear input | ||
| MessageInput.Text = string.Empty; | ||
| SendButton.Disabled = true; | ||
| } |
There was a problem hiding this comment.
Добавьте валидацию входных данных и ограничение частоты отправки
В методе SendMessage отсутствует:
- Санитизация входных данных для предотвращения XSS
- Ограничение частоты отправки сообщений (rate limiting)
- Проверка на спам-символы
Предлагаемые изменения:
private void SendMessage()
{
var activeChat = _pendingChat ?? _currentChat;
if (activeChat == null || string.IsNullOrWhiteSpace(MessageInput.Text))
return;
+ // Проверка на спам-символы
+ if (ContainsSpamCharacters(MessageInput.Text))
+ return;
+
+ // Rate limiting
+ if (!CanSendMessage())
+ return;
+
var messageContent = MessageInput.Text;
+ messageContent = SanitizeInput(messageContent);Committable suggestion skipped: line range outside the PR's diff.
|
@trest100 фикси тесты давай |
|
Наночат на корвакс-нексте — хуйня. Он работает как контакты на телефоне, я бы сделал что бы можно было выбирать по манифесту, кому написать. Так же наночат вроде привязон не к id-карте, а к игроку (но это не точно). |
Описание PR
портирован наночат.
Оригинальный ПР: space-syndicate/space-station-14-next#153
Медиа
Тип PR
Изменения
🆑 trest100
Summary by CodeRabbit
Заметки о выпуске
Новые возможности
Улучшения интерфейса
Дополнительно