-
Notifications
You must be signed in to change notification settings - Fork 150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Port] Surgery fix PartAppearance and other minor fixes #945
Conversation
WalkthroughВ данном пулл-реквесте внесены изменения в несколько файлов, касающихся системы управления внешним видом частей тела, компонента пилы для костей, системы обработки частей тела и конфигурации клавиш управления. Основные изменения включают улучшение логики обработки внешнего вида частей тела, добавление начального значения для свойства скорости в компоненте пилы, оптимизацию процесса гиблирования частей тела и обновление конфигурации клавиш для управления камерой. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Camera
User->>Camera: CameraRotateLeft (with Control)
User->>Camera: CameraRotateRight (with Control)
User->>Camera: ZoomIn (with Control)
User->>Camera: ZoomOut (with Control)
User->>Camera: ResetZoom (with Control)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
Content.Shared/Backmen/Surgery/Tools/BoneSawComponent.cs (1)
11-11
: Хорошее исправление, но нужна документацияУстановка значения по умолчанию
1f
- это правильное решение для предотвращения фатальной ошибки. Однако рекомендуется добавить комментарий, объясняющий выбор этого значения для будущего сопровождения кода.Предлагаемые изменения:
[DataField] - public float Speed { get; set; } = 1f; + /// <summary> + /// Базовая скорость работы пилы. Значение 1 установлено как нейтральный множитель скорости. + /// </summary> + public float Speed { get; set; } = 1f;Content.Shared/Backmen/Surgery/Body/SharedBodySystem.PartAppearance.cs (1)
149-159
: Улучшите обработку ошибок!Текущие изменения улучшают надёжность кода, но рекомендуется добавить логирование для отладки:
if (TerminatingOrDeleted(uid) || TerminatingOrDeleted(args.Part) || !TryComp(uid, out HumanoidAppearanceComponent? bodyAppearance)) +{ + Logger.Debug($"OnPartDroppedFromBody: Пропуск обработки для {uid}/{args.Part} (Terminating/Deleted/NoAppearance)"); return; +} if (!HasComp<BodyPartAppearanceComponent>(args.Part)) +{ + Logger.Debug($"OnPartDroppedFromBody: Добавление отсутствующего BodyPartAppearanceComponent для {args.Part}"); EnsureComp<BodyPartAppearanceComponent>(args.Part); +}Content.Shared/Body/Systems/SharedBodySystem.Body.cs (1)
Line range hint
399-412
: Логика удаления частей тела улучшена!Порядок операций теперь корректный - сначала удаляются дочерние части, затем обрабатываются органы. Это предотвращает потенциальные проблемы с null-ссылками.
Предлагаю добавить проверку на null для organ.Id перед вызовом TryGibEntityWithRef для большей надежности.
foreach (var organ in GetPartOrgans(partId, part)) { + if (!organ.Id.IsValid()) + continue; _gibbingSystem.TryGibEntityWithRef(bodyEnt, organ.Id, GibType.Drop, GibContentsOption.Skip, ref gibs, playAudio: false, launchImpulse: GibletLaunchImpulse * splatModifier, launchImpulseVariance: GibletLaunchImpulseVariance, launchCone: splatCone); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
Content.Shared/Backmen/Surgery/Body/SharedBodySystem.PartAppearance.cs
(4 hunks)Content.Shared/Backmen/Surgery/Tools/BoneSawComponent.cs
(1 hunks)Content.Shared/Body/Systems/SharedBodySystem.Body.cs
(2 hunks)Resources/keybinds.yml
(1 hunks)
🔇 Additional comments (4)
Content.Shared/Backmen/Surgery/Body/SharedBodySystem.PartAppearance.cs (3)
31-31
: Улучшение читаемости кода!
Добавление пустой строки после проверки улучшает читаемость и соответствует общепринятым практикам форматирования кода.
38-41
: Правильная инициализация компонента!
Установка базовых значений ID и Type даже при отсутствии оригинального тела - это хорошая практика защитного программирования, которая предотвращает возможные NullReferenceException в дальнейшем.
180-180
: Проверьте влияние отключения синхронизации!
Изменение параметра синхронизации в AddMarking
может повлиять на сетевое поведение. Убедитесь, что это не приведет к рассинхронизации между клиентом и сервером.
✅ Verification successful
Отключение синхронизации в AddMarking является корректным
Анализ кодовой базы показывает, что параметр sync: false
используется в большинстве вызовов AddMarking
. Это стандартное поведение, которое можно увидеть в:
SharedHumanoidAppearanceSystem.cs
(множественные вызовы)- Текущая реализация согласуется с общим паттерном использования
Изменение является безопасным и соответствует существующей практике в кодовой базе.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Поиск других вызовов AddMarking для сравнения параметров синхронизации
rg "AddMarking.*sync:" -A 2
Length of output: 29
Script:
#!/bin/bash
# Search for AddMarking method calls with broader pattern
rg "AddMarking\(" -A 2
# Find AddMarking method definition to understand its parameters
ast-grep --pattern 'AddMarking($$$) { $$$ }'
Length of output: 3405
Resources/keybinds.yml (1)
73-73
: Добавление модификатора Control для управления камерой
Изменения корректно разрешают конфликт горячих клавиш путём добавления модификатора Control к функциям управления камерой.
Давайте проверим отсутствие конфликтов с системой прицеливания:
Also applies to: 77-77, 81-81, 85-85, 89-89
✅ Verification successful
Let me gather more context about how these NumPad keys are used.
Подтверждение разрешения конфликта горячих клавиш
Изменения успешно разрешают конфликт между системой управления камерой и системой прицеливания:
- Все функции управления камерой теперь требуют модификатор Control
- Система прицеливания использует те же клавиши NumPad, но без модификаторов
- Конфликты отсутствуют, так как комбинации клавиш теперь различаются
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Проверка конфликтов между управлением камерой и системой прицеливания
# Поиск всех привязок клавиш для NumPad
rg "NumpadNum[1-9]" Resources/keybinds.yml
Length of output: 260
Script:
#!/bin/bash
# Get full context for each NumPad key binding to understand their functions
rg -A 3 "key: NumpadNum[1-9]" Resources/keybinds.yml
Length of output: 811
foreach (var item in _inventory.GetHandOrInventoryEntities(partId)) | ||
{ | ||
SharedTransform.AttachToGridOrMap(item); | ||
gibs.Add(item); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Необходима обработка ошибок при прикреплении предметов
Текущая реализация не обрабатывает потенциальные ошибки при прикреплении предметов к сетке или карте. В случае сбоя предметы могут быть потеряны.
foreach (var item in _inventory.GetHandOrInventoryEntities(partId))
{
- SharedTransform.AttachToGridOrMap(item);
+ try
+ {
+ SharedTransform.AttachToGridOrMap(item);
+ gibs.Add(item);
+ }
+ catch (Exception e)
+ {
+ Log.Error($"Failed to attach inventory item {ToPrettyString(item)} during gibbing: {e}");
+ SharedTransform.DropNextTo(item, partId);
+ }
- gibs.Add(item);
}
📝 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.
foreach (var item in _inventory.GetHandOrInventoryEntities(partId)) | |
{ | |
SharedTransform.AttachToGridOrMap(item); | |
gibs.Add(item); | |
} | |
foreach (var item in _inventory.GetHandOrInventoryEntities(partId)) | |
{ | |
try | |
{ | |
SharedTransform.AttachToGridOrMap(item); | |
gibs.Add(item); | |
} | |
catch (Exception e) | |
{ | |
Log.Error($"Failed to attach inventory item {ToPrettyString(item)} during gibbing: {e}"); | |
SharedTransform.DropNextTo(item, partId); | |
} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coderabbitai Нет, ошибки хендлятся не так в ыы14, ты просто позволяешь ошибке случиться, и она уже отлавливается дебаггером/опускается сама, если не имеет значения. Короче это не бекенд
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TokenStyle Спасибо за пояснение. Понимаю, что в вашем проекте обработка ошибок осуществляется иначе.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вроде бы работает, одобряю
Описание PR
Исправил отображение конечностей
SharedBodySystem.PartAppearance.cs
и кусков тела с Marking'ами (если имеются), т.е теперь всё отображается. ИсправилBoneSawComponent.cs
опечатку, добавив базовую скорость по умолчанию, т.к без неё фатальная ошибка. Исправил Control для прокрутки камеры, т.к если два одинаковых горячих клавиши, то одно из действий проигрываться не будет.Медиа
Тип PR
Изменения
🆑
Summary by CodeRabbit
Новые функции
Исправления ошибок