[феатуре] ms paint #279
[феатуре] ms paint #279
Conversation
|
Warning Rate limit exceeded@RedFoxIV has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 29 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughВ пул-реквесте внесены изменения в систему креонов и обработку событий рук. В клиентских и серверных компонентах креона обновлена логика событий, подписки и обработки UI, удалены устаревшие переменные и методы, а также добавлены новые методы для обработки взаимодействий. В компонентах рук добавлены новые подписки и сетевые события для передачи состояния деселекции. В общей части кода изменена структура компонента креона (новые поля, изменение сигнатуры конструктора) и добавлена функциональность для управления углом через колесико мыши. Также обновлён YAML-прототип с увеличением ёмкости. Changes
Suggested labels
Suggested reviewers
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. 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
Content.Client/Hands/Systems/HandsSystem.cs (1)
77-87: Качественная реализация обработчика сетевого события деселекции.Метод OnHandDeselectedCrutchWrap правильно преобразует полученное сетевое событие в локальное событие HandDeselectedEvent. Комментарий хорошо объясняет проблему и решение: код выбрасывания не выполняется на клиенте, поэтому HandDeselectedEvent не вызывается на клиентской стороне.
Возможно, стоит рассмотреть более элегантное решение в будущем, но текущий подход работоспособен и эффективен.
Content.Shared/Crayon/SharedCrayonSystem.cs (3)
47-56: Проверка валидности указателя на прототип может быть расширена.Сейчас при неудачном поиске
DecalPrototypeили отсутствии нужногоTagпросто выполняетсяreturn. Возможно, следует добавить сообщение пользователю или лог при ошибке, чтобы упростить отладку.
57-66: Добавить более строгую валидацию цвета.Вы проверяете, что цвет не совпадает с текущим и что
SelectableColorустановлен вtrue, однако, если предполагается ограничение доступных цветов, стоит убедиться, что выбираемый цвет допустим и корректно обработан.
104-115: Пересмотреть сброс угла при выборе/снятии с руки.Сейчас угол при
OnCrayonHandSelectedиOnCrayonHandDeselectedсбрасывается в 0. Если нужен сохранённый угол между переключениями предметов, возможно, стоит хранить и восстанавливать прежнее значение.Content.Shared/Crayon/CrayonComponent.cs (2)
30-32: Рассмотрите нормализацию значения угла.Если предполагается беспрерывная прокрутка колеса, угол может выйти за ожидаемый диапазон. Можно нормализовать угол (например, через
Angle.Normalized) или использовать более жёсткие ограничения.
57-61: ПолеUIUpdateNeededможет быть выпилено или перенесено на клиентскую сторону.Так как по комментариям это надо только на клиенте, вместо хранения в
Shared-компоненте можно рассмотреть вариант отсеять такое поле из общего состояния.Content.Client/_White/Overlays/CrayonPreviewOverlay.cs (2)
46-62: Убедитесь в корректной обработке изменений состояния мелкаМетод отрисовки правильно обрабатывает изменение выбранного состояния мелка, но при каждом обновлении кадра выполняется проверка изменения состояния. Это не является критичным для производительности, но стоит рассмотреть возможность обновления текстуры только при получении события изменения состояния.
73-76: Подавление предупреждения компилятораОтключение предупреждения RA0002 может быть необходимым для данного случая, но желательно документировать причину подавления предупреждения для облегчения будущего сопровождения.
-#pragma warning disable RA0002 // ffs +#pragma warning disable RA0002 // Необходимо для правильного рендеринга текстуры с вращением и прозрачностью
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
Content.Client/Crayon/CrayonComponent.cs(1 hunks)Content.Client/Crayon/CrayonSystem.cs(2 hunks)Content.Client/Hands/Systems/HandsSystem.cs(2 hunks)Content.Client/_White/Overlays/CrayonPreviewOverlay.cs(1 hunks)Content.Server/Crayon/CrayonComponent.cs(2 hunks)Content.Server/Crayon/CrayonSystem.cs(5 hunks)Content.Server/Hands/Systems/HandsSystem.cs(2 hunks)Content.Shared/Crayon/CrayonComponent.cs(3 hunks)Content.Shared/Crayon/SharedCrayonSystem.cs(1 hunks)Content.Shared/_White/Hands/HandDeselectedNetworkCrutchWrap.cs(1 hunks)Resources/Prototypes/Entities/Objects/Fun/crayons.yml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Content.Server/Crayon/CrayonComponent.cs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: YAML Linter
- GitHub Check: Test Packaging
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (24)
Content.Shared/_White/Hands/HandDeselectedNetworkCrutchWrap.cs (2)
1-7: Корректное определение необходимых пространств имен.Импорты включают все необходимые зависимости для работы с сериализацией и сетевой передачей данных.
10-15: Хорошая реализация сетевого события для синхронизации деселекции руки.Класс корректно наследуется от EntityEventArgs и правильно помечен атрибутами [Serializable] и [NetSerializable]. Использование первичного конструктора делает код лаконичным. Поля Target и User хранят необходимую информацию для правильной обработки события деселекции на клиенте.
Content.Server/Hands/Systems/HandsSystem.cs (2)
30-31: Корректное добавление необходимых пространств имен.Импорты Robust.Shared.Serialization и Content.Shared._White.Hands необходимы для работы с новым сетевым событием HandDeselectedNetworkCrutchWrap.
293-296: Правильная реализация распространения события деселекции руки на клиент.Добавлен механизм отправки сетевого события для синхронизации состояния деселекции руки между сервером и клиентом при выбрасывании предмета. Комментарий хорошо объясняет причину необходимости этого изменения - исходное событие HandDeselectedEvent не передавалось на клиент, поскольку код выбрасывания выполняется только на сервере.
Content.Client/Hands/Systems/HandsSystem.cs (2)
31-31: Добавлен импорт для работы с сетевым событием.Корректно добавлено пространство имен Content.Shared._White.Hands для обработки сетевого события HandDeselectedNetworkCrutchWrap.
72-72: Корректная подписка на сетевое событие.Добавлена подписка на событие HandDeselectedNetworkCrutchWrap, что позволяет клиенту получать уведомления о деселекции руки, которые происходят на сервере.
Content.Shared/Crayon/SharedCrayonSystem.cs (3)
26-27: Отличная реализация абстрактного системного класса.Код соответствует шаблону ECS, и объявление
abstract class SharedCrayonSystemвыглядит корректно.
33-45: Правильное связывание событий и команд ввода.Подписки и бинды для событий (HandSelectedEvent, HandDeselectedEvent и т.д.) и привязки мышиного колеса выглядят грамотно реализованными.
67-71: Убедитесь, что интерфейс действительно закрывается только для соответствующего пользователя.В текущем виде происходят вызовы
_uiSystem.CloseUi(...)без явной проверки, чтоargs.User- это тот, кто открыл меню. В большинстве случаев это корректно, но иногда может потребоваться дополнительная логика для много-пользовательских сценариев.Content.Shared/Crayon/CrayonComponent.cs (1)
12-13: Лаконичное и последовательное сетевое объявление компонента.Использование
[RegisterComponent, NetworkedComponent, AutoGenerateComponentState(true)]выглядит целесообразно и упрощает синхронизацию состояния.Resources/Prototypes/Entities/Objects/Fun/crayons.yml (1)
24-24: Проверьте согласованность значенияcapacity.Значение увеличено до 50, однако в файле
CrayonComponent.csпо умолчаниюCapacity = 30. Убедитесь, что это не создаёт противоречий при инициализации таких объектов.Content.Client/Crayon/CrayonComponent.cs (1)
5-16: Архитектурное изменение, обеспечивающее улучшенное разделение компонентовКомментирование клиентского
CrayonComponentи перенос его в общий компонент является хорошим архитектурным решением, устраняющим дублирование кода и улучшающим поддержку состояния между клиентом и сервером.Content.Client/_White/Overlays/CrayonPreviewOverlay.cs (3)
15-44: Хорошая реализация и инициализация предпросмотра мелковКласс предпросмотра мелков правильно инициализирует зависимости и корректно устанавливает начальное состояние объекта. Использование инъекции зависимостей соответствует принятым в кодовой базе практикам.
63-72: Корректная реализация позиционирования и прозрачностиПравильный расчет позиции мыши и корректировка прозрачности в зависимости от расстояния до игрока. Это обеспечивает хорошую обратную связь с пользователем.
65-65: Новая функциональность вращения рисунковДобавление угла поворота (Angle) в отрисовку предпросмотра хорошо реализовано. Корректно учитывается текущее вращение камеры с помощью
_eye.CurrentEye.Rotation.Content.Server/Crayon/CrayonSystem.cs (4)
74-82: Успешная реализация вращения рисунков мелкомДобавление функции поворота рисунков мелком хорошо реализовано. Код корректно получает целевое относительное вращение из
InputMoverComponentпользователя и применяет его к декали.
36-41: Аккуратное управление подписками на событияПеренос подписок на события в общий класс логично соответствует архитектурным изменениям, связанным с переносом компонента в shared-часть. Комментарии четко объясняют причины этих изменений.
44-49: Переход на систему AutoStateКомментирование метода
OnCrayonGetStateи переход на систему AutoState является хорошим архитектурным решением, сокращающим объем повторяющегося кода для управления состоянием компонента.
81-81:❓ Verification inconclusive
Проверьте корректность смещения позиции декали
Сдвиг позиции на
(-0.5f, -0.5f)необходим для правильного центрирования декали, однако стоит убедиться, что это смещение соответствует размеру текстуры и ожидаемому положению.
🏁 Script executed:
#!/bin/bash # Проверка использования смещения в других методах добавления декалей echo "Проверка использования смещения (-0.5f, -0.5f) в декалях:" rg -A 2 "TryAddDecal.*Offset\(" --glob "*.cs"Length of output: 508
Проверьте корректность смещения (-0.5f, -0.5f) для декалей
Проведён поиск по всему кодовому основанию, и удалось установить, что фиксированное смещение (-0.5f, -0.5f) используется только в файлеContent.Server/Crayon/CrayonSystem.cs. Несмотря на то, что данный сдвиг применяется для центрирования декали, необходимо убедиться, что он соответствует половине размеров используемой текстуры и обеспечивает ожидаемое позиционирование. Если в будущем текстура изменится или появятся другие алгоритмы центрирования, рекомендуется параметризовать или рассчитать смещение динамически.Content.Client/Crayon/CrayonSystem.cs (5)
37-43: Корректная инициализация обработчиков событийДобавление новых подписок на события для обработки состояния мелка реализовано правильно. Система теперь должным образом реагирует на взаимодействия, удаление компонента и завершение сущности.
73-78: Корректное управление визуальным отображением при выборе мелкаОбработка событий выбора мелка (
OnCrayonHandSelected) реализована правильно. Дополнительное удаление оверлея перед добавлением нового - хорошая практика для предотвращения дублирования.
80-84: Корректное удаление визуального отображения при отмене выбора мелкаОбработка события отмены выбора мелка (
OnCrayonHandDeselected) реализована корректно и удаляет предпросмотр, что предотвращает появление "призрачных" предпросмотров.
68-71:❓ Verification inconclusive
Потенциальная несогласованность в управлении состоянием UI
В методе
CrayonAfterAutoStateустанавливается флагUIUpdateNeeded = true, что может быть несовместимо с архитектурными изменениями, учитывая, чтоUIUpdateNeededбыло удалено изCrayonComponentкласса в Client. Убедитесь, что это поле объявлено в общем компоненте.
🏁 Script executed:
#!/bin/bash # Проверка наличия поля UIUpdateNeeded в Shared компоненте echo "Проверка объявления UIUpdateNeeded в Shared компоненте:" rg "UIUpdateNeeded.*{.*get.*set" --glob "*Shared*Crayon*"Length of output: 307
Обратите внимание: Проверьте корректность объявления поля UIUpdateNeeded в общем (Shared) компоненте
В методе
CrayonAfterAutoState(Content.Client/Crayon/CrayonSystem.cs, строки 68-71) устанавливается флагUIUpdateNeeded = true. Согласно архитектурным изменениям, это поле было удалено из клиентской версииCrayonComponent. Необходимо удостовериться, что полеUIUpdateNeededобъявлено в общем (Shared) компоненте, чтобы избежать проблемы несовместимости.
- Проверьте, что в файлах, относящихся к общей логике (Shared), присутствует объявление поля
UIUpdateNeeded.- Если поле отсутствует, рассмотрите возможность его добавления или изменение логики обновления UI в методе
CrayonAfterAutoState.Обратите внимание на первоначальный скрипт проверки:
rg "UIUpdateNeeded.*{.*get.*set" --glob "*Shared*Crayon*"Он завершился с ошибкой из-за некорректного регулярного выражения. Пожалуйста, используйте исправленный запрос, экранирующий фигурную скобку, например:
rg "UIUpdateNeeded.*\\{.*get.*set" --glob "*Shared*Crayon*"
46-50:❓ Verification inconclusive
Упрощенная логика закрытия UI
Закрытие UI, когда количество зарядов меньше или равно 1, помогает улучшить опыт пользователя, но необходимо проверить, что это соответствует бизнес-логике. Возможно, UI должен закрываться только когда заряды полностью исчерпаны (равны 0).
🏁 Script executed:
#!/bin/bash # Проверка логики закрытия UI в других местах echo "Проверка логики закрытия UI для мелков:" rg -A 5 "CloseUi.*CrayonUiKey" --glob "*.cs"Length of output: 1120
Обратите внимание: Проверьте соответствие бизнес-логики
В методе
CrayonAfterInteract(Content.Client/Crayon/CrayonSystem.cs, строки 46–50) UI закрывается при условииif (comp.Charges <= 1). Аналогичный вызов закрытия UI обнаружен вContent.Shared/Crayon/SharedCrayonSystem.cs. Это свидетельствует о согласованном использовании данной проверки во всех местах.Если по требованиям бизнес-логики UI должен закрываться только при полном исчерпании зарядов (то есть, когда
comp.Charges == 0), то условие следует пересмотреть и скорректировать. В противном случае, если закрытие при одном оставшемся заряде соответствует улучшению пользовательского опыта, логику можно оставить без изменений.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
Content.Client/_White/Overlays/CrayonPreviewOverlay.cs (5)
15-18: Рекомендация по модификаторуsealed
Если класс в перспективе может расширяться другими наследниками, стоит рассмотреть удалениеsealed.
19-28: Рекомендация: использоватьreadonlyтам, где это возможно
Если значения_crayonCompи_crayonUidне меняются в рантайме, имеет смысл объявить их какreadonlyдля большей надёжности.
32-44: Добавить логирование для отсутствующегоDecalPrototype
При неудачеTryIndex, стоит логировать предупреждение, чтобы понимать причину прозрачной текстуры при отладке.if (_proto.TryIndex<DecalPrototype>(_currentState, out var proto)) { _tex = _sprite.Frame0(proto.Sprite); } else { + Logger.Warning($"DecalPrototype {_currentState} не найден. Использую прозрачную текстуру."); _tex = Texture.Transparent; }
52-61: Устранить дублирование кода при смене текстуры
Аналогичный блок кода сTryIndex<DecalPrototype>есть и в конструкторе. Можно вынести в отдельный метод для упрощения поддержки.private void UpdateTexture(string state) { if (_proto.TryIndex<DecalPrototype>(state, out var proto)) _tex = _sprite.Frame0(proto.Sprite); else _tex = Texture.Transparent; } // И использовать: if (_currentState != _crayonComp.SelectedState) { _currentState = _crayonComp.SelectedState; UpdateTexture(_currentState); }
63-72: Улучшить визуальную индикацию выхода за радиус
Можно реализовать плавное уменьшение прозрачности вместо резкого снижения до 0.1 при удалении от игрока.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Content.Client/_White/Overlays/CrayonPreviewOverlay.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: YAML map schema validator
- GitHub Check: build (ubuntu-latest)
- GitHub Check: YAML Linter
- GitHub Check: Test Packaging
🔇 Additional comments (4)
Content.Client/_White/Overlays/CrayonPreviewOverlay.cs (4)
1-14: Подключаемые модули и пространство имён выглядят корректно
Здесь нет очевидных проблем или конфликтов.
29-31: Инициализация полей
_spriteуже помечено какreadonly, что соответствует хорошей практике. Без замечаний.
46-51: Проверка валидности сущностей
Условие выхода при несуществующем креоне или неверномplayerUidвыглядит разумно и предотвращает потенциальные ошибки.
73-77: Проверка оправданности отключения предупреждения
Использование#pragma warning disable RA0002желательно подкрепить пониманием отключаемой диагностики, чтобы впоследствии избежать ошибок, которые она могла предотвратить.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
Content.Client/_White/Overlays/CrayonPreviewOverlay.cs (3)
20-25: Убедитесь в необходимости прямых зависимостей.
Внедрение нескольких сервисов напрямую (IInputManager,IPlayerManager,IEyeManager,IEntityManager,IPrototypeManager) может усложнить поддержку кода, если потребности в них будут меняться. Рассмотрите вариант инкапсуляции подобных зависимостей в отдельный сервис или класс-«фасад», чтобы упростить тестирование и расширяемость.
26-32: Сгруппируйте поля инициализации или упростите их настройку.
Переменные_crayonComp,_crayonUid,_currentStateи_texтесно связаны между собой. Рассмотрите возможность инициализировать их в едином месте (например, в конструкторе), чтобы упростить понимание кода и избежать рассинхронизации.
71-74: Используйте константы или настройки для значений прозрачности.
Текущее решение жёстко задаётalpha = 0.6fи в противном случаеalpha = 0.1f, что может потребовать ручного пересмотра при изменении игровых условий. Рассмотрите вынесение этих значений в константы, конфигурацию или параметры, чтобы упростить балансировку игры.Content.Shared/Crayon/SharedCrayonSystem.cs (3)
47-65: Проверка валидности состоиния и цвета креона.Методы
OnCrayonBoundUIиOnCrayonBoundUIColorкорректно валидируют входные данные (например, проверка, что выбранный decal прототип имеет тег "crayon"). Убедитесь, что для невалидного цвета выбрасываются нужные уведомления, если этого требует логика.
67-71: Закрытие интерфейса при сбросе креона.Метод
OnCrayonDroppedграмотно закрывает UI. Учтите, что существует TODO-комментарий о переиспользовании общего события — возможно, стоит дополнительно уточнить сроки его реализации, чтобы избежать дублирования кода.
106-116: Сброс угла при выборе/снятии креона в руке.В
OnCrayonHandSelectedиOnCrayonHandDeselectedугол сбрасывается в ноль. Это может быть нежелательным, если игрок ожидает, что угол сохранится между действиями. Рекомендуется уточнить требования UX и, при необходимости, сохранять угол.Content.Server/Crayon/CrayonSystem.cs (3)
30-30: Новая зависимость:SharedTransformSystem.Проверьте, действительно ли требуется
SharedTransformSystemв этом файле. Если это задел на будущее, стоит добавить краткий комментарий, чтобы избежать путаницы в коде.
73-84: Добавлена поддержка вращения при рисовании.Получение
TargetRelativeRotationизInputMoverComponentи учётcomponent.Angleпри добавлении decal. Это даёт гибкость пользователю задавать наклон рисунка. Проверьте, не возникнет ли конфликтов в случаях, когда пользователь уже повернул креон через колесо мыши.
112-119: Тоггл окна интерфейса креона.Метод
OnCrayonUseоткрывает или закрывает UI, а также устанавливает начальное состояние. Логика актуальна, однако стоит подтвердить, что при повторном вызове не произойдёт конфликт между синхронизацией состояния на клиенте и сервере.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Content.Client/_White/Overlays/CrayonPreviewOverlay.cs(1 hunks)Content.Server/Crayon/CrayonSystem.cs(5 hunks)Content.Shared/Crayon/SharedCrayonSystem.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Test Packaging
- GitHub Check: YAML Linter
- GitHub Check: YAML map schema validator
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (12)
Content.Client/_White/Overlays/CrayonPreviewOverlay.cs (2)
33-45: Проверьте механизм выбора текстуры.
Код повторяет логикуTryIndex<DecalPrototype>и при неудаче подгружаетTexture.Transparent. Если предполагается, что состояние иногда не будет найдено, то текущее поведение корректно. Однако если это нежелательная ситуация, имеет смысл логгировать предупреждение, чтобы упростить диагностику ошибок.
65-68: Уточните смысл вычисляемого угла.
Текущая реализация вычитает_eye.CurrentEye.Rotationиз_crayonComp.Angle. Если цель — повернуть текстуру относительно направления камеры, код корректен. Однако убедитесь, что это поведение согласуется с остальным игровым процессом (например, другие инструменты могут использовать иной подход).Content.Shared/Crayon/SharedCrayonSystem.cs (6)
1-21: Импорты выглядят упорядоченно.Дублирующихся пространств имён не замечено, подключенные зависимости кажутся релевантными для работы системы.
25-28: Добавление абстрактного системного класса.Создание абстрактного класса
SharedCrayonSystemрасширяет возможности модуля для работы с креоном. При этом структура оставляет пространство для переопределения логики в серверной и клиентской частях.
29-32: Проверить использование всех зависимостей.Поля
_hands,_prototypeManagerи_uiSystemкорректно внедрены через [Dependency]. Убедитесь, что все зависимости действительно используются и не вызывают дополнительных накладных расходов.
33-45: Инициализация систем и привязка команд.Метод
Initialize()связывает колесо мыши с методами вращения и подписывается на локальные события. Для долгоживущих систем обычно не требуется отписка от этих событий, однако при необходимости переинициализации стоит добавить соответствующий код в методShutdown().
73-88: Вращение креона при прокрутке колеса мыши (увеличение угла).Логика в
MouseWheelUpвыглядит корректно: проверяется, что игрок держит предмет, и что это действительноCrayonComponent. Возможно, стоит убедиться, что возвращаемое значение (false) верно, и в дальнейшем не нужно прерывать обработку события.
90-104: Вращение креона при прокрутке колеса мыши (уменьшение угла).Метод
MouseWheelDownаналогичен предыдущему, но уменьшает угол. При необходимости запрета дальнейшей обработки события можно возвращать true — убедитесь, что текущая логика (return false) удовлетворяет вашей концепции управления.Content.Server/Crayon/CrayonSystem.cs (4)
12-12: Добавлен импорт для движения.Подключение
using Content.Shared.Movement.Components;указывает на необходимость работы с компонентомInputMoverComponent. Убедитесь, что логика движения тесно связана с системой креона и не вынесена в ненужную зависимость.
36-37: Перенос подписки на события в общую систему.Комментарий указывает, что обработка
CrayonSelectMessageиCrayonColorMessageперенесена вSharedCrayonSystem. Убедитесь, что не остались лишние ссылки на эти события в локальном контексте.
40-41: Подписки на события сброшены.Удалены подписки на
DroppedEventиComponentGetState— согласно комментариям, их заменили механизмы вSharedCrayonSystemи AutoState. Убедитесь, что новая реализация покрывает все кейсы, которые обрабатывались раньше.
103-103: Отправка сообщения интерфейсу о использовании креона.Строка
_uiSystem.ServerSendUiMessageинформирует UI о том, что креон был использован, передавая состояниеCrayonUsedMessage. Убедитесь, что это не приводит к дублированию сообщений при массовом использовании креона.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Content.Client/_White/Overlays/CrayonPreviewOverlay.cs (2)
33-45: Избежать дублирования кода при загрузке спрайта
Код для полученияDecalPrototypeи установки_texповторяется в конструкторе и в блоке проверки_currentState != _crayonComp.SelectedState. Рекомендуется вынести эту логику в отдельный метод, чтобы повысить удобочитаемость и облегчить внесение изменений.Пример возможного рефакторинга:
+private void UpdateTextureFromState(string newState) +{ + if (_proto.TryIndex<DecalPrototype>(newState, out var proto)) + _tex = _sprite.Frame0(proto.Sprite); + else + _tex = Texture.Transparent; +} public CrayonPreviewOverlay(SpriteSystem sprite, CrayonComponent comp) { ... - if (_proto.TryIndex<DecalPrototype>(_currentState, out var proto)) - _tex = _sprite.Frame0(proto.Sprite); - else - _tex = Texture.Transparent; + UpdateTextureFromState(_currentState); } protected override void Draw(in OverlayDrawArgs args) { ... - if (_proto.TryIndex<DecalPrototype>(_crayonComp.SelectedState, out var proto)) - _tex = _sprite.Frame0(proto.Sprite); - else - _tex = Texture.Transparent; + UpdateTextureFromState(_crayonComp.SelectedState); }Also applies to: 56-63
76-78: Избавиться от лишней директивы подавления предупреждения
Директива#pragma warning disable RA0002может скрывать потенциально важные замечания анализатора. Проверьте, можно ли устранить первопричину предупреждения (например, с помощью корректировки аргументов или другим способом), чтобы избежать необходимости в явном подавлении.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Content.Client/_White/Overlays/CrayonPreviewOverlay.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: YAML Linter
- GitHub Check: YAML map schema validator
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Test Packaging
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (2)
Content.Client/_White/Overlays/CrayonPreviewOverlay.cs (2)
18-18: Убедиться, что отрисовка под сущностями действительно оправдана
Текущее значениеOverlaySpace.WorldSpaceBelowEntitiesозначает, что оверлей будет отображаться под объектами. Проверьте, совпадает ли это с желаемым визуальным эффектом, поскольку элементы интерфейса могут быть визуально перекрыты другими спрайтами.
49-52: Уточнить логику прерывания отрисовки
В случае, если объект креона не существует или игрок невалиден, оверлей просто завершается. При этом, если игрок держит компонентHoldingDropComponent, отрисовка тоже отключается. Убедитесь, что это согласуется со сценарием использования, и в таких случаях оверлей действительно должен скрываться.
DVOniksWyvern
merged commit efca951
into
WWhiteDreamProject:master
Описание PR
Даёт возможность поворачивать рисунки мелками.
Медиа
12X2pLMozL.mp4
Изменения
🆑