-
Notifications
You must be signed in to change notification settings - Fork 154
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] фиксы синхронизации системы лежания #936
Conversation
ПроцессВнесены изменения в классы Изменения
Стихотворение
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (3)Content.Client/Backmen/Standing/LayingDownSystem.cs (3)
Добавление подписки на событие
Добавление дополнительного условия для проверки текущего значения
Требуются улучшения в обработке состояний! Предыдущие замечания по коду всё ещё актуальны:
Дополнительные замечания:
Предлагаемое решение: + /// <summary>
+ /// Углы поворота для состояния лежания
+ /// </summary>
+ private const float LAYING_DOWN_ANGLE = 270f;
+ private const float ALTERNATE_LAYING_ANGLE = 90f;
private void OnChangeStanding(Entity<StandingStateComponent> ent, ref AfterAutoHandleStateEvent args)
{
- if(_animation.HasRunningAnimation(ent, "rotate"))
+ if(_animation?.HasRunningAnimation(ent, "rotate") ?? false)
return;
+ if (Deleted(ent))
+ return;
if (!TryComp<SpriteComponent>(ent, out var sprite))
{
return;
}
if (ent.Comp.Standing)
{
sprite.Rotation = Angle.Zero;
return;
}
- if (sprite.Rotation != Angle.FromDegrees(270) && sprite.Rotation != Angle.FromDegrees(90))
+ if (sprite.Rotation != Angle.FromDegrees(LAYING_DOWN_ANGLE) &&
+ sprite.Rotation != Angle.FromDegrees(ALTERNATE_LAYING_ANGLE))
{
- sprite.Rotation = Angle.FromDegrees(270);
+ sprite.Rotation = Angle.FromDegrees(LAYING_DOWN_ANGLE);
}
} 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 (2)
Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs (2)
200-203
: Оптимизация обработки сетевых событийПроверка
IsFirstTimePredicted
предотвращает дублирование сетевых событий при синхронизации состояния между клиентом и сервером. Это важное изменение для корректной работы системы предсказания.Убедитесь, что все остальные методы, вызывающие
RaisePredictiveEvent
, также проверяютIsFirstTimePredicted
для консистентности поведения.
Line range hint
1-365
: Общие рекомендации по улучшению кодаВ целом изменения корректны и улучшают синхронизацию, но есть несколько моментов для рассмотрения:
- Рекомендуется добавить комментарии, объясняющие логику предсказания событий
- Стоит рассмотреть добавление логирования для отладки проблем с синхронизацией
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
Content.Client/Backmen/Standing/LayingDownSystem.cs
(2 hunks)Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs
(4 hunks)Content.Shared/Standing/StandingStateComponent.cs
(1 hunks)
🔇 Additional comments (7)
Content.Shared/Standing/StandingStateComponent.cs (2)
6-6
: Правильное использование AutoGenerateComponentState!
Явное указание параметра true
для автоматической генерации состояния улучшит синхронизацию между сервером и клиентом.
14-14
: Проверьте корректность синхронизации состояний
Добавление ViewVariables
с доступом на чтение и запись позволит отлаживать состояния в реальном времени. Рекомендуется проверить:
- Корректную синхронизацию между сервером и клиентом
- Правильное обновление состояния при переходах между Standing/Lying
✅ Verification successful
Based on the search results, I can see comprehensive evidence of proper synchronization between client and server for the StandingStateComponent:
- The component is properly networked with AutoNetworkedField
- Client-side LayingDownSystem subscribes to state changes via AfterAutoHandleStateEvent
- Server and shared systems properly handle state transitions
- Integration tests verify the state transitions
I can now provide the final response:
Синхронизация состояний реализована корректно
Проверка кодовой базы показала надёжную реализацию синхронизации:
- Компонент корректно сетевой (AutoNetworkedField)
- На клиенте есть обработчик изменений состояния (LayingDownSystem)
- Имеются интеграционные тесты, проверяющие переходы состояний
- ViewVariables не нарушает существующую логику синхронизации
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Проверка обработки состояний в связанных системах
# Поиск обработчиков событий изменения состояния
rg -A 5 "StandingStateComponent" --type cs
# Поиск связанных систем
ast-grep --pattern 'class $_ : ISystem {
$$$
StandingStateComponent
$$$
}'
Length of output: 12870
Content.Client/Backmen/Standing/LayingDownSystem.cs (3)
25-25
: Корректное добавление зависимости!
Зависимость SharedAppearanceSystem
добавлена в соответствии с существующим паттерном инъекции зависимостей.
33-33
: Подписка на событие добавлена корректно!
Подписка на AfterAutoHandleStateEvent
для компонента StandingStateComponent
правильно связана с обработчиком OnChangeStanding
.
71-74
: Улучшена логика проверки глубины отрисовки!
Добавленное условие корректно проверяет текущую глубину отрисовки перед её изменением, что предотвращает ненужные обновления состояния.
Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs (2)
57-57
: Добавлена зависимость IGameTiming для улучшения синхронизации
Добавление IGameTiming
позволяет корректно обрабатывать предсказание событий между клиентом и сервером.
339-339
: Улучшение синхронизации состояния
Добавление вызова Dirty
после изменения CurrentState
гарантирует, что изменение состояния будет корректно синхронизировано между клиентом и сервером.
Давайте проверим, что состояние корректно синхронизируется во всех случаях:
✅ Verification successful
Состояние корректно синхронизируется во всех случаях изменения
По результатам проверки кодовой базы видно, что после каждого изменения CurrentState
в классах, связанных с системой Standing/LayingDown, всегда вызывается метод Dirty
для синхронизации:
- В
SharedLayingDownSystem.cs
после всех измененийstandingState.CurrentState
вызываетсяDirty(uid, standingState)
- В
StandingStateSystem.cs
также соблюдается этот паттерн - В
StandingStateComponent.cs
изменение происходит через свойство, которое правильно обрабатывается системой
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Проверяем все места, где изменяется CurrentState
# Поиск всех мест изменения CurrentState
ast-grep --pattern 'standingState.CurrentState = $_'
# Поиск вызовов Dirty после изменения состояния
rg -A 1 'CurrentState = '
Length of output: 5071
private void OnChangeStanding(Entity<StandingStateComponent> ent, ref AfterAutoHandleStateEvent args) | ||
{ | ||
if(_animation.HasRunningAnimation(ent, "rotate")) | ||
return; | ||
|
||
if (!TryComp<SpriteComponent>(ent, out var sprite)) | ||
{ | ||
return; | ||
} | ||
|
||
if (ent.Comp.Standing) | ||
{ | ||
sprite.Rotation = Angle.Zero; | ||
return; | ||
} | ||
|
||
if (sprite.Rotation != Angle.FromDegrees(270) && sprite.Rotation != Angle.FromDegrees(90)) | ||
{ | ||
sprite.Rotation = Angle.FromDegrees(270); | ||
} | ||
} |
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
Необходимы улучшения в обработке поворота спрайта!
Текущая реализация имеет несколько потенциальных улучшений:
- Отсутствует проверка на null для системы анимации
- Магические числа (270, 90) лучше вынести в константы
- Логика поворота может быть упрощена
Предлагаемые изменения:
+ private const float LAYING_DOWN_ANGLE = 270f;
+ private const float ALTERNATE_LAYING_ANGLE = 90f;
private void OnChangeStanding(Entity<StandingStateComponent> ent, ref AfterAutoHandleStateEvent args)
{
- if(_animation.HasRunningAnimation(ent, "rotate"))
+ if(_animation?.HasRunningAnimation(ent, "rotate") ?? false)
return;
if (!TryComp<SpriteComponent>(ent, out var sprite))
{
return;
}
if (ent.Comp.Standing)
{
sprite.Rotation = Angle.Zero;
return;
}
- if (sprite.Rotation != Angle.FromDegrees(270) && sprite.Rotation != Angle.FromDegrees(90))
+ if (sprite.Rotation != Angle.FromDegrees(LAYING_DOWN_ANGLE) &&
+ sprite.Rotation != Angle.FromDegrees(ALTERNATE_LAYING_ANGLE))
{
- sprite.Rotation = Angle.FromDegrees(270);
+ sprite.Rotation = Angle.FromDegrees(LAYING_DOWN_ANGLE);
}
}
📝 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 OnChangeStanding(Entity<StandingStateComponent> ent, ref AfterAutoHandleStateEvent args) | |
{ | |
if(_animation.HasRunningAnimation(ent, "rotate")) | |
return; | |
if (!TryComp<SpriteComponent>(ent, out var sprite)) | |
{ | |
return; | |
} | |
if (ent.Comp.Standing) | |
{ | |
sprite.Rotation = Angle.Zero; | |
return; | |
} | |
if (sprite.Rotation != Angle.FromDegrees(270) && sprite.Rotation != Angle.FromDegrees(90)) | |
{ | |
sprite.Rotation = Angle.FromDegrees(270); | |
} | |
} | |
private const float LAYING_DOWN_ANGLE = 270f; | |
private const float ALTERNATE_LAYING_ANGLE = 90f; | |
private void OnChangeStanding(Entity<StandingStateComponent> ent, ref AfterAutoHandleStateEvent args) | |
{ | |
if(_animation?.HasRunningAnimation(ent, "rotate") ?? false) | |
return; | |
if (!TryComp<SpriteComponent>(ent, out var sprite)) | |
{ | |
return; | |
} | |
if (ent.Comp.Standing) | |
{ | |
sprite.Rotation = Angle.Zero; | |
return; | |
} | |
if (sprite.Rotation != Angle.FromDegrees(LAYING_DOWN_ANGLE) && | |
sprite.Rotation != Angle.FromDegrees(ALTERNATE_LAYING_ANGLE)) | |
{ | |
sprite.Rotation = Angle.FromDegrees(LAYING_DOWN_ANGLE); | |
} | |
} |
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.
@voko421: Понятно, спасибо за ваш ответ!
🆑
Summary by CodeRabbit
Обновленные заметки о выпуске
Новые функции
Исправления ошибок
Документация