Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughРефакторинг подсистем наблюдения, близости и страха: удалён старый EyeWatching API, введены pooled-ориентированные TryGet*/IsWatched/CanSee/TryGetWatchingTargets, добавлен CollectionPool, переработана логика close‑fear/fall‑off и звуков, пересмотрены события на by‑ref record structs и обновлены клиенты и прототипы. (≈41 слов) Changes
Sequence Diagram(s)sequenceDiagram
participant Viewer as Viewer (potential watcher)
participant EyeWatch as EyeWatchingSystem
participant Lookup as EntityLookupSystem
participant Prox as ProximitySystem
participant Target as Candidate (Blinkable/T)
Viewer->>EyeWatch: TryGetWatchers(targetUid, ...)
EyeWatch->>Lookup: GetEntitiesInRange(targetPos, SeeRange, flags)
Lookup-->>EyeWatch: potentialCandidates (pooled collection)
EyeWatch->>Prox: IsRightType / IsInProximity checks
Prox-->>EyeWatch: proximity result
EyeWatch->>Target: CanSee / FOV / Blink / MobState validation
Target-->>EyeWatch: visible? (adds to realWatchers)
EyeWatch-->>Viewer: return watchers list / boolean (pooled result)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
Content.Shared/_Scp/Blinking/SharedBlinkingSystem.cs (1)
260-264: Рассмотрите замену LINQ.Any()на простой цикл для единообразия.PR декларирует устранение LINQ, но здесь используется
.Any(). Можно заменить на цикл для консистентности с остальными оптимизациями.♻️ Замена LINQ на цикл
- return _scp173List.Any(e => _watching.CanBeWatched(player, e)); + foreach (var scp173 in _scp173List) + { + if (_watching.CanBeWatched(player, scp173)) + return true; + } + return false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_Scp/Blinking/SharedBlinkingSystem.cs` around lines 260 - 264, The code currently uses LINQ Any on _scp173List to check _watching.CanBeWatched for consistency with the PR's removal of LINQ: replace the .Any(e => _watching.CanBeWatched(player, e)) call with an explicit foreach loop over _scp173List that returns true as soon as _watching.CanBeWatched(player, e) is true and returns false after the loop; keep the preceding _scp173List.Clear() and the _watching.TryGetAllEntitiesVisibleTo(player, _scp173List, _scp173SearchList) check unchanged.Content.Shared/_Scp/Watching/EyeWatchingSystem.API.cs (1)
94-119: Сделайте fill-API самодостаточными: сейчас они накапливают старые элементы в переданных буферах.
TryGetAllEntitiesVisibleToиIsWatchedBy(..., realViewers)дописывают в коллекции, но не очищают их сами. При повторном использовании одного и того же списка без внешнегоClear()останутся старые сущности, аbool-результат станет ложноположительным.💡 Предлагаемый фикс
public bool TryGetAllEntitiesVisibleTo<T>( Entity<TransformComponent?> ent, List<Entity<T>> potentialWatchers, HashSet<Entity<T>> searchSet, LineOfSightBlockerLevel type = LineOfSightBlockerLevel.Transparent, LookupFlags flags = LookupFlags.All) where T : IComponent { + potentialWatchers.Clear(); + searchSet.Clear(); + if (!Resolve(ent.Owner, ref ent.Comp)) return false; - searchSet.Clear(); _lookup.GetEntitiesInRange(ent.Comp.Coordinates, SeeRange, searchSet, flags);public bool IsWatchedBy(EntityUid target, List<Entity<BlinkableComponent>> potentialViewers, List<Entity<BlinkableComponent>> realViewers, bool useFov = true, float? fovOverride = null) { + realViewers.Clear(); + foreach (var viewer in potentialViewers) { if (!CanBeWatched(viewer.AsNullable(), target)) continue;Also applies to: 131-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.cs` around lines 94 - 119, The fill-API methods are appending to caller-provided buffers without resetting them; update TryGetAllEntitiesVisibleTo to clear the output collection at the start (call potentialWatchers.Clear()) before populating it and ensure the related IsWatchedBy overload that accepts realViewers also clears the passed-in realViewers collection before adding results; keep existing Resolve/searchSet logic but make both methods self-sufficient by clearing their output buffers up-front so repeated calls with the same list don't accumulate stale entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Content.Server/_Scp/Fear/FearSystem.cs`:
- Around line 87-89: В коде в FearSystem проверка видимых источников страха
инвертирована: _watching.TryGetAllEntitiesVisibleTo(...) возвращает false, когда
рядом нет FearSourceComponent, но текущая строка uses ! и делает ранний return
false, мешая успокоению; исправьте условие, убрав отрицание — то есть заменить
if (!_watching.TryGetAllEntitiesVisibleTo(ent.Owner, _fearSourceList,
_fearSourceSearchList, ent.Comp.SeenBlockerLevel)) return false; на проверку без
! (if (_watching.TryGetAllEntitiesVisibleTo(...)) return false;) чтобы система
возвращала false только когда действительно есть видимые источники страха;
проверьте переменные _fearSourceList, _fearSourceSearchList и параметры
ent.Owner / ent.Comp.SeenBlockerLevel при правке.
In `@Content.Server/_Scp/Fear/FearSystem.Fears.cs`:
- Around line 61-68: TryGetAllEntitiesVisibleTo fills _moodList with potential
observers but not those who actually saw the event; update the loop so you only
call AddNegativeMoodEffect(uid, MoodSomeoneDiedOnMyEyes) after verifying each
candidate truly observed ev.Target (e.g. use the watching/FOV/blindness check on
_watching such as an _watching.IsWatching/CanSee/IsObserving call or the
project's FOV/VisibilitySystem method) instead of unconditionally adding the
debuff for every uid from _moodList; keep the initial TryGetAllEntitiesVisibleTo
call and add the per-uid visibility check before calling AddNegativeMoodEffect.
In
`@Content.Server/_Scp/Research/Artifacts/Effects/_ScpSpecific/Scp096/Madness/ArtifactScp096MadnessSystem.cs`:
- Around line 37-38: The code is shuffling `_list` and calling `TakePercentage`
but then still iterating over `_list`, so only the `IEnumerable` result of
`_list.ShuffleRobust(_random).TakePercentage(ent.Comp.Percent)` should be used;
capture that result into a collection (e.g., `selected =
_list.ShuffleRobust(_random).TakePercentage(ent.Comp.Percent).ToList()`) and
then iterate `foreach (var target in selected)` instead of `foreach (var target
in _list)`, ensuring you reference `_list`, `ShuffleRobust`, `TakePercentage`,
and `ent.Comp.Percent` when making the change.
- Line 23: Поле _list сейчас объявлено как List<Entity<BlinkableComponent>>;
добавь приватное поле HashSet<EntityUid> _searchSet (или подходящий тип HashSet
для сущностей) и в методе OnActivated заменяй вызов TryGetAllEntitiesVisibleTo,
который сейчас использует 3-параметровую перегрузку с LineOfSightBlockerLevel,
на 5-параметровую перегрузку, передавая _searchSet в качестве третьего параметра
и сохраняя остальные аргументы (включая LineOfSightBlockerLevel в
соответствующей позиции), а затем используй результаты как раньше; локализуй
изменения к полям _list, новому _searchSet и вызову
TryGetAllEntitiesVisibleTo/OnActivated.
In `@Content.Shared/_Scp/Scp173/SharedScp173System.cs`:
- Around line 222-239: Метод IsWatched считает кандидатов только по LOS; добавь
дополнительные проверки по фактическому взгляду перед инкрементом viewersCount:
после вызова Watching.CanBeWatched(viewer.AsNullable(), scp) проверяй, что
наблюдатель не слеп в глазах (IsEyeBlinded / аналогичный флаг) и действительно
держит SCP в поле зрения (FOV-проверка, например Watching.IsInFov или
эквивалент), и только тогда увеличивай viewersCount; обнови логику в IsWatched и
используй существующий _blinkableList/Watching методы для этих дополнительных
условий.
In `@Content.Shared/_Scp/Watching/EyeWatching.cs`:
- Around line 52-77: Replace the direct allocations of new
List<Entity<BlinkableComponent>>() for potentialViewers and realViewers with
pooled lists obtained via RentBlinkableList(), and ensure every early exit
returns those lists to the pool via ReturnBlinkableList(); specifically, get
potentialViewers = RentBlinkableList() before calling
TryGetAllEntitiesVisibleTo(uid, potentialViewers, _searchList) and
ReturnBlinkableList(potentialViewers) on each continue path (after the failed
TryGetAllEntitiesVisibleTo, after the SimpleMode continue, and at the end of the
loop), and get realViewers = RentBlinkableList() before calling IsWatchedBy(uid,
potentialViewers, realViewers) and ReturnBlinkableList(realViewers) on each
continue/exit path (also remember to ReturnBlinkableList(potentialViewers) after
realViewers processing); keep calls to TryGetAllEntitiesVisibleTo and
IsWatchedBy and checks of watchingComponent.SimpleMode unchanged except for
using the pooled lists.
In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.cs`:
- Around line 33-45: The current implementation sets watchersCount =
potentialWatchers.Count which counts LOS candidates instead of actual watchers;
change it to compute watchersCount as the number of entries in potentialWatchers
that satisfy the actual watching/visibility check used by the IsWatched logic
(i.e., only entities that truly see the target), or adjust the called overload
so it fills potentialWatchers with only real watchers; specifically after
calling IsWatched(ent, potentialWatchers, searchSet, ...) replace watchersCount
= potentialWatchers.Count with a count of only those candidates that pass the
same "is watching" predicate (use the internal visibility/check routine the
IsWatched overload uses) so thresholds like SCP-173's watcher limit are correct.
---
Nitpick comments:
In `@Content.Shared/_Scp/Blinking/SharedBlinkingSystem.cs`:
- Around line 260-264: The code currently uses LINQ Any on _scp173List to check
_watching.CanBeWatched for consistency with the PR's removal of LINQ: replace
the .Any(e => _watching.CanBeWatched(player, e)) call with an explicit foreach
loop over _scp173List that returns true as soon as
_watching.CanBeWatched(player, e) is true and returns false after the loop; keep
the preceding _scp173List.Clear() and the
_watching.TryGetAllEntitiesVisibleTo(player, _scp173List, _scp173SearchList)
check unchanged.
In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.cs`:
- Around line 94-119: The fill-API methods are appending to caller-provided
buffers without resetting them; update TryGetAllEntitiesVisibleTo to clear the
output collection at the start (call potentialWatchers.Clear()) before
populating it and ensure the related IsWatchedBy overload that accepts
realViewers also clears the passed-in realViewers collection before adding
results; keep existing Resolve/searchSet logic but make both methods
self-sufficient by clearing their output buffers up-front so repeated calls with
the same list don't accumulate stale entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b671fd8-55ec-408a-9ad4-338c787ada0f
📒 Files selected for processing (17)
Content.Server/_Scp/Fear/FearSystem.Fears.csContent.Server/_Scp/Fear/FearSystem.csContent.Server/_Scp/Research/Artifacts/Effects/_ScpSpecific/Scp096/Madness/ArtifactScp096MadnessComponent.csContent.Server/_Scp/Research/Artifacts/Effects/_ScpSpecific/Scp096/Madness/ArtifactScp096MadnessSystem.csContent.Server/_Scp/Scp173/Scp173System.csContent.Shared/_Scp/Blinking/SharedBlinkingSystem.csContent.Shared/_Scp/Fear/Systems/SharedFearSystem.Helpers.csContent.Shared/_Scp/Fear/Systems/SharedFearSystem.csContent.Shared/_Scp/Helpers/ScpHelpers.csContent.Shared/_Scp/Scp106/Systems/SharedScp106System.Phantom.csContent.Shared/_Scp/Scp173/SharedScp173System.csContent.Shared/_Scp/Watching/EyeWatching.Pools.csContent.Shared/_Scp/Watching/EyeWatching.csContent.Shared/_Scp/Watching/EyeWatchingSystem.API.csContent.Shared/_Scp/Watching/EyeWatchingSystem.Events.csContent.Shared/_Scp/Watching/WatchingTargetComponent.csResources/Prototypes/_Scp/Entities/Mobs/Player/Scp/Main/scp096.yml
...r/_Scp/Research/Artifacts/Effects/_ScpSpecific/Scp096/Madness/ArtifactScp096MadnessSystem.cs
Outdated
Show resolved
Hide resolved
...r/_Scp/Research/Artifacts/Effects/_ScpSpecific/Scp096/Madness/ArtifactScp096MadnessSystem.cs
Outdated
Show resolved
Hide resolved
Collaborator
Author
|
@coderabbitai рассмотри возможность и целесообразность создавать обертку над HashSet и List для легкой утилизации через using и реализации интерейса IDisposbable вместо прямой ручной утилизации. Подготовь красивый отчет, табличку ЗА ПРОТИВ и примеры как это интегрировать на примере цикла EyeWatching |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 18352
📋 Отчёт: IDisposable-обёртка над пуловыми коллекциямиКонтекст текущей реализацииВ // ❌ EyeWatching.cs — Update() — пул не используется!
var potentialViewers = new List<Entity<BlinkableComponent>>();
// ...
var realViewers = new List<Entity<BlinkableComponent>>();Тогда как в // ✅ API.cs — правильное использование
var potentialWatchers = RentBlinkableList();
// ...
ReturnBlinkableList(potentialWatchers);⚖️ Таблица: ЗА и ПРОТИВ IDisposable-обёртки
Вывод: для синхронного hot path ( 🔧 Реализация1. Добавить
|
| Задача | Рекомендация |
|---|---|
Исправить new List<>() в Update() |
Обязательно — текущие аллокации в hot path не используют пул |
Внедрить ref struct обёртку |
Целесообразно для внутреннего кода EyeWatchingSystem |
Применять к полям внешних систем (FearSystem._fearSourceList и т.д.) |
Не нужно — они арендуют коллекции единожды как class-level поля |
Внешние системы (FearSystem, BlinkingSystem, ScpHelpers) хранят List<>/HashSet<> как поля класса и переиспользуют их между тиками — это уже оптимальный паттерн, пул там не нужен.
Collaborator
Author
|
@coderabbitai рассмотри возможность использовать пулы в других системах. Рассмотри как можно поддерживать все возможные компоненты без ручного создания в EyeWatching |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 80464
📋 Часть 1: Нужны ли пулы в других системах?Вывод: нет. Все внешние системы уже используют оптимальный паттерн — постоянные поля класса:
Эти поля создаются один раз при старте, очищаются перед использованием и никогда больше не аллоцируют. Пул здесь не нужен — он только добавит сложность без выгоды. Пул нужен только внутри // ❌ EyeWatching.cs Update() — пул полностью игнорируется!
var potentialViewers = new List<Entity<BlinkableComponent>>(); // аллокация каждый тик
// ...
var realViewers = new List<Entity<BlinkableComponent>>(); // ещё одна аллокация каждый тик📋 Часть 2: Универсальный пул без ручного создания per-типТекущая проблема: чтобы поддержать новый private readonly Stack<List<Entity<NewComponent>>> _newListPool = new(); // +1
private List<Entity<NewComponent>> RentNewList() { ... } // +1
private void ReturnNewList(...) { ... } // +1Решение: type-keyed generic poolОдин словарь с // Content.Shared/_Scp/Watching/EyeWatching.Pools.cs — полностью заменяет текущую реализацию
public sealed partial class EyeWatchingSystem
{
private readonly Dictionary<Type, Stack<object>> _listPool = new();
private readonly Dictionary<Type, Stack<object>> _setPool = new();
// --- Rent ---
private List<Entity<T>> RentList<T>() where T : IComponent
{
if (_listPool.TryGetValue(typeof(T), out var stack) && stack.TryPop(out var obj))
return (List<Entity<T>>) obj;
return [];
}
private HashSet<Entity<T>> RentSet<T>() where T : IComponent
{
if (_setPool.TryGetValue(typeof(T), out var stack) && stack.TryPop(out var obj))
return (HashSet<Entity<T>>) obj;
return [];
}
// --- Return ---
private void ReturnList<T>(List<Entity<T>> list) where T : IComponent
{
list.Clear();
if (!_listPool.TryGetValue(typeof(T), out var stack))
_listPool[typeof(T)] = stack = new Stack<object>();
stack.Push(list);
}
private void ReturnSet<T>(HashSet<Entity<T>> set) where T : IComponent
{
set.Clear();
if (!_setPool.TryGetValue(typeof(T), out var stack))
_setPool[typeof(T)] = stack = new Stack<object>();
stack.Push(set);
}
// --- Rent UID list (отдельно, т.к. не Entity<T>) ---
private readonly Stack<List<EntityUid>> _uidListPool = new();
private List<EntityUid> RentUidList()
=> _uidListPool.TryPop(out var l) ? l : [];
private void ReturnUidList(List<EntityUid> list)
{
list.Clear();
_uidListPool.Push(list);
}
}
ref struct обёртки для
|
| Аспект | Type-keyed Dictionary pool | Текущий hardcoded pool |
|---|---|---|
| Поддержка новых типов | ✅ Автоматически | ❌ Ручное добавление 3 членов |
| Аллокации на первое использование типа | 1× Stack<object> |
1× Stack<> при объявлении поля |
| Аллокации в hot path | ✅ 0 | ✅ 0 |
| Боксинг | Ссылка на List<>/HashSet<> — одна аллокация per slot |
Нет |
| Безопасность возврата | ✅ через ref struct + using |
❌ ручной Return, легко забыть |
| Применимость в async | ✅ class версия |
✅ |
| Сложность реализации | Средняя | Низкая |
Конкретные задачи по приоритету:
| # | Задача | Приоритет |
|---|---|---|
| 1 | Исправить new List<>() в Update() |
🔴 Обязательно — текущий hot path аллоцирует каждый тик |
| 2 | Заменить hardcoded stacks на type-keyed pool | 🟡 Желательно — упрощает поддержку новых типов |
| 3 | Добавить PooledList/PooledSet ref structs |
🟡 Желательно — устраняет риск забытого ReturnXxx |
| 4 | Трогать внешние системы (Fear, Blinking, и т.д.) | 🟢 Не нужно — они уже оптимальны |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
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)
249-252:⚠️ Potential issue | 🟡 MinorКонтракт
IsScpNearby()больше не совпадает с реализацией.XML-комментарий все еще обещает
SCP-173иSCP-096, а код теперь ищет толькоScp173Component. Так какOnClosedEyes()использует этот helper в ветке автопомаргивания, это уже изменение поведения, а не просто устаревшая документация. Если 096 исключен осознанно, лучше сразу сузить имя/комментарий; если нет — сюда потерялась вторая проверка.Also applies to: 258-262
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_Scp/Blinking/SharedBlinkingSystem.cs` around lines 249 - 252, The XML-doc and method contract for IsScpNearby no longer match its behavior: it documents checking for SCP-173 and SCP-096 but the implementation only checks for Scp173Component, which alters behavior used by OnClosedEyes/autoblink; either restore the missing check or update the contract and identifier to match intent. Fix by updating the IsScpNearby implementation to also test for Scp096Component (e.g., check entity.HasComponent<Scp096Component>() alongside Scp173Component) if 096 should be considered, or else change the XML comment and method/name to explicitly state it only checks for Scp173 to avoid misleading callers (also apply the same update to the duplicate docblock referenced around the other occurrence).
🧹 Nitpick comments (4)
Content.Shared/_Scp/Helpers/CollectionPool.cs (1)
61-65: Не логируйте предупреждение на каждомReturn()после насыщения пула.Если pool упрется в лимит, этот
Logger.Warning()попадет в hot path и сам создаст лишние аллокации/I/O. Здесь лучше молча дропать коллекцию или логировать один раз / с rate limit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_Scp/Helpers/CollectionPool.cs` around lines 61 - 65, The Return() path currently calls Logger.Warning whenever Pool.Count >= 512 which causes hot-path allocations; change Return() in CollectionPool to silently drop the collection when Pool.Count >= 512 instead of logging on every call, or implement a rate-limited/one-time warning (e.g. a private bool _poolOverflowWarned or a timestamp-based throttle) so Logger.Warning is only emitted once or at a controlled rate; ensure you reference Pool.Count and the 512 cap constant and update behavior in the Return method accordingly.Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watched.cs (1)
2-2: Удалите неиспользуемый импортSystem.Linq.После замены
.Any()на.Count != 0импорт станет не нужен.♻️ Предлагаемое исправление
using System.Diagnostics.CodeAnalysis; -using System.Linq; using Content.Shared._Scp.Blinking;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watched.cs` at line 2, Remove the now-unused using directive System.Linq from the top of Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watched.cs because you've replaced usages of .Any() with Count != 0; verify there are no remaining LINQ extension method calls (e.g., .Any()) in the file before deleting the using line to avoid breakage.Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watching.cs (1)
1-2: Неиспользуемый импортSystem.Linqи использование LINQ в hot path.PR заявляет об удалении LINQ для оптимизации, но
System.Linqимпортирован и используется на строке 47. Рекомендуется удалить импорт и заменитьtargets.Any()наtargets.Count != 0.♻️ Предлагаемое исправление
-using System.Linq; using Content.Shared._Scp.Helpers; using Content.Shared._Scp.Proximity;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watching.cs` around lines 1 - 2, Remove the unused System.Linq import and eliminate the LINQ hot-path call by replacing targets.Any() with a direct count check (targets.Count != 0) in the API watching code; update the file to delete the "using System.Linq;" line and change the call site where the local variable targets is queried (the Any() invocation in EyeWatchingSystem.API.Watching.cs) to use the Count property instead.Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Base.cs (1)
80-91: Упрощение: избыточная проверка.Строки 87-90 можно упростить, убрав промежуточную проверку.
♻️ Предлагаемое упрощение
public bool TryGetAnyEntitiesVisibleTo<T>( Entity<TransformComponent?> viewer, LineOfSightBlockerLevel type = LineOfSightBlockerLevel.Transparent, LookupFlags flags = LookupFlags.Uncontained | LookupFlags.Approximate) where T : IComponent { using var searchSet = HashSetPoolEntity<T>.Rent(); - if (!TryGetAnyEntitiesVisibleTo(viewer, out _, searchSet.Value, type, flags)) - return false; - - return true; + return TryGetAnyEntitiesVisibleTo(viewer, out _, searchSet.Value, type, flags); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Base.cs` around lines 80 - 91, The method TryGetAnyEntitiesVisibleTo<T>(Entity<TransformComponent?> viewer, ...) contains an unnecessary conditional; replace the if/return pattern with a direct return of the inner call. Specifically, keep the using var searchSet = HashSetPoolEntity<T>.Rent(); line and then return the result of TryGetAnyEntitiesVisibleTo(viewer, out _, searchSet.Value, type, flags) instead of the current if (!... ) return false; return true; sequence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Content.Shared/_Scp/Helpers/CollectionPool.cs`:
- Around line 80-121: PooledCollection is a public mutable struct and therefore
copyable, which can produce multiple owners that call Dispose() and
double-return the same collection; make PooledCollection non-copyable by
changing its declaration to a ref struct (or convert it to a sealed class owner
if ref struct is not viable) and keep the existing members Value and Dispose
semantics so callers cannot create independent copies; additionally stop
spamming logs from Return by removing or demoting the per-call warning when
Pool.Count >= 512 (e.g., log at a lower level, increase threshold, or emit the
warning only once using a static/atomic flag), and update the Return and any
pooling bookkeeping to remain thread-safe with the new behavior.
In `@Content.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.Target.cs`:
- Around line 193-194: The current check uses _watching.CanSee(viewer, scp,
false) together with ignoreBlinded, but CanSee(..., false) also accounts for
incapacitation and CanSeeAttemptEvent, so setting ignoreBlinded = true
unintentionally bypasses non-blind vision blocks; replace the call with a
blindness-only check: keep the general visibility check using
_watching.CanSee(viewer, scp, false) for overall sight, but when evaluating
ignoreBlinded consult only the blindness state (e.g. a dedicated
IsBlind/IsBlinking helper on the EyeWatchingSystem or directly query the
blindness/blink status component or status effect on viewer) instead of CanSee;
update the condition in SharedScp096System.Target (the if that currently
references _watching.CanSee(... ) && !ignoreBlinded) so ignoreBlinded only
ignores pure blindness by calling that blindness-only check (or add a small
helper method in EyeWatchingSystem.API.Base like IsViewerBlind(viewer) and
reference it).
In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watched.cs`:
- Around line 22-24: Call to TryGetWatchers passes the two bools in the wrong
order; swap the sixth and seventh arguments so the call matches the signature:
use TryGetWatchers(target, realWatchers.Value, type, flags, checkProximity,
useFov, fovOverride) (referencing the TryGetWatchers invocation and the
parameters checkProximity, useFov, realWatchers.Value, fovOverride).
- Line 66: Замените использование LINQ Any() на проверку свойства Count для
коллекции realWatchers в методе/свойстве, возвращающем булево значение (где
сейчас используется realWatchers.Any()); конкретно: вместо вызова
realWatchers.Any() используйте проверку длины коллекции через свойство Count
(realWatchers.Count != 0) чтобы избежать лишней аллокации при работе с List<T>.
In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watching.cs`:
- Line 47: Replace the LINQ allocator call targets.Any() with a direct count
check by changing the return expression to return targets.Count != 0; ensure the
variable referenced is the List<T> (or otherwise exposes Count) named targets so
the O(1) non-allocating property is used instead of Any().
---
Outside diff comments:
In `@Content.Shared/_Scp/Blinking/SharedBlinkingSystem.cs`:
- Around line 249-252: The XML-doc and method contract for IsScpNearby no longer
match its behavior: it documents checking for SCP-173 and SCP-096 but the
implementation only checks for Scp173Component, which alters behavior used by
OnClosedEyes/autoblink; either restore the missing check or update the contract
and identifier to match intent. Fix by updating the IsScpNearby implementation
to also test for Scp096Component (e.g., check
entity.HasComponent<Scp096Component>() alongside Scp173Component) if 096 should
be considered, or else change the XML comment and method/name to explicitly
state it only checks for Scp173 to avoid misleading callers (also apply the same
update to the duplicate docblock referenced around the other occurrence).
---
Nitpick comments:
In `@Content.Shared/_Scp/Helpers/CollectionPool.cs`:
- Around line 61-65: The Return() path currently calls Logger.Warning whenever
Pool.Count >= 512 which causes hot-path allocations; change Return() in
CollectionPool to silently drop the collection when Pool.Count >= 512 instead of
logging on every call, or implement a rate-limited/one-time warning (e.g. a
private bool _poolOverflowWarned or a timestamp-based throttle) so
Logger.Warning is only emitted once or at a controlled rate; ensure you
reference Pool.Count and the 512 cap constant and update behavior in the Return
method accordingly.
In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Base.cs`:
- Around line 80-91: The method
TryGetAnyEntitiesVisibleTo<T>(Entity<TransformComponent?> viewer, ...) contains
an unnecessary conditional; replace the if/return pattern with a direct return
of the inner call. Specifically, keep the using var searchSet =
HashSetPoolEntity<T>.Rent(); line and then return the result of
TryGetAnyEntitiesVisibleTo(viewer, out _, searchSet.Value, type, flags) instead
of the current if (!... ) return false; return true; sequence.
In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watched.cs`:
- Line 2: Remove the now-unused using directive System.Linq from the top of
Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watched.cs because you've
replaced usages of .Any() with Count != 0; verify there are no remaining LINQ
extension method calls (e.g., .Any()) in the file before deleting the using line
to avoid breakage.
In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watching.cs`:
- Around line 1-2: Remove the unused System.Linq import and eliminate the LINQ
hot-path call by replacing targets.Any() with a direct count check
(targets.Count != 0) in the API watching code; update the file to delete the
"using System.Linq;" line and change the call site where the local variable
targets is queried (the Any() invocation in EyeWatchingSystem.API.Watching.cs)
to use the Count property instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3a9b23d8-4042-4c35-844e-91ef61522896
📒 Files selected for processing (17)
Content.Server/_Scp/Fear/FearSystem.Fears.csContent.Server/_Scp/Fear/FearSystem.csContent.Server/_Scp/Research/Artifacts/Effects/_ScpSpecific/Scp096/Madness/ArtifactScp096MadnessSystem.csContent.Server/_Scp/Scp173/Scp173System.csContent.Shared/_Scp/Blinking/SharedBlinkingSystem.csContent.Shared/_Scp/Fear/Systems/SharedFearSystem.Helpers.csContent.Shared/_Scp/Fear/Systems/SharedFearSystem.csContent.Shared/_Scp/Helpers/CollectionPool.csContent.Shared/_Scp/Helpers/ScpHelpers.csContent.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.Target.csContent.Shared/_Scp/Scp106/Systems/SharedScp106System.Phantom.csContent.Shared/_Scp/Scp173/SharedScp173System.csContent.Shared/_Scp/Watching/EyeWatching.csContent.Shared/_Scp/Watching/EyeWatchingSystem.API.Base.csContent.Shared/_Scp/Watching/EyeWatchingSystem.API.Watched.csContent.Shared/_Scp/Watching/EyeWatchingSystem.API.Watching.csContent.Shared/_Scp/Watching/EyeWatchingSystem.API.cs
💤 Files with no reviewable changes (1)
- Content.Shared/_Scp/Watching/EyeWatchingSystem.API.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- Content.Server/_Scp/Research/Artifacts/Effects/_ScpSpecific/Scp096/Madness/ArtifactScp096MadnessSystem.cs
- Content.Server/_Scp/Scp173/Scp173System.cs
…date loop. Decrease WatchingCheckInterval from 0.3 to 0.1
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (2)
Content.Shared/_Scp/Helpers/CollectionPool.cs (1)
77-118:⚠️ Potential issue | 🟠 Major
PooledCollectionвсё ещё копируемый owner-хендл.Как
public struct, он копируется при присваивании и передаче по значению. Здесь копии будут делить один и тот же_value, поэтому дваDispose()могут дважды вернуть одну и ту же коллекцию в пул и затем выдать один экземпляр двум арендаторам. Для такого API безопаснееref structс pattern-basedDispose()или ссылочная обёртка. Перед переводом наref structстоит только проверить, что текущие call site не сохраняют wrapper в полях и не тянут его вasync.♻️ Возможный фикс
- public struct PooledCollection : IDisposable + public ref struct PooledCollection {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_Scp/Helpers/CollectionPool.cs` around lines 77 - 118, PooledCollection is a copyable struct so copies can share the same _value and cause double-Return; fix by changing PooledCollection to a non-copyable owner: either (A) make it a ref struct (e.g., "ref struct PooledCollection") and keep the current Dispose/Value logic (but first verify no call sites store it in fields or use it in async methods), or (B) convert it to a sealed reference type wrapper (class PooledCollection) that tracks disposal and nulls out _value on first Dispose to prevent double Return; in both cases keep using the existing Value and Dispose members and ensure the Return(_value) call is only ever invoked once.Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watched.cs (1)
127-142:⚠️ Potential issue | 🟠 MajorВ
IsWatchedByAny()перепутаныcheckProximityиuseFov.
IsWatchedBy()принимает их как(..., checkProximity, useFov, ...), а здесь передается(..., useFov, checkProximity, ...). Пока оба аргумента равны дефолту, баг скрыт, но любой кастомный вызов начнет отключать не ту проверку.💡 Предлагаемое исправление
- if (!IsWatchedBy(target, viewer, type, useFov, checkProximity, useTimeCompensation, checkBlinking, fovOverride)) + if (!IsWatchedBy(target, viewer, type, checkProximity, useFov, useTimeCompensation, checkBlinking, fovOverride)) continue;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watched.cs` around lines 127 - 142, In IsWatchedByAny(), the boolean arguments checkProximity and useFov are passed to IsWatchedBy() in the wrong order; locate the call to IsWatchedBy(target, viewer, type, useFov, checkProximity, useTimeCompensation, checkBlinking, fovOverride) and swap the two booleans so it becomes IsWatchedBy(target, viewer, type, checkProximity, useFov, useTimeCompensation, checkBlinking, fovOverride), leaving all other parameters unchanged.
🧹 Nitpick comments (2)
Content.Shared/_Scp/Proximity/ProximitySystem.cs (1)
273-274: Рассмотрите оптимизациюIsNearby<T>для избежания LINQ аллокаций.Использование
.Any()с лямбдой может создавать аллокации на каждый вызов. Если этот метод вызывается часто, рассмотрите замену на циклforeach.♻️ Предлагаемый рефакторинг
public bool IsNearby<T>(EntityUid uid, float range, LineOfSightBlockerLevel level = LineOfSightBlockerLevel.None) where T : IComponent { - return _lookup.GetEntitiesInRange<T>(Transform(uid).Coordinates, range, LookupFlags.Uncontained | LookupFlags.Approximate) - .Any(e => IsRightType(uid, e, level, out _)); + foreach (var e in _lookup.GetEntitiesInRange<T>(Transform(uid).Coordinates, range, LookupFlags.Uncontained | LookupFlags.Approximate)) + { + if (IsRightType(uid, e, level, out _)) + return true; + } + return false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_Scp/Proximity/ProximitySystem.cs` around lines 273 - 274, The IsNearby<T> call allocates a lambda via .Any(...); replace the LINQ Any usage with an explicit loop over the IEnumerable returned by _lookup.GetEntitiesInRange<T>(Transform(uid).Coordinates, range, LookupFlags.Uncontained | LookupFlags.Approximate) and call IsRightType(uid, e, level, out _) for each entity, returning true immediately when IsRightType returns true and false after the loop; this removes the per-call delegate allocation while preserving the same behavior and parameters.Content.Server/_Scp/Fear/FearSystem.SoundEffects.cs (1)
83-87: Отсутствует вызов базового методаbase.StopBreathing(ent).В методе
StartBreathing(строка 66) вызываетсяbase.StartBreathing(ent), но вStopBreathingаналогичный вызов отсутствует. Хотя сейчас базовый метод пуст, для консистентности и защиты от будущих изменений в базовом классе рекомендуется добавить вызов.♻️ Предлагаемое исправление
protected override void StopBreathing(Entity<FearActiveSoundEffectsComponent> ent) { + base.StopBreathing(ent); + ent.Comp.BreathingAudioStream = _audio.Stop(ent.Comp.BreathingAudioStream); DirtyField(ent, ent.Comp, nameof(FearActiveSoundEffectsComponent.BreathingAudioStream)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Server/_Scp/Fear/FearSystem.SoundEffects.cs` around lines 83 - 87, The StopBreathing override omits calling the base implementation; add a call to base.StopBreathing(ent) (as StartBreathing already does) before or after stopping the audio stream in StopBreathing to maintain consistency and future-proof behavior; update the StopBreathing method that manipulates ent.Comp.BreathingAudioStream and calls DirtyField(ent, ent.Comp, nameof(FearActiveSoundEffectsComponent.BreathingAudioStream)) to also invoke base.StopBreathing(ent).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Content.Shared/_Scp/Blinking/SharedBlinkingSystem.cs`:
- Around line 259-267: Метод IsScpNearby в SharedBlinkingSystem сейчас ищет
только Scp173Component, исключая SCP-096; верните ожидаемое поведение либо
добавив SCP-096 в проверку (расширить арендуемый список и условие Any чтобы
учитывать Scp096Component / соответствующий компонент SCP-096), либо реализовав
в SharedScp096System альтернативный механизм, который предотвращает визуальные
артефакты моргания (подписка на события просмотра или изменение логики
_watching.CanBeWatched/WatchingTargetComponent для SimpleMode) так, чтобы
игроки, наблюдающие SCP-096, также обрабатывались корректно.
In `@Content.Shared/_Scp/Fear/Components/Traits/FearFaintingComponent.cs`:
- Around line 30-31: Fix the double space in the field declaration: change
"public EntProtoId StatusEffect = \"StatusEffectForcedSleeping\";" to use a
single space after the access modifier so the declaration reads "public
EntProtoId StatusEffect = \"StatusEffectForcedSleeping\";". Locate the
declaration of StatusEffect in the FearFaintingComponent class and adjust the
spacing only.
In `@Content.Shared/_Scp/Fear/Systems/SharedFearSystem.Fears.cs`:
- Around line 27-29: The warning message is misleading: it claims
HemophobiaComponent will be deleted but the code only returns; update the
Log.Warning call (which uses ToPrettyString(ent), nameof(HemophobiaComponent)
and nameof(FearComponent)) to accurately state the component is present without
FearComponent and will NOT be deleted (or otherwise reflect the actual
behavior), e.g. change the phrase "will be deleted" to "will not be deleted" or
similar so the log matches the code path.
In `@Content.Shared/_Scp/Fear/Systems/SharedFearSystem.Gameplay.cs`:
- Around line 185-197: ManageFallOff currently removes
ActiveFearFallOffComponent unconditionally in the else branch causing add/remove
thrashing; change logic so you only add the component when State >=
FallOffRequiredState and it does not exist (using EnsureComp/Dirty) and only
remove it when State < FallOffRequiredState and the component exists (use
HasComp + RemComp). Update ManageFallOff to check ent.Comp.State against
ent.Comp.FallOffRequiredState in both branches and call
RemComp<ActiveFearFallOffComponent>(ent) only when the state dropped below the
required threshold.
In `@Content.Shared/_Scp/Helpers/CollectionPool.cs`:
- Around line 54-71: The XML summary and the implementation disagree on the
capacity threshold (summary says 1024 but Return currently checks for Capacity >
2048); introduce a single named constant (e.g., MaxReusableCapacity) and use it
in Return's checks for List<T> and HashSet<T>, update the XML comment to
reference that same threshold value (or the constant name) so the documentation
and logic remain consistent, and ensure Pool.Count limit (512) is also left
as-is or similarly extracted if desired.
In `@Content.Shared/_Scp/Proximity/ProximityReceiverComponent.cs`:
- Around line 22-23: Поле Flags в ProximityReceiverComponent не сетится клиенту
потому что отсутствует сетевой атрибут; добавьте атрибут [AutoNetworkedField]
(оставив/сочетая [DataField] если нужно) над публичным полем LookupFlags Flags =
LookupFlags.Uncontained | LookupFlags.Approximate; чтобы значение (и map/runtime
override) реплицировалось на клиентов; проверьте также что класс
ProximityReceiverComponent участвует в автосете состояния и что тип LookupFlags
поддерживает сериализацию для AutoNetworkedField.
In `@Content.Shared/_Scp/Scp173/SharedScp173System.cs`:
- Around line 186-190: IsContained currently uses
Watching.TryGetAnyEntitiesVisibleTo<Scp173BlockStructureDamageComponent>(...)
which searches using SeeRange and thus exceeds the intended
ContainmentRoomSearchRadius; update IsContained to first limit candidates to
ContainmentRoomSearchRadius (e.g., use a radius-based query such as
Watching.GetEntitiesInRange / TryGetEntitiesInRange or the watcher API variant
that accepts a maxDistance) and then check visibility/line-of-sight only for
those candidates, so Scp173BlockStructureDamageComponent checks are restricted
to ContainmentRoomSearchRadius and CanBlind logic stays correct.
In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Base.cs`:
- Around line 245-248: CanSeeAttemptEvent is used by handlers expecting a by-ref
event but is declared without the [ByRefEvent] attribute and RaiseLocalEvent is
called without ref, so add the [ByRefEvent] attribute to the CanSeeAttemptEvent
declaration and change both call sites from RaiseLocalEvent(viewer,
canSeeAttempt) to RaiseLocalEvent(viewer, ref canSeeAttempt) (the shown call
where canSeeAttempt is created and the analogous call in BlindableSystem where
CanSeeAttemptEvent is raised) so that handlers using ref CanSeeAttemptEvent args
can propagate Cancel()/Blind changes back to the original event instance.
In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watched.cs`:
- Around line 93-111: TryGetWatchersFrom is appending to the caller-provided
realWatchers list without clearing it, causing stale entries/duplicates on
repeated calls; modify TryGetWatchersFrom (the method taking parameters
EntityUid target, List<Entity<BlinkableComponent>> realWatchers,
ICollection<Entity<BlinkableComponent>> potentialWatchers, ...) to call
realWatchers.Clear() at the start of the method before iterating
potentialWatchers so the list is freshly populated each invocation.
In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.cs`:
- Around line 51-53: Цель: исправить пропуск SetNextTime() при отсутствии
кандидатов/валидных зрителей — в блоках где вы делаете using var
potentialWatchers = ListPoolEntity<BlinkableComponent>.Rent() и вызываете
TryGetAllEntitiesVisibleTo(uid, potentialWatchers.Value) (и аналогичный блок на
79-81), вызовите SetNextTime(uid) перед continue (или замените continue на вызов
SetNextTime(uid) и продолжение), чтобы корректно обновить NextTimeWatchedCheck и
соблюсти троттлинг WatchingCheckInterval в Update().
In `@Content.Shared/_Scp/Watching/WatchingTargetComponent.cs`:
- Around line 28-29: The comparison against the nullable NextTimeWatchedCheck
must guard for null: in EyeWatchingSystem.cs where it does if (_timing.CurTime <
watchingComponent.NextTimeWatchedCheck) add a HasValue check (e.g.
watchingComponent.NextTimeWatchedCheck.HasValue && _timing.CurTime <
watchingComponent.NextTimeWatchedCheck.Value) or initialize NextTimeWatchedCheck
in WatchingTargetComponent (NextTimeWatchedCheck = TimeSpan.Zero or a sensible
default) to avoid a NullReferenceException before SetNextTime() runs; update any
dependent logic that assumes non-null to use .Value only after the null check.
---
Duplicate comments:
In `@Content.Shared/_Scp/Helpers/CollectionPool.cs`:
- Around line 77-118: PooledCollection is a copyable struct so copies can share
the same _value and cause double-Return; fix by changing PooledCollection to a
non-copyable owner: either (A) make it a ref struct (e.g., "ref struct
PooledCollection") and keep the current Dispose/Value logic (but first verify no
call sites store it in fields or use it in async methods), or (B) convert it to
a sealed reference type wrapper (class PooledCollection) that tracks disposal
and nulls out _value on first Dispose to prevent double Return; in both cases
keep using the existing Value and Dispose members and ensure the Return(_value)
call is only ever invoked once.
In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watched.cs`:
- Around line 127-142: In IsWatchedByAny(), the boolean arguments checkProximity
and useFov are passed to IsWatchedBy() in the wrong order; locate the call to
IsWatchedBy(target, viewer, type, useFov, checkProximity, useTimeCompensation,
checkBlinking, fovOverride) and swap the two booleans so it becomes
IsWatchedBy(target, viewer, type, checkProximity, useFov, useTimeCompensation,
checkBlinking, fovOverride), leaving all other parameters unchanged.
---
Nitpick comments:
In `@Content.Server/_Scp/Fear/FearSystem.SoundEffects.cs`:
- Around line 83-87: The StopBreathing override omits calling the base
implementation; add a call to base.StopBreathing(ent) (as StartBreathing already
does) before or after stopping the audio stream in StopBreathing to maintain
consistency and future-proof behavior; update the StopBreathing method that
manipulates ent.Comp.BreathingAudioStream and calls DirtyField(ent, ent.Comp,
nameof(FearActiveSoundEffectsComponent.BreathingAudioStream)) to also invoke
base.StopBreathing(ent).
In `@Content.Shared/_Scp/Proximity/ProximitySystem.cs`:
- Around line 273-274: The IsNearby<T> call allocates a lambda via .Any(...);
replace the LINQ Any usage with an explicit loop over the IEnumerable returned
by _lookup.GetEntitiesInRange<T>(Transform(uid).Coordinates, range,
LookupFlags.Uncontained | LookupFlags.Approximate) and call IsRightType(uid, e,
level, out _) for each entity, returning true immediately when IsRightType
returns true and false after the loop; this removes the per-call delegate
allocation while preserving the same behavior and parameters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f0ca147d-32cf-4820-93e5-933dd3b8cd4f
📒 Files selected for processing (48)
Content.Client/_Scp/Blinking/BlinkingSystem.csContent.Client/_Scp/Fear/FearSystem.csContent.Client/_Scp/Shaders/Common/Grain/GrainOverlaySystem.csContent.Client/_Scp/Shaders/Common/Vignette/VignetteOverlaySystem.csContent.Server/_Scp/Fear/FearSystem.EntityEffects.csContent.Server/_Scp/Fear/FearSystem.Gameplay.csContent.Server/_Scp/Fear/FearSystem.SoundEffects.csContent.Server/_Scp/Fear/FearSystem.Traits.csContent.Server/_Scp/Fear/FearSystem.csContent.Shared/_Scp/Blinking/SharedBlinkingSystem.EyeClosing.csContent.Shared/_Scp/Blinking/SharedBlinkingSystem.csContent.Shared/_Scp/Fear/Components/ActiveCloseFearComponent.csContent.Shared/_Scp/Fear/Components/ActiveFearFallOffComponent.csContent.Shared/_Scp/Fear/Components/FearActiveSoundEffectsComponent.csContent.Shared/_Scp/Fear/Components/FearComponent.csContent.Shared/_Scp/Fear/Components/FearSourceComponent.csContent.Shared/_Scp/Fear/Components/Fears/HemophobiaComponent.csContent.Shared/_Scp/Fear/Components/Traits/FearFaintingComponent.csContent.Shared/_Scp/Fear/Components/Traits/FearStuporComponent.csContent.Shared/_Scp/Fear/Systems/SharedFearSystem.CloseFear.csContent.Shared/_Scp/Fear/Systems/SharedFearSystem.Fears.csContent.Shared/_Scp/Fear/Systems/SharedFearSystem.Gameplay.csContent.Shared/_Scp/Fear/Systems/SharedFearSystem.Helpers.csContent.Shared/_Scp/Fear/Systems/SharedFearSystem.SoundEffects.csContent.Shared/_Scp/Fear/Systems/SharedFearSystem.Traits.csContent.Shared/_Scp/Fear/Systems/SharedFearSystem.csContent.Shared/_Scp/Helpers/CollectionPool.csContent.Shared/_Scp/Proximity/ActiveProximityTargetComponent.csContent.Shared/_Scp/Proximity/ProximityReceiverComponent.csContent.Shared/_Scp/Proximity/ProximitySystem.csContent.Shared/_Scp/Scp173/SharedScp173System.csContent.Shared/_Scp/Shaders/Grain/GrainOverlayComponent.csContent.Shared/_Scp/Shaders/SharedShaderStrengthSystem.csContent.Shared/_Scp/Shaders/Vignette/VignetteOverlayComponent.csContent.Shared/_Scp/Watching/EyeWatchingSystem.API.Base.csContent.Shared/_Scp/Watching/EyeWatchingSystem.API.Watched.csContent.Shared/_Scp/Watching/EyeWatchingSystem.API.Watching.csContent.Shared/_Scp/Watching/EyeWatchingSystem.csContent.Shared/_Scp/Watching/WatchingTargetComponent.csResources/Prototypes/_Scp/Roles/Jobs/Administration/security_commander.ymlResources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/external_administrative_zone_commandant.ymlResources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/external_administrative_zone_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/junior_external_administrative_zone_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/senior_external_administrative_zone_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/heavy_containment_zone_commandant.ymlResources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/heavy_containment_zone_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/junior_heavy_containment_zone_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/senior_heavy_containment_zone_officer.yml
💤 Files with no reviewable changes (2)
- Content.Server/_Scp/Fear/FearSystem.EntityEffects.cs
- Content.Server/_Scp/Fear/FearSystem.Gameplay.cs
✅ Files skipped from review due to trivial changes (5)
- Content.Client/_Scp/Fear/FearSystem.cs
- Resources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/heavy_containment_zone_commandant.yml
- Resources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/external_administrative_zone_commandant.yml
- Content.Shared/_Scp/Fear/Components/ActiveCloseFearComponent.cs
- Content.Shared/_Scp/Fear/Components/ActiveFearFallOffComponent.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- Content.Server/_Scp/Fear/FearSystem.cs
- Content.Shared/_Scp/Fear/Systems/SharedFearSystem.cs
- Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Watching.cs
Content.Shared/_Scp/Fear/Components/Traits/FearFaintingComponent.cs
Outdated
Show resolved
Hide resolved
coderabbitai
bot
added
🪟 Интерфейс 🪟
and removed
🛠️ Есть C# 🛠️
📦 YAML | Прототип 📦
⚖️ Ребаланс ⚖️
labels
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
Content.Server/_Scp/Scp939/Scp939System.Actions.cs (1)
101-118:Identity.Nameлучше вычислить доforeach.Имя говорящего не зависит от конкретного
scp, а сейчас одно и то же resolution повторяется для каждой найденной 939 в горячем пути.♻️ Вариант правки
if (TryComp<TTSComponent>(ent, out var ttsComponent)) voicePrototype = ttsComponent.VoicePrototypeId; + var username = Identity.Name(ent, EntityManager); + foreach (var scp in scp939Set.Value) { if (!_examine.InRangeUnOccluded(ent, scp)) continue; @@ - var username = Identity.Name(ent, EntityManager); scp.Comp.RememberedMessages.TryAdd(message, new(username, voicePrototype)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Server/_Scp/Scp939/Scp939System.Actions.cs` around lines 101 - 118, Compute the speaker name once before iterating scp939Set instead of calling Identity.Name(ent, EntityManager) inside the foreach; move the username = Identity.Name(ent, EntityManager) out of the loop (above the foreach that iterates scp in scp939Set.Value) and reuse that local variable when adding to scp.Comp.RememberedMessages to avoid redundant resolution in the hot path.Content.Client/_Scp/Scp939/Scp939HudSystem.cs (1)
255-266: Проверьте необходимостьAsNullable()при вызовеSetColor.На строке 262 используется
ent.AsNullable()при вызове_sprite.SetColor. Если метод принимаетEntity<SpriteComponent>напрямую, преобразование в nullable может быть избыточным. Убедитесь, что сигнатура метода требует именноEntity<SpriteComponent>?.#!/bin/bash # Поиск сигнатуры SetColor в SpriteSystem ast-grep --pattern 'public $_ SetColor($$$)'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Client/_Scp/Scp939/Scp939HudSystem.cs` around lines 255 - 266, In RestoreCachedBaseAlphas iterate over CachedBaseAlphas and call _sprite.SetColor using the entity directly if the SetColor signature accepts Entity<SpriteComponent> (not nullable); verify the actual SetColor overload on SpriteSystem (or _sprite) and remove the unnecessary ent.AsNullable() call if it expects a non-nullable Entity<SpriteComponent>, or alternatively change the SetColor parameter to accept Entity<SpriteComponent>? or perform a null-check before calling if nullable is required; ensure usage around ent.Comp.Color.WithAlpha(baseAlpha) remains valid after adjusting nullability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Content.Shared/_Scp/Scp939/ActiveScp939VisibilityComponent.cs`:
- Around line 5-9: VisibilityAcc is currently marked AutoNetworkedField causing
server deltas to overwrite client-side animation accumulation (see
ActiveScp939VisibilityComponent.VisibilityAcc and
Scp939VisibilityComponent.InitialVisibilityAcc); remove the AutoNetworkedField
network attribute (and/or DataField network wiring) so VisibilityAcc becomes
purely client-side (or move it into a client-only partial component), leaving
the configuration fields (HideTime/MinValue/MaxValue) as networked if needed;
alternatively implement a separate networked reset signal instead of syncing the
accumulator directly.
---
Nitpick comments:
In `@Content.Client/_Scp/Scp939/Scp939HudSystem.cs`:
- Around line 255-266: In RestoreCachedBaseAlphas iterate over CachedBaseAlphas
and call _sprite.SetColor using the entity directly if the SetColor signature
accepts Entity<SpriteComponent> (not nullable); verify the actual SetColor
overload on SpriteSystem (or _sprite) and remove the unnecessary
ent.AsNullable() call if it expects a non-nullable Entity<SpriteComponent>, or
alternatively change the SetColor parameter to accept Entity<SpriteComponent>?
or perform a null-check before calling if nullable is required; ensure usage
around ent.Comp.Color.WithAlpha(baseAlpha) remains valid after adjusting
nullability.
In `@Content.Server/_Scp/Scp939/Scp939System.Actions.cs`:
- Around line 101-118: Compute the speaker name once before iterating scp939Set
instead of calling Identity.Name(ent, EntityManager) inside the foreach; move
the username = Identity.Name(ent, EntityManager) out of the loop (above the
foreach that iterates scp in scp939Set.Value) and reuse that local variable when
adding to scp.Comp.RememberedMessages to avoid redundant resolution in the hot
path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 27db1b94-2ffe-4978-8ef2-38c1acf27ddd
📒 Files selected for processing (12)
Content.Client/_Scp/Blinking/BlinkingSystem.csContent.Client/_Scp/Scp939/Scp939HudSystem.csContent.Client/_Scp/Scp939/Scp939ResetAlphaOverlay.csContent.Client/_Scp/Scp939/Scp939SetAlphaOverlay.csContent.Server/_Scp/Scp939/Scp939System.Actions.csContent.Server/_Scp/Scp939/Scp939System.Visibility.csContent.Server/_Scp/Scp939/Scp939System.csContent.Shared/_Scp/Blinking/SharedBlinkingSystem.csContent.Shared/_Scp/Scp939/ActiveScp939VisibilityComponent.csContent.Shared/_Scp/Scp939/Scp939Component.csContent.Shared/_Scp/Scp939/Scp939VisabilityComponent.csResources/Prototypes/_Scp/Alerts/alerts.yml
✅ Files skipped from review due to trivial changes (1)
- Resources/Prototypes/_Scp/Alerts/alerts.yml
coderabbitai
bot
added
🛠️ Есть C# 🛠️
📦 YAML | Прототип 📦
and removed
🪟 Интерфейс 🪟
labels
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Base.cs (1)
252-253:⚠️ Potential issue | 🟠 MajorПоднимайте
CanSeeAttemptEventпоref.На Line 253 событие отправляется по значению, поэтому изменения
Blindиз подписчиков не попадут обратно в локальныйcanSeeAttempt. Тогда проверка на Line 255 может пропустить ослепление или отмену видимости.Предлагаемое исправление
var canSeeAttempt = new CanSeeAttemptEvent(); - RaiseLocalEvent(viewer, canSeeAttempt); + RaiseLocalEvent(viewer, ref canSeeAttempt);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Base.cs` around lines 252 - 253, Перенесите вызов RaiseLocalEvent так, чтобы CanSeeAttemptEvent передавался по ссылке, чтобы изменения подписчиков (например поле Blind) применялись к локальной переменной canSeeAttempt перед последующей проверкой; конкретно измените вызов RaiseLocalEvent(viewer, canSeeAttempt) на вариант, передающую canSeeAttempt по ref, и убедитесь, что обработчики и сигнатуры RaiseLocalEvent поддерживают прием события по ссылке, чтобы проверка после вызова (использующая canSeeAttempt) учитывала изменения.
🧹 Nitpick comments (1)
Content.Client/_Scp/Scp939/Scp939HudSystem.cs (1)
30-33: Рассмотрите инкапсуляциюCachedBaseAlphas.
CachedBaseAlphasобъявлен какinternalс модификатором доступа к изменяемомуList. Если оверлеи (Scp939SetAlphaOverlay,Scp939ResetAlphaOverlay) напрямую модифицируют эту коллекцию, это создаёт неявную связанность. Для повышения ясности можно добавить методыCacheBaseAlpha(Entity<SpriteComponent> ent, float alpha)иClearCachedAlphas()вместо прямого доступа.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Client/_Scp/Scp939/Scp939HudSystem.cs` around lines 30 - 33, CachedBaseAlphas is exposed as an internal mutable List causing implicit coupling; make it private and add two methods on Scp939HudSystem: CacheBaseAlpha(Entity<SpriteComponent> ent, float alpha) which appends the tuple to the internal list, and ClearCachedAlphas() which clears it, then update Scp939SetAlphaOverlay and Scp939ResetAlphaOverlay to call these methods instead of accessing CachedBaseAlphas directly (optionally provide a read-only snapshot getter if overlays need enumeration).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Content.Shared/_Scp/Fear/Systems/SharedFearSystem.Gameplay.cs`:
- Around line 44-50: Компонент ActiveFearFallOffComponent не всегда получает
FallOffNextCheckTime, потому что SetNextFallOffTime вызывается только в
OnFallOffMapInit; добавьте обработку при runtime-добавлении компонента —
подпишитесь на событие ComponentAdd для ActiveFearFallOffComponent и внутри
обработчика вызовите SetNextFallOffTime(ent) (альтернативно — вызвать
SetNextFallOffTime сразу после EnsureComp в ManageFallOff), чтобы гарантированно
инициализировать FallOffNextCheckTime и избежать немедленного триггера в
обработчике MoveEvent.
In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Base.cs`:
- Around line 76-99: The method TryGetAllEntitiesVisibleTo currently appends to
potentialWatchers and uses its Count to determine success, which can leave stale
entries from prior calls (and on early return when Resolve fails). Fix by
clearing potentialWatchers at the start of TryGetAllEntitiesVisibleTo (call
potentialWatchers.Clear() before the Resolve(ent.Owner, ref ent.Comp) check) so
the list is empty for every invocation; then proceed with searchSet.Clear(),
_lookup.GetEntitiesInRange(...), the proximity check IsInProximity(...) and
adding to potentialWatchers as before.
---
Duplicate comments:
In `@Content.Shared/_Scp/Watching/EyeWatchingSystem.API.Base.cs`:
- Around line 252-253: Перенесите вызов RaiseLocalEvent так, чтобы
CanSeeAttemptEvent передавался по ссылке, чтобы изменения подписчиков (например
поле Blind) применялись к локальной переменной canSeeAttempt перед последующей
проверкой; конкретно измените вызов RaiseLocalEvent(viewer, canSeeAttempt) на
вариант, передающую canSeeAttempt по ref, и убедитесь, что обработчики и
сигнатуры RaiseLocalEvent поддерживают прием события по ссылке, чтобы проверка
после вызова (использующая canSeeAttempt) учитывала изменения.
---
Nitpick comments:
In `@Content.Client/_Scp/Scp939/Scp939HudSystem.cs`:
- Around line 30-33: CachedBaseAlphas is exposed as an internal mutable List
causing implicit coupling; make it private and add two methods on
Scp939HudSystem: CacheBaseAlpha(Entity<SpriteComponent> ent, float alpha) which
appends the tuple to the internal list, and ClearCachedAlphas() which clears it,
then update Scp939SetAlphaOverlay and Scp939ResetAlphaOverlay to call these
methods instead of accessing CachedBaseAlphas directly (optionally provide a
read-only snapshot getter if overlays need enumeration).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a7004964-37db-4c8b-9c60-f0c68b975fd1
📒 Files selected for processing (9)
Content.Client/_Scp/Scp939/Scp939HudSystem.csContent.Server/_Scp/Scp939/Scp939System.Visibility.csContent.Shared/_Scp/Fear/Components/Traits/FearFaintingComponent.csContent.Shared/_Scp/Fear/Systems/SharedFearSystem.Fears.csContent.Shared/_Scp/Fear/Systems/SharedFearSystem.Gameplay.csContent.Shared/_Scp/Helpers/CollectionPool.csContent.Shared/_Scp/Scp173/SharedScp173System.csContent.Shared/_Scp/Watching/EyeWatchingSystem.API.Base.csContent.Shared/_Scp/Watching/EyeWatchingSystem.cs
✅ Files skipped from review due to trivial changes (1)
- Content.Shared/_Scp/Scp173/SharedScp173System.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- Content.Shared/_Scp/Fear/Systems/SharedFearSystem.Fears.cs
- Content.Shared/_Scp/Watching/EyeWatchingSystem.cs
- Content.Shared/_Scp/Helpers/CollectionPool.cs
…ed events with local player variants
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Краткое описание | Short description
Высокий онлайн потребовал заняться оптимизациями самых важных систем.
Оптимизировал EyeWatching убрав LINQ и избавившись от большинства аллокаций
Ссылка на багрепорт/Предложение | Related Issue/Bug Report
Медиа (Видео/Скриншоты) | Media (Video/Screenshots)
LINQ ликвидирован
Так выглядит 30 минут наблюдения за игрой в профайлере до оптимизаций
TODO
Changelog
🆑 ThereDrD
Summary by CodeRabbit
New Features
Improvements
Balance Changes