-
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
[Fix] Laying System SSD standing up #949
base: master
Are you sure you want to change the base?
Conversation
WalkthroughВ данном пулл-запросе внесены изменения в классы 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 (
|
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: 0
🧹 Outside diff range and nitpick comments (6)
Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs (1)
333-333
: Убедитесь, что скрытие действия вставания является ожидаемым поведениемУстановка
Hidden = true
скрывает индикатор прогресса действия от игрока. Это может привести к тому, что игроки не будут знать, что их персонаж пытается встать. Рассмотрите возможность оставить индикатор видимым для лучшего пользовательского опыта.Content.Shared/Mobs/Systems/MobStateSystem.cs (1)
Line range hint
1-100
: Рекомендации по архитектуре системыСистема корректно реализует базовую логику состояний мобов, но для решения проблемы с SSD персонажами рекомендуется:
- Добавить явную проверку SSD состояния в методы изменения состояния
- Рассмотреть добавление событий для синхронизации состояний между клиентом и сервером
- Документировать сетевое взаимодействие в комментариях к классу
Content.Shared/Mobs/Systems/MobStateSystem.StateMachine.cs (1)
Line range hint
89-108
: Рекомендуется добавить документацию для проверки DebrainedComponentЛогика проверки компонента
DebrainedComponent
не документирована. Предлагаю добавить комментарий, объясняющий почему состояние не должно меняться для debrained сущностей.Предлагаемое изменение:
if (oldState == newState || !component.AllowedStates.Contains(newState)) return; + // Prevent state changes for debrained entities to maintain death state if (oldState == MobState.Dead && HasComp<DebrainedComponent>(target)) return;
Content.Shared/Mobs/Systems/MobStateSystem.Subscribers.cs (3)
Line range hint
74-91
: Критическая проблема: Необоснованный вызов Stand при выходе из состоянийТекущая реализация может вызывать проблему с SSD персонажами, пытающимися встать. Вызов
_standing.Stand(target)
при выходе из состояний Critical и Dead может быть причиной нежелательного поведения.Предлагаемые изменения:
case MobState.Critical: - _standing.Stand(target); break; case MobState.Dead: RemComp<CollisionWakeComponent>(target); - _standing.Stand(target); break;
Line range hint
93-124
: Рекомендация по улучшению обработки состоянийТекущая реализация корректна, но можно улучшить надёжность:
- Добавить валидацию входных параметров
- Логировать переходы состояний для отладки
Предлагаемые улучшения:
private void OnStateEnteredSubscribers(EntityUid target, MobStateComponent component, MobState state) { + if (!Exists(target) || !TryComp<MobStateComponent>(target, out _)) + return; + if (_timing.ApplyingState) return; + + _sawmill.Debug($"Entity {ToPrettyString(target)} entering state {state}");
Line range hint
128-149
: Добавить документацию для специальной обработки речиЛогика обработки
AllowNextCritSpeechComponent
требует пояснения для будущего сопровождения кода.Предлагаемые изменения:
+ /// <summary> + /// Проверяет возможность речи с учетом специального компонента AllowNextCritSpeechComponent, + /// который позволяет произнести одну фразу в критическом состоянии. + /// </summary> private void OnSpeakAttempt(EntityUid uid, MobStateComponent component, SpeakAttemptEvent args) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs
(1 hunks)Content.Shared/Mobs/Systems/MobStateSystem.StateMachine.cs
(1 hunks)Content.Shared/Mobs/Systems/MobStateSystem.Subscribers.cs
(1 hunks)Content.Shared/Mobs/Systems/MobStateSystem.cs
(2 hunks)
🔇 Additional comments (7)
Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs (1)
332-332
:
Проверьте необходимость установки RequireCanInteract = false
Установка RequireCanInteract = false
позволяет персонажу начать вставать, даже если он не может взаимодействовать, например, находится под действием оглушения или паралича. Это может привести к тому, что недееспособные персонажи смогут встать, что может быть нежелательным поведением.
Content.Shared/Mobs/Systems/MobStateSystem.cs (2)
5-5
: Добавление зависимости для работы с сетью
Добавление Robust.Shared.Network
логично для синхронизации состояний между клиентом и сервером.
20-20
: Проверьте использование сетевого менеджера
Добавление INetManager
предполагает изменения в сетевой логике, но текущий файл не показывает его использование.
Выполните следующий скрипт для поиска использования сетевого менеджера:
Content.Shared/Mobs/Systems/MobStateSystem.StateMachine.cs (3)
Line range hint 24-33
: Отличное добавление параметра origin!
Добавление параметра origin
позволяет отслеживать источник изменения состояния моба, что улучшает отладку и управление состояниями. Это особенно полезно для решения проблемы с SSD персонажами.
Line range hint 115-119
: Хорошо структурированное событие!
Структура UpdateMobStateEvent
правильно использует атрибут ByRefEvent
для оптимизации производительности и включает все необходимые параметры для отслеживания изменений состояния.
Line range hint 1-119
: Необходимо проверить обработку SSD состояний
Давайте проверим, что изменения корректно обрабатывают взаимодействие между системами SSD и MobState.
✅ Verification successful
Проверка обработки состояний мертвых персонажей без мозга подтверждена
Анализ кода показывает, что:
- В
MobStateSystem
корректно реализована проверкаDebrainedComponent
при смене состояния - Система
DebrainedSystem
блокирует попытки встать (StandAttemptEvent
) для персонажей без мозга - При удалении мозга автоматически добавляется
DebrainedComponent
иDelayedDeathComponent
- Состояние смерти (
MobState.Dead
) в комбинации сDebrainedComponent
не позволяет изменить состояние персонажа
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Поиск связанных обработчиков SSD состояний
echo "Поиск обработчиков SSD состояний:"
rg -l "SSD.*MobState|MobState.*SSD"
# Поиск использования UpdateMobStateEvent
echo "Поиск использования UpdateMobStateEvent:"
ast-grep --pattern 'UpdateMobStateEvent'
# Поиск связанных тестов
echo "Поиск тестов:"
fd -e cs -e yaml test.*mob.*state -o test.*ssd
Length of output: 521
Script:
#!/bin/bash
# Поиск файла с SSD системой
echo "=== Поиск SSD системы ==="
rg -l "class.*SSDSystem"
# Поиск использования DebrainedComponent
echo "=== Проверка использования DebrainedComponent ==="
rg "DebrainedComponent" -A 5
# Поиск связи между SSD и MobState
echo "=== Поиск связей SSD и MobState ==="
rg "SSD.*State|State.*SSD" -A 5
# Поиск тестов для MobState
echo "=== Поиск тестов MobState ==="
fd "MobState.*Test.cs"
Length of output: 7367
Content.Shared/Mobs/Systems/MobStateSystem.Subscribers.cs (1)
Line range hint 47-53
: Проверить необходимость дополнительной проверки состояния
Текущая реализация может содержать избыточную проверку, так как система взаимодействия уже должна блокировать действия недееспособных сущностей.
✅ Verification successful
Based on the search results, I can now generate the final response. The results show that:
- There are similar TODO comments questioning interaction checks in other systems
- The interaction system has general blockers that should handle incapacitated states
- The ActionBlockerSystem is responsible for handling general interaction restrictions
Here's the final response:
Проверка состояния необходима для безопасности
Хотя система взаимодействия (SharedInteractionSystem
) действительно обрабатывает общие проверки, дополнительная проверка состояния в OnUnbuckleAttempt
является обоснованной мерой безопасности:
- Отстёгивание (unbuckling) - это специфическое действие, которое может иметь серьёзные последствия для недееспособных сущностей
- Подобные проверки присутствуют в других системах (например, в
SleepingSystem
) - Удаление этой проверки может привести к непредвиденным последствиям в игровом процессе
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Поиск других проверок взаимодействия в системе
ast-grep --pattern 'IsIncapacitated($entity)'
rg -A 3 "interaction.*check"
Length of output: 7217
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: 0
🧹 Outside diff range and nitpick comments (2)
Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs (2)
334-336
: Правильная реализация сокрытия анимации для SSD персонажей!Установка параметра Hidden на основе проверки наличия разума - это элегантное решение проблемы. Однако стоит рассмотреть добавление комментария для объяснения логики.
Предлагаю добавить поясняющий комментарий:
+ // Hide the standing up animation for SSD (mindless) entities Hidden = !_mindSystem.TryGetMind(uid, out EntityUid _, out MindComponent? _)
Line range hint
108-124
: Рекомендуется добавить дополнительные проверки состоянияТекущая реализация корректна, но можно усилить безопасность, добавив дополнительные проверки.
Предлагаю следующие изменения:
if (args.NewMobState == MobState.Alive) { + // Prevent standing up attempts if the entity is in an invalid state + if (HasComp<KnockedDownComponent>(ent) || _pulling.IsPulled(ent)) + return; + AutoGetUp(ent); TryStandUp(ent, ent, standingStateComponent); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs
(3 hunks)
🔇 Additional comments (2)
Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs (2)
59-59
: Корректное добавление зависимости от SharedMindSystem!
Добавление зависимости от SharedMindSystem необходимо для определения состояния разума сущности, что является ключевым компонентом для решения проблемы с SSD.
Line range hint 1-367
: Необходимо проверить все сценарии SSD состояния
Для полной уверенности в исправлении, рекомендуется провести тестирование следующих сценариев:
- SSD состояние при смерти
- SSD состояние при критическом уроне
- Переход в SSD во время попытки встать
- Выход из SSD во время лежачего состояния
✅ Verification successful
Let me analyze the code further to verify the SSD state handling.
Проверка обработки SSD состояния выполнена успешно
Код корректно обрабатывает SSD состояние через проверку наличия сессии игрока (TryGetSessionByEntity
). Если сессия отсутствует (SSD), то поведение определяется реализацией GetAutoGetUp
в серверной части.
Основные проверяемые сценарии:
- При SSD состоянии:
TryGetSessionByEntity
вернёт false - При активной сессии: решение о подъёме принимается через
GetAutoGetUp
- Дополнительная проверка
IsSafeStanUp
гарантирует безопасность действия - Контейнер проверяется через
IsEntityInContainer
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any other potential standing-related issues with SSD entities
rg -l "TryStandUp|StandingState|MobState" | xargs rg "Mind|SSD|Consciousness"
Length of output: 86347
Script:
#!/bin/bash
# Check for SSD-related functionality in LayingDownSystem
ast-grep --pattern $'TryProcessAutoGetUp(Entity<LayingDownComponent> ent) {
$$$
var autoUp = !_playerManager.TryGetSessionByEntity(ent, out var player) ||
GetAutoGetUp(ent, session: player);
$$$
}'
Length of output: 219
Script:
#!/bin/bash
# Check for SSD and standing state interactions
rg -A 5 -B 5 "TryGetSessionByEntity|IsSSD|SSDIndicator" Content.Shared.Backmen.Standing/SharedLayingDownSystem.cs Content.Server.Backmen.Standing/LayingDownSystem.cs
Length of output: 357
Script:
#!/bin/bash
# Search for LayingDown system files and their SSD handling
fd -e cs LayingDown | xargs rg -A 5 -B 5 "TryGetSessionByEntity|IsSSD|SSDIndicator"
Length of output: 1044
RequireCanInteract = false | ||
}; | ||
RequireCanInteract = false, | ||
Hidden = !_mindSystem.TryGetMind(uid, out EntityUid _, out MindComponent? _) |
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.
ты так всем НПС убьёшь автовставание
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.
HasComp<ActorComponent>
RequireCanInteract = false | ||
}; | ||
RequireCanInteract = false, | ||
Hidden = !_mindSystem.TryGetMind(uid, out EntityUid _, out MindComponent? _) |
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.
HasComp<ActorComponent>
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Описание PR
Исправил, что постоянное
DoAfter
, когда ССД персонажи всех рас (кроме людей почему-то) пытаются постоянно встать на клиентской стороне будучи в критическом состоянии/мёртвыми.Медиа
Тип PR
Изменения
🆑
Summary by CodeRabbit
Новые функции
Исправления ошибок
Документация