Изменение принципа работы моргания | Добавления привыкания к каплям Статиса#1058
Изменение принципа работы моргания | Добавления привыкания к каплям Статиса#1058WardexOfficial wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughВ систему моргания добавлена механика допуска (tolerance) для глазных капель. ChangesСистема допуска к глазным каплям
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
это баф или дебаф 173? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Content.Shared/_Scp/Blinking/ReducedBlinking/ReducedBlinkingSystem.cs (1)
62-122: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winРазбейте обработчик применения по цепочке
OnEvent -> TryDo -> CanDo -> Do.Сейчас в одном обработчике смешаны валидация, принятие решения и применение эффекта. Вынесите проверки (
CanDo) и саму модификацию состояния (Do) в отдельные методы — так проще сопровождать и безопаснее расширять механику допуска.Вариант минимального рефактора
- private void OnSuccess(Entity<ReducedBlinkingComponent> ent, ref EyeDropletsUsedDoAfterEvent args) - { - // проверки + применение + сайд-эффекты - } + private void OnSuccess(Entity<ReducedBlinkingComponent> ent, ref EyeDropletsUsedDoAfterEvent args) + { + TryApplyReducedBlinking(ent, ref args); + } + + private bool TryApplyReducedBlinking(Entity<ReducedBlinkingComponent> ent, ref EyeDropletsUsedDoAfterEvent args) + { + if (!CanApplyReducedBlinking(ent, ref args, out var target, out var blinkable)) + return false; + + DoApplyReducedBlinking(ent, target, ref blinkable); + return true; + }As per coding guidelines, enforce the interaction rule from
.agent/rules/ss14-interaction-flow.md:OnEvent -> TryDo -> CanDo -> Do.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Content.Shared/_Scp/Blinking/ReducedBlinking/ReducedBlinkingSystem.cs` around lines 62 - 122, Refactor the OnSuccess method in ReducedBlinkingSystem to follow the OnEvent -> TryDo -> CanDo -> Do interaction flow pattern. Extract all the validation checks (the conditions that return early with PopupPredicted calls) into a new CanDo method that takes the necessary parameters and returns a boolean indicating whether the operation can proceed. Extract all the state modification logic (creating ActiveReducedBlinkingUserComponent, updating blinkable properties, calling Dirty, ResetBlink, TryResetDelay, PlayPredicted, and UsageCount decrement/QueueDel) into a new Do method. Create a TryDo method that calls CanDo first and only calls Do if validation passes. Update OnSuccess to simply call TryDo with the appropriate parameters, maintaining the event handler's responsibility to route the event to the processing logic.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Content.Shared/_Scp/Blinking/SharedBlinkingSystem.cs`:
- Around line 28-31: The member declaration order in SharedBlinkingSystem.cs
does not follow the established convention. Move the constant
`ReducedBlinkingToleranceDecayPerSecond` to appear before the EntityQuery fields
`BlinkableQuery` and `ActiveReducedBlinkingQuery`. The correct member order for
System.cs files should be: dependencies, constants/static readonly, then runtime
cache/state. Ensure `ReducedBlinkingToleranceDecayPerSecond` is declared before
the EntityQuery runtime cache members.
In `@Resources/Locale/ru-RU/_strings/_scp/blinking/blinking.ftl`:
- Line 4: The localization key `eye-droplets-tolerance-too-high` is present only
in the Russian locale file but is missing from the English locale file. Without
this key in the English locale, the raw key identifier will be displayed instead
of translated text when the Russian localization is unavailable. Add the same
key `eye-droplets-tolerance-too-high` with an appropriate English translation to
the `Resources/Locale/en-US/_strings/_scp/blinking/blinking.ftl` file to provide
a fallback translation.
---
Outside diff comments:
In `@Content.Shared/_Scp/Blinking/ReducedBlinking/ReducedBlinkingSystem.cs`:
- Around line 62-122: Refactor the OnSuccess method in ReducedBlinkingSystem to
follow the OnEvent -> TryDo -> CanDo -> Do interaction flow pattern. Extract all
the validation checks (the conditions that return early with PopupPredicted
calls) into a new CanDo method that takes the necessary parameters and returns a
boolean indicating whether the operation can proceed. Extract all the state
modification logic (creating ActiveReducedBlinkingUserComponent, updating
blinkable properties, calling Dirty, ResetBlink, TryResetDelay, PlayPredicted,
and UsageCount decrement/QueueDel) into a new Do method. Create a TryDo method
that calls CanDo first and only calls Do if validation passes. Update OnSuccess
to simply call TryDo with the appropriate parameters, maintaining the event
handler's responsibility to route the event to the processing logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6110f7a9-e6d1-48aa-a7f5-16dfb7802cf8
📒 Files selected for processing (6)
Content.Server/_Scp/Blinking/ReducedBlinking/ReducedBlinkingSystem.csContent.Shared/_Scp/Blinking/BlinkableComponent.csContent.Shared/_Scp/Blinking/ReducedBlinking/ActiveReducedBlinkingUserComponent.csContent.Shared/_Scp/Blinking/ReducedBlinking/ReducedBlinkingSystem.csContent.Shared/_Scp/Blinking/SharedBlinkingSystem.csResources/Locale/ru-RU/_strings/_scp/blinking/blinking.ftl
Это дебаф капель |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Content.Shared/_Scp/Blinking/SharedBlinkingSystem.cs (1)
119-130:⚠️ Potential issue | 🟠 MajorИспользование
frameTimeдля изменения сетевого поля в предсказываемом коде создает рассинхронизацию.Логика пассивного распада
ReducedBlinkingTolerance(строки 123–126) выполняется в предсказываемом контексте (гейтIsFirstTimePredictedна строке 113 позволяет запускаться и на клиенте, и на сервере). ПосколькуframeTimeразличается между клиентом и сервером из-за разных частот кадров, значениеReducedBlinkingToleranceбудет расходиться между сторонами. Например, при 60 fps клиента и 30 fps сервера клиент и сервер будут уменьшать значение с разной скоростью, накапливая ошибку рассинхронизации.Это противоречит логике обработчиков событий
OnClosedEyes(строка 79) иOnOpenedEyes(строка 91), которые явно проверяютif (_net.IsServer)перед вызовомDirtyField. В Update логика должна следовать тому же паттерну.Рекомендация: добавьте проверку
if (_net.IsServer)перед блоком распада (строки 119–130), чтобы только сервер выполнял расчеты и отправлял обновленное значение клиентам.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Content.Shared/_Scp/Blinking/SharedBlinkingSystem.cs` around lines 119 - 130, The passive decay logic for ReducedBlinkingTolerance in the SharedBlinkingSystem is executing in a predictive context but using frameTime, which differs between client and server due to different frame rates, causing desynchronization. To fix this, add an `if (_net.IsServer)` check before the decay block that contains the MathF.Max calculation and DirtyField call for ReducedBlinkingTolerance. This ensures only the server performs the decay calculation and propagates the value to clients, matching the same pattern used in the OnClosedEyes and OnOpenedEyes event handlers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Content.Shared/_Scp/Blinking/SharedBlinkingSystem.cs`:
- Line 28: There is a TODO comment on line 16 about moving hardcoded values to
components, but the new constant ReducedBlinkingToleranceDecayPerSecond is being
added as a hardcoded value instead. To align with the stated goal and improve
flexibility, remove the hardcoded constant definition and add this value as a
[DataField] property in the BlinkableComponent class instead. This way the
blinking tolerance decay rate can be configured per component rather than being
a fixed system-wide constant.
---
Outside diff comments:
In `@Content.Shared/_Scp/Blinking/SharedBlinkingSystem.cs`:
- Around line 119-130: The passive decay logic for ReducedBlinkingTolerance in
the SharedBlinkingSystem is executing in a predictive context but using
frameTime, which differs between client and server due to different frame rates,
causing desynchronization. To fix this, add an `if (_net.IsServer)` check before
the decay block that contains the MathF.Max calculation and DirtyField call for
ReducedBlinkingTolerance. This ensures only the server performs the decay
calculation and propagates the value to clients, matching the same pattern used
in the OnClosedEyes and OnOpenedEyes event handlers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2bf67ec7-6dc3-4b08-aacf-723a052dc9d1
📒 Files selected for processing (1)
Content.Shared/_Scp/Blinking/SharedBlinkingSystem.cs
| [Dependency] private readonly IGameTiming _timing = default!; | ||
| [Dependency] private readonly INetManager _net = default!; | ||
|
|
||
| private const float ReducedBlinkingToleranceDecayPerSecond = 0.05f; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Hardcoded константа при наличии TODO о переносе значений в компоненты.
На строке 16 есть TODO-комментарий "Анхардкод: Перенос значений в компоненты". Новая константа ReducedBlinkingToleranceDecayPerSecond добавлена как hardcoded значение в системе. Для будущей гибкости рассмотрите возможность переноса этого значения в BlinkableComponent как [DataField].
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Content.Shared/_Scp/Blinking/SharedBlinkingSystem.cs` at line 28, There is a
TODO comment on line 16 about moving hardcoded values to components, but the new
constant ReducedBlinkingToleranceDecayPerSecond is being added as a hardcoded
value instead. To align with the stated goal and improve flexibility, remove the
hardcoded constant definition and add this value as a [DataField] property in
the BlinkableComponent class instead. This way the blinking tolerance decay rate
can be configured per component rather than being a fixed system-wide constant.
2 participants
Краткое описание | Short description
Теперь к времени следующего моргания прибавляется значение
BlinkingIntervalBonusдля увеличения данного фактора.Логика
ReducedBlinkingSystem.csтеперь работает черезBlinkingIntervalBonus, вместо изменения времени следующего моргания напрямую.🆑 Wardex
Summary by CodeRabbit