Open
Conversation
Contributor
📝 WalkthroughWalkthroughРеализована новая система захвата SCP-объектов. Персонажи с компонентом Changes
Sequence DiagramssequenceDiagram
participant Player as Игрок/Захватывающий
participant PullSys as PullingSystem
participant ScpSys as SharedScpHoldingSystem
participant HeldEnt as Захватанная сущность
participant ActionSys as ActionSystem
Player->>PullSys: Попытка вытягивания
PullSys->>ScpSys: TryRedirectPullToScpHold()
alt Захват допустим
ScpSys->>ScpSys: CanToggleHold() проверка
ScpSys->>HeldEnt: Создать ActiveScpHoldableComponent
ScpSys->>Player: Создать ActiveScpHolderComponent
ScpSys->>ActionSys: Заблокировать ограниченные действия
ScpSys-->>PullSys: true (обработано)
else Захват не допустим
ScpSys-->>PullSys: false (обычное вытягивание)
end
sequenceDiagram
participant HeldEnt as Захватанная сущность
participant Player as Игрок (захватывающий)
participant ScpSys as SharedScpHoldingSystem
participant DoAfterSys as DoAfterSystem
participant SoundSys as AudioSystem
participant AlertSys as WorldAlertSystem
HeldEnt->>ScpSys: Нажать действие вырывания
ScpSys->>ScpSys: TryBreakOut(viaMovement)
alt Мягкий захват
ScpSys->>ScpSys: Проверить SoftEscapeAvailableAt
ScpSys-->>HeldEnt: false (слишком рано)
else Полный захват
ScpSys->>DoAfterSys: Создать DoAfter (FullBreakoutDuration)
DoAfterSys->>ScpSys: На завершение DoAfter
ScpSys->>HeldEnt: Применить импульс
ScpSys->>Player: Применить урон + паралич
ScpSys->>AlertSys: TrySpawnAlert() уведомление
AlertSys->>SoundSys: Воспроизвести звук
ScpSys->>HeldEnt: ClearHoldState()
ScpSys->>HeldEnt: Применить иммунитет на 5 сек
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Это комплексная реализация новой системы механики с множеством взаимосвязанных компонентов ECS, событий, систем (клиентская, серверная, общая), интеграцией с существующими системами (вытягивание, действия, движение, SCP-096), локализацией на двух языках и обширной конфигурацией прототипов. Требуется тщательная проверка логики захватов, переходов состояния, обработки жизненного цикла компонентов, предсказания на клиенте и интеграции с движением. Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (4 errors, 1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
coderabbitai
bot
added
🛠️ Есть C# 🛠️
📦 YAML | Прототип 📦
📚 Изменения: Локализация 📚
labels
![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: 21
🤖 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.Client/_Scp/Holding/ScpHoldingPredictionSystem.cs`:
- Around line 22-26: Replace the fixed 500ms timeout suppression with
state-based suppression: remove BlockerRespawnSuppressionDuration and the use of
_suppressedUntil and any time comparisons, and instead keep suppression until
the authoritative ScpHolderComponent for the suppressed holder either changes
its Target or the component is removed; implement this by storing
_suppressedHolder and _suppressedTarget when you start suppression and, where
the code currently checks _suppressedUntil, instead query the current
ScpHolderComponent for that holder (or its existence) and clear suppression only
when ScpHolderComponent.Target is different from _suppressedTarget or the
component no longer exists. Ensure all locations that reference
BlockerRespawnSuppressionDuration or _suppressedUntil (including the code paths
around the logic that spawns/filters the virtual blocker) are updated to use the
new state-based check.
In `@Content.Shared/_Scp/Holding/ScpHeldHandBlockerComponent.cs`:
- Around line 8-20: Компонент ScpHeldHandBlockerComponent помечен как
[NetworkedComponent] но не генерирует состояние и не маркирует поля для сетевой
синхронизации, из‑за чего предсказание/откат в ScpHoldingPredictionSystem
некорректны; добавьте атрибут [AutoGenerateComponentState] к классу
ScpHeldHandBlockerComponent и пометьте поля Target и Holder атрибутом
[AutoNetworkedField], чтобы они синхронизировались и откатывались так же, как в
аналогичных компонентах ScpHolderComponent, ScpHeldComponent и ScpHoldComponent.
In `@Content.Shared/_Scp/Holding/ScpHoldableComponent.cs`:
- Around line 9-10: The ScpHoldableComponent in Shared lacks the
NetworkedComponent attribute needed for client-side replication; add the
[NetworkedComponent] attribute to the ScpHoldableComponent declaration (the
public sealed partial class ScpHoldableComponent : Component) in Content.Shared
so the component is networked and eligible checks replicate correctly, and
ensure any required using/import for the networking attribute's namespace is
added if missing.
In `@Content.Shared/_Scp/Holding/ScpHoldRestrictedComponent.cs`:
- Around line 5-9: Добавьте атрибуты автогенерации и автосинхронизации для
сетевого компонента: пометьте класс ScpHoldRestrictedComponent атрибутом
[AutoGenerateComponentState] и пометьте поле Stage атрибутом
[AutoNetworkedField]; это гарантирует, что поле Stage будет участвовать в
сериализации/десериализации и откате состояния при предсказании для
NetworkedComponent.
In `@Content.Shared/_Scp/Holding/SharedScpHoldingSystem.Actions.cs`:
- Around line 22-40: CanToggleHold currently only validates starting a hold and
thus disagrees with TryToggleHold's actual toggle logic; update CanToggleHold to
mirror TryToggleHold's preconditions by returning true for the release case when
the holder's active target equals the provided target, false when the holder is
already holding a different target (so UI/prediction matches the PopupHolder
rejection), and otherwise perform the start-hold checks (or delegate to
CanStartHold) when the holder has no active target; ensure the method signature
and behavior follow the OnEvent -> TryDo -> CanDo -> Do pattern and reference
CanToggleHold and TryToggleHold so any callers/predictions stay consistent with
the public API.
- Around line 245-247: Вызов PopupTarget(held.Owner, "scp-hold-breakout-start")
выполняется без prediction-gating; либо оберните этот вызов в проверку
IsFirstTimePredicted (как это сделано в ShowBreakoutAttemptFeedback), либо
замените вызов на prediction-safe хелпер (например
PopupPredicted/PopupEntity-перенаправление), чтобы обеспечить one-shot UX effect
только при первой предсказанной итерации; отредактируйте функцию PopupTarget или
место вызова, чтобы использовать IsFirstTimePredicted или вызвать
соответствующий prediction-safe wrapper (см. ShowBreakoutAttemptFeedback,
PopupTarget, PopupPredicted, IsFirstTimePredicted).
In `@Content.Shared/_Scp/Holding/SharedScpHoldingSystem.Drag.cs`:
- Around line 104-112: GetSoftDragDirection currently falls back to
Transform(holderUid).LocalRotation.ToWorldVec(), which returns a parent-relative
direction and can mismatch the map-space vectors (offset, holderVelocity);
change the fallback to use the entity's world rotation instead by replacing the
LocalRotation usage with WorldRotation so the returned direction is in
world/map-space and consistent with the normalized offset and velocity checks
(symbols: GetSoftDragDirection, SoftDragSnapTolerance,
SoftDragVelocityDirectionThreshold,
Transform(holderUid).WorldRotation.ToWorldVec()).
In `@Content.Shared/_Scp/Holding/SharedScpHoldingSystem.Events.cs`:
- Around line 221-233: OnHolderBlockerDropped currently releases the holder
contribution but lets the normal drop flow continue, which can race with the
blocker being removed; after calling ReleaseHolderContribution(args.User,
ent.Comp.Target, clearIfEmpty: true) immediately stop the default drop handling
on the GettingDroppedAttemptEvent (e.g. call args.Cancel() or set args.Cancel =
true / args.Handled = true depending on the event API) so the virtual item's
drop logic does not run against the already-processed blocker; keep this change
inside OnHolderBlockerDropped right after the ReleaseHolderContribution call.
- Around line 135-155: The handlers OnHeldAttemptMobCollide and
OnHeldAttemptMobTargetCollide are currently cancelling all mob-collisions for
any ScpHeldComponent; change them to only cancel when the held is in a FullHold
state OR when the other entity is the actual holder for this held pair.
Concretely: in both OnHeldAttemptMobCollide and OnHeldAttemptMobTargetCollide,
early-return unless ent.Component.FullHold is true OR
_holderQuery.TryComp(args.OtherEntity, out var holder) && holder.Target ==
ent.Owner; keep the finer-grained logic in OnHeldPreventCollide untouched so
pair-wise holder↔held collisions still work.
In `@Content.Shared/_Scp/Holding/SharedScpHoldingSystem.Hands.cs`:
- Around line 27-69: SyncPlaceholderHands currently calls DeleteHeldHandBlockers
unconditionally and then recreates all virtual blockers; change it to reuse
existing ScpHeldHandBlockerComponent entities like SyncHolderHandBlocker does
by: first collect existing blocker entities for held.Owner (inspect components
via ScpHeldHandBlockerComponent and _placeholderIcons), compute which blockers
already match a holder in held.Comp.Holders (matching blocker.Holder ==
holderUid and blocker.Target == held.Owner) and keep them, delete only obsolete
blockers and their virtual items, then for any missing holders spawn virtual
items with _virtualItem.TrySpawnVirtualItemInHand,
EnsureComp<UnremoveableComponent> and set
ScpHeldHandBlockerComponent.Target/Holder as now done; do not unconditionally
call DeleteHeldHandBlockers and avoid dropping/respawning valid held items via
_hands.TryGetHeldItem/_hands.DoDrop unless necessary.
In `@Content.Shared/_Scp/Holding/SharedScpHoldingSystem.Restrictions.cs`:
- Around line 10-16: В методе OnHoldRestrictedActionAttempt (обработчик
ActionAttemptEvent) замените вызов _popup.PopupClient(...) на
_popup.PopupPredicted(...), чтобы использовать предсказанный вариант попапа для
предсказанных событий и избежать двойного отображения; оставьте те же аргументы
(сообщение Loc.GetString("scp-hold-action-restricted"), args.User, args.User)
при замене, как в аналогичных системах (SharedArtifactCrusherSystem,
ScpMaskSystem).
In `@Content.Shared/Interaction/SharedInteractionSystem.Blocking.cs`:
- Around line 44-47: В коде комментарии-маркеры "Fire edit start" и "Fire edit
end" (см. строку с "// Fire edit start - do not let a blocker cancel its own
shutdown refresh" и соответствующий конец блока) используют неправильный формат;
заменить эти форк-маркеры на Sunrise-формат: используйте "Sunrise-Edit" for
single-line or "Sunrise edit start" / "Sunrise edit end" for multi-line edit
blocks so они соответствуют стандарту репозитория (обновить оба места, где
встречается "Fire edit start" и "Fire edit end").
In `@Content.Shared/Movement/Pulling/Systems/PullingSystem.cs`:
- Around line 532-535: TryRedirectPullToScpHold currently runs before CanPull
and bypasses key checks and events (NeedsHands, CanInteract/distance, bound
status) and does not raise StartPullAttemptEvent / BeingPulledAttemptEvent /
PullAttemptEvent, so subscriber systems (CuffableSystem, BuckleSystem,
VentCrawSystem, CarryingSystem, AdminFrozenSystem, etc.) can't veto the action;
fix by unifying the guard logic: either have TryRedirectPullToScpHold call the
same validation and raise the same events as CanPull (including NeedsHands,
CanInteract checks and binding checks) before returning a redirect, or extract
the shared pre-pull guard into a helper (e.g., ValidateAndRaisePullAttemptEvents
or similar) and call it from both TryRedirectPullToScpHold and the original pull
path so both paths perform identical checks and raise StartPullAttemptEvent /
BeingPulledAttemptEvent / PullAttemptEvent.
In `@Resources/Locale/en-US/_strings/_scp/holding/holding.ftl`:
- Around line 13-14: Update the locale entry alerts-scp-held-desc so the player
instruction is marked as out-of-character: prefix the sentence "Move or click
this alert to try to break free." with "OOC:" (e.g., "OOC: Move or click this
alert to try to break free.") while keeping the rest of the description intact;
edit the alerts-scp-held-desc string accordingly to ensure the control hint
complies with OOC tagging guidelines.
In `@Resources/Prototypes/_Scp/Roles/Jobs/Administration/security_commander.yml`:
- Around line 36-39: В текущей конфигурации у Security Commander блок ScpHold
использует holdableWhitelist, который содержит только ClassDAppearance и потому
не покрывает весь персонал; исправьте это, расширив или заменив
holdableWhitelist в записи для Security Commander так, чтобы включить все типы
персонала (например добавить/заменить на сущности вроде ResearchAppearance,
StaffAppearance, GuardAppearance, SecurityAppearance или общий маркер
PersonnelAppearance/персонал вместо ClassDAppearance), либо удалить/исправить
whitelist на более общий критерий, чтобы соответствовать заявленному охвату
механика; изменяйте именно секцию ScpHold и поле holdableWhitelist в записи
Security Commander.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_botanist.yml`:
- Around line 26-34: В файле конфигурации ClassDBotanist удалите назначение
механики удержания: уберите или переместите блок type: ScpHold (и связанные поля
holdableWhitelist / holdableBlacklist) из class_d_botanist.yml, т.е. удалите
упоминание ScpHold у ClassDBotanist и перенесите эту фичу в правильный
scope/роль, где удержание должно быть доступно; убедитесь, что поля
holdableWhitelist и holdableBlacklist больше не присутствуют в описании
ClassDBotanist (или добавьте вместо них ссылку на корректный класс/роль,
отвечающую за ScpHold).
In
`@Resources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/heavy_containment_zone_officer.yml`:
- Around line 31-40: Конфигурация ScpHold с одинаковыми секциями
holdableWhitelist и holdableBlacklist (components: Scp096, ClassDAppearance и
blacklist: ActiveScp096Rage, ActiveScp096HeatingUp, ActiveScp096WithoutFace)
повторяется в нескольких job-файлах; вынесите её в общий шаблон или базовый
job-прототип (например: базовый прототип/компонент с именем ScpHoldBase) и
замените дублирующие блоки в файлах на ссылку/наследование этого шаблона,
сохранив возможность переопределения по нужде.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/junior_heavy_containment_zone_officer.yml`:
- Around line 30-39: В блоке конфигурации ScpHold удалите запись
ClassDAppearance из holdableWhitelist — оставьте только Scp096 в whitelist и
сохраните текущий holdableBlacklist (ActiveScp096Rage, ActiveScp096HeatingUp,
ActiveScp096WithoutFace) чтобы фокусировать механику только на SCP-096;
проверьте секцию ScpHold и поля holdableWhitelist/holdableBlacklist и уберите
только ClassDAppearance, не трогая остальные ключи.
In `@Resources/Prototypes/Actions/types.yml`:
- Around line 264-265: The comment on the upstream entry for the action type
ScpHoldRestricted uses the wrong upstream marker; replace the inline comment "#
Fire edit" with the required repository upstream marker "# Sunrise-Edit" (exact
casing) so the ScpHoldRestricted line uses the Sunrise upstream edit marker as
per guidelines.
In `@Resources/Prototypes/Entities/Mobs/Species/base.yml`:
- Around line 281-282: The upstream YAML changes use a Fire-format inline
comment instead of the required Sunrise markers; replace the Fire comment on the
modified entries (types ScpHold and ScpHoldable) with the Sunrise upstream edit
markers—wrap the edited lines with the Sunrise edit markers (e.g., use the
"Sunrise-Edit" or "Sunrise edit start" / "Sunrise edit end" pattern) so the
change follows the repository's Sunrise upstream-edit convention for the Species
base.yml entries where ScpHold and ScpHoldable are altered.
- Around line 281-282: В BaseMobSpeciesOrganic удалите запись типа ScpHold
(оставьте ScpHoldable если нужно), чтобы базовый органический прототип больше не
получал механику удержания; вместо этого назначьте компонент ScpHold только
нужным ролям через job "special" (проверьте где определяется job "special" и
добавьте/переместите тип ScpHold туда).
🪄 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: 4181336a-3b73-42ff-802a-15d03d2894e6
📒 Files selected for processing (51)
Content.Client/_Scp/Holding/ScpHoldingPredictionSystem.csContent.IntegrationTests/Tests/_Scp/ScpHoldingTest.csContent.Shared/Hands/EntitySystems/SharedHandsSystem.Drop.csContent.Shared/Interaction/SharedInteractionSystem.Blocking.csContent.Shared/Movement/Pulling/Systems/PullingSystem.csContent.Shared/_Scp/Holding/PullingSystem.ScpHolding.csContent.Shared/_Scp/Holding/ScpHeldComponent.csContent.Shared/_Scp/Holding/ScpHeldHandBlockerComponent.csContent.Shared/_Scp/Holding/ScpHoldComponent.csContent.Shared/_Scp/Holding/ScpHoldHandBlockerComponent.csContent.Shared/_Scp/Holding/ScpHoldImmuneComponent.csContent.Shared/_Scp/Holding/ScpHoldRestrictedComponent.csContent.Shared/_Scp/Holding/ScpHoldStage.csContent.Shared/_Scp/Holding/ScpHoldableComponent.csContent.Shared/_Scp/Holding/ScpHolderComponent.csContent.Shared/_Scp/Holding/ScpHoldingEvents.csContent.Shared/_Scp/Holding/SharedScpHoldingSystem.Actions.csContent.Shared/_Scp/Holding/SharedScpHoldingSystem.Drag.csContent.Shared/_Scp/Holding/SharedScpHoldingSystem.Events.csContent.Shared/_Scp/Holding/SharedScpHoldingSystem.Feedback.csContent.Shared/_Scp/Holding/SharedScpHoldingSystem.Hands.csContent.Shared/_Scp/Holding/SharedScpHoldingSystem.Restrictions.csContent.Shared/_Scp/Holding/SharedScpHoldingSystem.State.csContent.Shared/_Scp/Holding/SharedScpHoldingSystem.csContent.Shared/_Scp/Scp096/Main/Components/Scp096Component.csContent.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.Holding.csContent.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.Rage.csContent.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.WithoutFace.csContent.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.csResources/Locale/en-US/_strings/_scp/holding/holding.ftlResources/Locale/ru-RU/_strings/_scp/holding/holding.ftlResources/Prototypes/Actions/types.ymlResources/Prototypes/Entities/Mobs/Species/base.ymlResources/Prototypes/_Scp/Actions/scp096.ymlResources/Prototypes/_Scp/Alerts/holding.ymlResources/Prototypes/_Scp/Entities/Mobs/Player/Scp/Main/scp096.ymlResources/Prototypes/_Scp/Entities/StatusEffects/holding.ymlResources/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/field_medical_specialist.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/LowAccessPersonnel/class_d.ymlResources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_botanist.ymlResources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_cook.ymlResources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_janitor.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
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
♻️ Duplicate comments (5)
Resources/Prototypes/_Scp/Roles/Jobs/Administration/security_commander.yml (1)
36-39:⚠️ Potential issue | 🟠 MajorРасширьте Security Commander до всего персонала.
ClassDAppearanceоставляет удержание только для D-класса, хотя для командного контура заявлено удержание всего персонала. Используйте общий humanoid whitelist и тот же SCP blacklist, что у CommandantSquad.Предлагаемое исправление
- type: ScpHolder holdableWhitelist: components: - - ClassDAppearance + - HumanoidAppearance + holdableBlacklist: + components: + - Scp🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Prototypes/_Scp/Roles/Jobs/Administration/security_commander.yml` around lines 36 - 39, The ScpHolder config for Security Commander currently limits holds to ClassDAppearance (ScpHolder with holdableWhitelist.components including ClassDAppearance); change it to use the general humanoid whitelist and apply the same SCP blacklist used by CommandantSquad: replace the holdableWhitelist.components entry that references ClassDAppearance with the shared humanoid whitelist identifier (the same whitelist key used by CommandantSquad) and ensure the ScpHolder includes the identical scp blacklist/settings from CommandantSquad so Security Commander holds all personnel rather than only D-class.Resources/Prototypes/Entities/Mobs/Species/base.yml (1)
281-282:⚠️ Potential issue | 🔴 CriticalУберите
ScpHolderиз базового органического прототипа.Line 281 выдаёт механику держателя всем органическим мобам, обходя job
specialи ролевые ограничения PR. Оставьте здесь только holdable-маркер, если цель — сделать органиков доступными для удержания.Предлагаемое исправление
- - type: ScpHolder # TODO: Убрать перед мержем - - type: ScpHoldable + - type: ScpHoldable # Sunrise-EditAs per coding guidelines: "This repository's repository name is
sunrise-station, so use theSunriseprefix, the_Sunriseproject folder, andSunrise-Edit/Sunrise edit start/endmarkers for unavoidable upstream edits."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Prototypes/Entities/Mobs/Species/base.yml` around lines 281 - 282, Удалите маркер типа ScpHolder из базового органического прототипа и оставьте только ScpHoldable (убирается строка с "type: ScpHolder" и не трогать/восстановить "type: ScpHoldable"), чтобы не давать всем органикам механики держателя в обход job `special` и ролевых ограничений; если это неизбежный апстрим-фикс — поместите изменения в проектную папку _Sunrise, используйте префикс Sunrise для необходимых идентификаторов и обрамите правку метками "Sunrise-Edit start" / "Sunrise-Edit end".Content.Shared/_Scp/Holding/Components/ScpHoldRestrictedComponent.cs (1)
5-9:⚠️ Potential issue | 🟠 MajorСинхронизируйте
Stageу сетевого компонента.Сейчас компонент помечен как
NetworkedComponent, но его состояние не генерируется иStageне попадает в auto-networked state. Это может рассинхронизировать ограничения действий на клиенте и сервере.Предлагаемое исправление
-[RegisterComponent, NetworkedComponent] +[RegisterComponent, NetworkedComponent, AutoGenerateComponentState] public sealed partial class ScpHoldRestrictedComponent : Component { - [DataField] + [DataField, AutoNetworkedField] public ScpHoldStage Stage = ScpHoldStage.Full; }As per coding guidelines, "Apply the
[AutoGenerateComponentState]attribute to components used in prediction" and "Ensure all predicted fields in networked components are marked with[AutoNetworkedField]".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_Scp/Holding/Components/ScpHoldRestrictedComponent.cs` around lines 5 - 9, The networked component ScpHoldRestrictedComponent currently has a predicted field Stage that isn't included in auto-networked state; add the [AutoGenerateComponentState] attribute to the ScpHoldRestrictedComponent class and mark the Stage field with [AutoNetworkedField] so its value is included in generated component state for prediction and syncing between client and server.Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_botanist.yml (1)
26-34:⚠️ Potential issue | 🟠 Major
ClassDBotanistвсе еще не должен получатьScpHolder.Это расширяет механику удержания на low-access роль вне заявленного scope. Удержание SCP-096 должно оставаться у ОСН/целевой роли процедуры.
Предлагаемое исправление
- - type: ScpHolder - holdableWhitelist: - components: - - Scp096 - holdableBlacklist: - components: - - ActiveScp096Rage - - ActiveScp096HeatingUp - - ActiveScp096WithoutFace🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_botanist.yml` around lines 26 - 34, В файле конфигурации роли ClassDBotanist удалите или отключите блок типа ScpHolder (строки с "type: ScpHolder", "holdableWhitelist" и "holdableBlacklist") чтобы ClassDBotanist больше не получает возможность удерживать SCP-096; найдите упоминание "ClassDBotanist" и уберите весь связанный подблок ScpHolder/holdableWhitelist/holdableBlacklist так, чтобы механика удержания оставалась только у целевых SCP/ОСН ролей.Resources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/junior_heavy_containment_zone_officer.yml (1)
30-39:⚠️ Potential issue | 🟠 MajorУберите
ClassDAppearanceиз whitelist.Для ОСН здесь должен быть доступ к удержанию SCP-096, а
ClassDAppearanceрасширяет механику на Class D/похожие цели.Предлагаемое исправление
- type: ScpHolder holdableWhitelist: components: - Scp096 - - ClassDAppearance holdableBlacklist: components: - ActiveScp096Rage - ActiveScp096HeatingUp - ActiveScp096WithoutFace🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/junior_heavy_containment_zone_officer.yml` around lines 30 - 39, В блоке конфигурации для ScpHolder удалите ClassDAppearance из списка holdableWhitelist: оставьте только Scp096 в holdableWhitelist компонентов, чтобы роль могла удерживать исключительно SCP-096; убедитесь, что запись holdableWhitelist в сущности ScpHolder больше не содержит ClassDAppearance (остальные blacklist-элементы, например ActiveScp096Rage/HeatingUp/WithoutFace, оставьте без изменений).
🤖 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.Client/_Scp/Holding/ScpHoldingSystem.cs`:
- Around line 199-229: The server-side path currently treats any
VirtualItemComponent with a matching BlockingEntity as an authoritative blocker
even if it lacks the marker component; update the loop so you check
_blockerQuery.HasComp(heldItem) for server-side holders before assigning
authoritativeBlocker (i.e. move or duplicate the _blockerQuery.HasComp check
into the !IsClientSide branch), keeping the existing behavior for
predictedBlocker and QueueDel; ensure references to _virtualItemQuery.TryComp,
VirtualItemComponent.BlockingEntity, IsClientSide, _blockerQuery.HasComp,
authoritativeBlocker, predictedBlocker and QueueDel are used to locate and apply
this change.
In `@Content.IntegrationTests/Tests/_Scp/ScpHoldingTwoClientCombatTest.cs`:
- Around line 435-459: The two local overloads named RunTicks that loop awaiting
WaitRunTicks(1) should be replaced with the standard test helpers that guarantee
server/client alignment: remove these custom RunTicks methods and instead call
the framework helpers (e.g. RunTicksSync(...) to advance server and all client
instances together and then SyncTicks(targetDelta: ...) after critical steps)
wherever the custom RunTicks was used; ensure you reference WaitRunTicks only
inside those helpers and use RunTicksSync and SyncTicks(targetDelta: ...) to
prevent cross-instance desync during replication/combat assertions.
- Around line 44-52: The test creates raw ServerIntegrationInstance and
ClientIntegrationInstance with Pool = false and using-var instead of the pooled
lifecycle; replace this by acquiring a pooled pair via
PoolManager.GetServerClient(CreateServerOptions/CreateClientOptions) using
"await using var pair = await PoolManager.GetServerClient(...)" and call
pair.CleanReturnAsync() (or explicit await pair.CleanReturnAsync() before
disposal) so the server/client are returned to the pool, or if you must keep
Pool = false add an explicit justification comment and implement deterministic
async cleanup for ServerIntegrationInstance and ClientIntegrationInstance
(ensure Connect and RunTicks usages remain unchanged); update all similar blocks
referenced (lines ~251-257 and ~382-432) to follow the same pattern.
- Around line 80-96: Move all mutations/side-effects (SpawnEntity calls,
sEntMan.EnsureComponent<ScpHolderComponent>(...),
sTransform.SetCoordinates(...), and any creation/positioning or calls like
GetCombatToggleAction(...)) out of WaitAssertion and into a WaitPost block so
they only run once and cannot be repeated on retry; leave only checks/assertions
(Assert.Multiple, Assert.That on targetSession.AttachedEntity and
holderSession.AttachedEntity, reading sTarget/sHolder values for assertions)
inside WaitAssertion. Ensure SpawnEntity is called once in WaitPost before the
WaitAssertion that asserts attached entities, and that WaitAssertion only
verifies state without performing mutations.
In `@Content.Server/_Scp/Holding/ScpHoldingSystem.cs`:
- Around line 17-23: In the OnHeldStateShutdown method, the foreach loop
iterates directly over held.Comp.Holders while the RemComp call triggers a chain
of events that removes elements from this same collection, which can cause
elements to be skipped during iteration. Fix this by creating a snapshot of the
Holders collection before iterating (for example, by calling ToList() on the
collection) and then iterate over that snapshot instead of the original
collection, ensuring all holders are properly processed even as the original
collection is modified.
In `@Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Actions.cs`:
- Around line 76-193: CanToggleHold (and CanStartHold / CanPassHoldAttempt)
currently produce side effects (Popup calls and event dispatches) which must be
removed so these methods are pure boolean checks; refactor by moving all
user-feedback and event-raising into the Try... layer (e.g., implement
TryToggleHold/TryStartHold that call CanToggleHold/CanStartHold then perform
Popup(holder, ...) and dispatch the hold attempt events only on success/failure
there), ensure CanToggleHold only computes and returns true/false and does not
call Popup, Try...must not call Can... in a way that triggers handlers, and
update related locations referenced by the review (around the Can... usages at
the other noted ranges) to follow OnEvent -> TryDo -> CanDo -> Do pattern and
avoid mutating components or raising events inside any Can... method.
In
`@Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.BreakoutAttempt.cs`:
- Around line 72-76: В cancel-ветке обработчика BreakoutAttempt (в
SharedScpHoldingSystem.BreakoutAttempt.cs) пометить отменённый do-after как
обработанный: перед вызовом return после Popup(ent,
"scp-hold-breakout-interrupted") установить args.Handled = true, чтобы
отменённое событие не оставалось доступным для дальнейшей обработки;
гарантировать, что логика для args.Cancelled и для успешного ветвления ведёт
себя консистентно.
In `@Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Restrictions.cs`:
- Around line 12-14: The shared predicted handler is calling the one-shot popup
directly (e.g., _popup.PopupClient inside the ActionAttemptEvent path) which can
cause duplicate popups during prediction replay; replace direct calls with the
shared predicted-safe mechanism by either using the Popup(...) hook or the
predicted helper (e.g., PopupPredicted/PlayPredicted) or guard the call with
IsFirstTimePredicted/ ApplyingState so the UX side-effect runs only once. Locate
uses of _popup.PopupClient in SharedScpHoldingSystem.Restrictions (and lines
~65-71) and refactor them to call the predicted popup helper or wrap them with
an IsFirstTimePredicted check.
- Around line 28-31: В обработчике OnRestrictRemove (удаление
ScpHoldRestrictedComponent по ComponentShutdown) не нужно вызывать
ValidateActions() — это приводит к повторному применению отключения действий и
оставляет их disabled после удаления компонента; уберите вызов ValidateActions()
из OnRestrictRemove или замените его на проверку реального состояния удержания
(например, проверять наличие компонента удержания/флага удержания у владельца) и
вызывать ValidateActions() только когда удержание действительно завершается
(функции/события, которые обрабатывают реальный релиз), сохранив ссылки на
ScpHoldRestrictedComponent, ComponentShutdown и метод ValidateActions для
навигации по коду.
In `@Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.State.cs`:
- Around line 76-85: EnsureHeldState currently updates
ActiveScpHoldableComponent.RequiredHolderCount without marking the component
dirty for network prediction; after setting held.RequiredHolderCount =
GetRequiredHolderCount(target) call Dirty(target, held) so the change is
replicated and prediction/reconciliation works correctly, and apply the same fix
wherever RequiredHolderCount is modified (the other block referenced in the diff
that updates RequiredHolderCount) to call Dirty(uid, comp) after each
assignment.
In `@Content.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.Holding.cs`:
- Around line 45-56: held.Holders can contain stale UIDs so before calling
ApplyHoldBreakoutEffects you must filter to only the current active holder(s):
check that each HolderUid still has an ActiveScpHolderComponent whose Target
equals ent.Owner and skip any that don’t match; update the creation of the
holders array (or the second loop) to call
_entityManager.TryGetComponent/HasComponent for ActiveScpHolderComponent and
compare ActiveScpHolderComponent.Target == ent.Owner, only then compute position
via _transform.GetWorldPosition(holderUid) and call ApplyHoldBreakoutEffects for
that holder.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/junior_external_administrative_zone_officer.yml`:
- Around line 30-36: The new ScpHolder block (the added fragment containing
ScpHolder with holdableWhitelist and holdableBlacklist entries, including
HumanoidAppearance and Scp) lacks the required Sunrise fork marker; insert a
single-line comment "# Sunrise-Edit" immediately above the ScpHolder mapping to
mark this YAML change as a Sunrise edit, ensuring the marker follows the
coding-guideline format for YAML/FTL/Python/Shell files and does not alter the
existing keys (ScpHolder, holdableWhitelist, holdableBlacklist).
In `@Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_janitor.yml`:
- Around line 23-31: В файле роли ClassDJanitor удалите или откатите блок
ScpHolder (включая поля holdableWhitelist/holdableBlacklist и ссылку на Scp096),
потому что low-access роль не должна иметь возможность удерживать SCP-096; либо
переместите этот ScpHolder-конфиг в правильную роль с ОСН-скоупом.
Обновите/проверьте упоминания Scp096 в полях
holdableWhitelist/holdableBlacklist, чтобы гарантировать, что только роли с
корректным scope (ОСН) содержат эту возможность.
In `@Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d.yml`:
- Around line 21-29: В файле ClassD удалите или отключите блок типа ScpHolder
(ключи type: ScpHolder, holdableWhitelist: components: - Scp096), т.к. он даёт
обычным ClassD возможность удерживать SCP-096; вместо этого перенесите либо
добавьте аналогичный ScpHolder-блок только в файлы ролей командного состава и
ОСН (например роли, которые отвечают за командный состав/ChaosInsurgency) или
ограничьте доступ в этом блоке конкретными флагами/ролью, чтобы только эти
специализированные роли могли удерживать Scp096.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/heavy_containment_zone_officer.yml`:
- Around line 31-40: The added ScpHolder block (the block starting with "type:
ScpHolder" containing holdableWhitelist/holdableBlacklist and components like
Scp096 and ClassDAppearance) lacks the required Sunrise fork marker; insert a
single-line comment "# Sunrise-Edit" immediately above that ScpHolder block so
the YAML change is clearly marked as a Sunrise fork edit, ensuring the marker
uses exact capitalization and spacing.
---
Duplicate comments:
In `@Content.Shared/_Scp/Holding/Components/ScpHoldRestrictedComponent.cs`:
- Around line 5-9: The networked component ScpHoldRestrictedComponent currently
has a predicted field Stage that isn't included in auto-networked state; add the
[AutoGenerateComponentState] attribute to the ScpHoldRestrictedComponent class
and mark the Stage field with [AutoNetworkedField] so its value is included in
generated component state for prediction and syncing between client and server.
In `@Resources/Prototypes/_Scp/Roles/Jobs/Administration/security_commander.yml`:
- Around line 36-39: The ScpHolder config for Security Commander currently
limits holds to ClassDAppearance (ScpHolder with holdableWhitelist.components
including ClassDAppearance); change it to use the general humanoid whitelist and
apply the same SCP blacklist used by CommandantSquad: replace the
holdableWhitelist.components entry that references ClassDAppearance with the
shared humanoid whitelist identifier (the same whitelist key used by
CommandantSquad) and ensure the ScpHolder includes the identical scp
blacklist/settings from CommandantSquad so Security Commander holds all
personnel rather than only D-class.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_botanist.yml`:
- Around line 26-34: В файле конфигурации роли ClassDBotanist удалите или
отключите блок типа ScpHolder (строки с "type: ScpHolder", "holdableWhitelist" и
"holdableBlacklist") чтобы ClassDBotanist больше не получает возможность
удерживать SCP-096; найдите упоминание "ClassDBotanist" и уберите весь связанный
подблок ScpHolder/holdableWhitelist/holdableBlacklist так, чтобы механика
удержания оставалась только у целевых SCP/ОСН ролей.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/junior_heavy_containment_zone_officer.yml`:
- Around line 30-39: В блоке конфигурации для ScpHolder удалите ClassDAppearance
из списка holdableWhitelist: оставьте только Scp096 в holdableWhitelist
компонентов, чтобы роль могла удерживать исключительно SCP-096; убедитесь, что
запись holdableWhitelist в сущности ScpHolder больше не содержит
ClassDAppearance (остальные blacklist-элементы, например
ActiveScp096Rage/HeatingUp/WithoutFace, оставьте без изменений).
In `@Resources/Prototypes/Entities/Mobs/Species/base.yml`:
- Around line 281-282: Удалите маркер типа ScpHolder из базового органического
прототипа и оставьте только ScpHoldable (убирается строка с "type: ScpHolder" и
не трогать/восстановить "type: ScpHoldable"), чтобы не давать всем органикам
механики держателя в обход job `special` и ролевых ограничений; если это
неизбежный апстрим-фикс — поместите изменения в проектную папку _Sunrise,
используйте префикс Sunrise для необходимых идентификаторов и обрамите правку
метками "Sunrise-Edit start" / "Sunrise-Edit end".
🪄 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: c112d4df-81d1-44b7-b0b5-a9d2d8fb7314
📒 Files selected for processing (45)
Content.Client/_Scp/Holding/ScpHoldingSystem.Feedback.csContent.Client/_Scp/Holding/ScpHoldingSystem.csContent.IntegrationTests/Tests/_Scp/ScpHoldingTest.csContent.IntegrationTests/Tests/_Scp/ScpHoldingTwoClientCombatTest.csContent.Server/_Scp/Holding/ScpHoldingSystem.Feedback.csContent.Server/_Scp/Holding/ScpHoldingSystem.csContent.Shared/_Scp/Holding/Compatibility/PullingSystem.ScpHolding.csContent.Shared/_Scp/Holding/Components/ActiveScpHoldableComponent.csContent.Shared/_Scp/Holding/Components/ActiveScpHolderComponent.csContent.Shared/_Scp/Holding/Components/ActiveStateScpHoldableFullHoldComponent.csContent.Shared/_Scp/Holding/Components/ActiveStateScpHolderSlowdownComponent.csContent.Shared/_Scp/Holding/Components/ScpBreakoutAttemptComponent.csContent.Shared/_Scp/Holding/Components/ScpHeldHandBlockerComponent.csContent.Shared/_Scp/Holding/Components/ScpHoldHandBlockerComponent.csContent.Shared/_Scp/Holding/Components/ScpHoldImmuneComponent.csContent.Shared/_Scp/Holding/Components/ScpHoldRestrictedComponent.csContent.Shared/_Scp/Holding/Components/ScpHoldableComponent.csContent.Shared/_Scp/Holding/Components/ScpHolderComponent.csContent.Shared/_Scp/Holding/ScpHoldingEvents.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Actions.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.BreakoutAttempt.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Drag.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Feedback.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Hands.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Lifecycle.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Restrictions.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.State.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.csContent.Shared/_Scp/Scp096/Main/Components/Scp096Component.csContent.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.Holding.csResources/Prototypes/Entities/Mobs/Species/base.ymlResources/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/field_medical_specialist.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/LowAccessPersonnel/class_d.ymlResources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_botanist.ymlResources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_cook.ymlResources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_janitor.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
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Collaborator
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 30
♻️ Duplicate comments (12)
Resources/Prototypes/_Scp/Roles/Jobs/Administration/security_commander.yml (1)
36-39:⚠️ Potential issue | 🟠 Major
holdableWhitelistне соответствует охвату фичи.По целям PR командный состав должен иметь возможность удерживать весь персонал, однако
ScpHolderу Security Commander ограничен толькоClassDAppearance. Расширьте whitelist (например, наHumanoidAppearanceс blacklistScp, по аналогии сSeniorExternalAdministrativeZoneOfficer) либо уточните требования.💡 Предлагаемое исправление
- type: ScpHolder holdableWhitelist: components: - - ClassDAppearance + - HumanoidAppearance + holdableBlacklist: + components: + - Scp🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Prototypes/_Scp/Roles/Jobs/Administration/security_commander.yml` around lines 36 - 39, Расширьте конфигурацию ScpHolder для роли Security Commander: вместо единственного элемента в holdableWhitelist (ClassDAppearance) добавьте также HumanoidAppearance и добавьте соответствующий blacklist (Scp) по аналогии с реализацией в SeniorExternalAdministrativeZoneOfficer; убедитесь, что ключи holdableWhitelist и blacklist правильно указаны и соответствуют остальной структуре файла.Resources/Prototypes/Actions/types.yml (1)
264-265:⚠️ Potential issue | 🟡 MinorЗамените
# Fire editна# Sunrise-Edit.
Resources/Prototypes/Actions/types.yml— upstream-файл, правка должна помечаться Sunrise-маркером.💡 Предлагаемое исправление
- - type: ScpHoldRestricted # Fire edit - block combat mode while held + - type: ScpHoldRestricted # Sunrise-Edit - block combat mode while held stage: SoftAs per coding guidelines: "use the
Sunriseprefix … andSunrise-Edit/Sunrise edit start/endmarkers for unavoidable upstream edits."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Prototypes/Actions/types.yml` around lines 264 - 265, В блоке, где определён type ScpHoldRestricted (the YAML mapping with "type: ScpHoldRestricted" and "stage: Soft"), замените комментарий "# Fire edit - block combat mode while held" на маркер Sunrise согласно гайду — используйте именно "# Sunrise-Edit" (или при многострочных изменениях обрамляющие "Sunrise edit start"/"Sunrise edit end"), чтобы правка помечалась как upstream Sunrise-изменение.Resources/Prototypes/Entities/Mobs/Species/base.yml (1)
280-282:⚠️ Potential issue | 🔴 Critical
ScpHolderвBaseMobSpeciesOrganic+ неверный маркер форка.
ScpHolderдобавлен в базовый органический прототип сTODO: Убрать перед мержем— в итоге механика удержания выдаётся всем органикам, а не только целевым ролям черезspecialв job'ах (Security Commander / Senior External Administrative Zone Officer). Это ломает ролевые ограничения, заявленные в PR.- Блок помечен
# Fire start/# Fire end— это upstream-файл, для него должны использоваться маркеры# Sunrise-Edit/# Sunrise edit start-# Sunrise edit end.Уберите
ScpHolderдо мержа и замените маркеры.💡 Предлагаемое исправление
- # Fire start - - type: ScpHolder # TODO: Убрать перед мержем - - type: ScpHoldable + # Sunrise edit start + - type: ScpHoldable + # Sunrise edit endAs per coding guidelines: "use the
Sunriseprefix, the_Sunriseproject folder, andSunrise-Edit/Sunrise edit start/endmarkers for unavoidable upstream edits."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Prototypes/Entities/Mobs/Species/base.yml` around lines 280 - 282, Remove the unintended ScpHolder entry from the BaseMobSpeciesOrganic prototype (the line with "type: ScpHolder" and its TODO) so holding mechanics are not granted to all organics; leave or keep "type: ScpHoldable" only if that is required for the specific role mechanics. Also replace the upstream fork markers "# Fire start" / "# Fire end" with the required Sunrise markers: add a single "# Sunrise-Edit" header and wrap the local modification block with "# Sunrise edit start" and "# Sunrise edit end" (use the Sunrise prefix for any project-specific markers/folders) to mark the unavoidable upstream edit. Ensure references to ScpHolder and BaseMobSpeciesOrganic are the targets of the change.Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_janitor.yml (1)
23-31:⚠️ Potential issue | 🟠 MajorНе выдавайте удержание SCP-096 роли
ClassDJanitor.Это low-access роль, а заявленный scope PR — Command Squad для персонала и ОСН для SCP-096. Сейчас
ClassDJanitorсможет участвовать в удержании SCP-096 вне целевого баланса.Предлагаемая правка
- - type: ScpHolder - holdableWhitelist: - components: - - Scp096 - holdableBlacklist: - components: - - ActiveScp096Rage - - ActiveScp096HeatingUp - - ActiveScp096WithoutFace🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_janitor.yml` around lines 23 - 31, В роли ClassDJanitor удалите или отключите блок ScpHolder (т.е. удалите ключи type: ScpHolder и связанные holdableWhitelist/holdableBlacklist), чтобы ClassDJanitor больше не имел возможности участвовать в удержании SCP-096; найдите секцию с именем ClassDJanitor и ключами ScpHolder / holdableWhitelist / holdableBlacklist и удалите её целиком или замените на разрешённый для low-access набор, если нужно сохранить структуру.Resources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/junior_external_administrative_zone_officer.yml (1)
30-36:⚠️ Potential issue | 🟡 MinorДобавьте
# Sunrise-Editк добавленномуScpHolder.Блок добавлен в существующий YAML-прототип без обязательного Sunrise-маркера.
Предлагаемая правка
- - type: ScpHolder + - type: ScpHolder # Sunrise-Edit holdableWhitelist: components: - HumanoidAppearanceAs per coding guidelines,
**/*.{yaml,yml,ftl,py,sh}: Use# Sunrise-Editmarker format in YAML, FTL, Python, and Shell script files for Sunrise fork edits.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/junior_external_administrative_zone_officer.yml` around lines 30 - 36, Добавленный блок ScpHolder не содержит обязательного маркера Sunrise и должен быть помечен; перед строкой с "type: ScpHolder" вставьте строку с комментарием "# Sunrise-Edit" (в том же YAML-уровне отступа), чтобы отметить правку по правилам для файлов YAML/FTL/PY/SH; проверьте блокы с идентификаторами ScpHolder, holdableWhitelist и holdableBlacklist чтобы маркер однозначно относится к этому добавленному прототипу.Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_botanist.yml (1)
26-34:⚠️ Potential issue | 🟠 Major
ClassDBotanistне должен получать удержание SCP-096.Это low-access роль вне заявленного scope PR. Переименование компонента на
ScpHolderне меняет проблему: роль всё ещё получает возможность удерживать SCP-096.Предлагаемая правка
- - type: ScpHolder - holdableWhitelist: - components: - - Scp096 - holdableBlacklist: - components: - - ActiveScp096Rage - - ActiveScp096HeatingUp - - ActiveScp096WithoutFace🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_botanist.yml` around lines 26 - 34, ClassDBotanist currently gets Scp096 via the ScpHolder block — remove Scp096 from its holdableWhitelist (or remove the ScpHolder entry entirely for ClassDBotanist) so the role cannot hold SCP-096; specifically edit the ClassDBotanist definition to either delete the ScpHolder section or ensure holdableWhitelist does not list Scp096 (or add Scp096 to holdableBlacklist) to prevent granting hold ability.Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d.yml (1)
21-29:⚠️ Potential issue | 🟠 MajorНе выдавайте удержание обычному
ClassD.По scope PR удержание должно быть у командного состава и ОСН для SCP-096; этот блок даёт low-access роли возможность удерживать SCP-096 и расширяет доступ к механике.
Предлагаемое исправление
- - type: ScpHolder - holdableWhitelist: - components: - - Scp096 - holdableBlacklist: - components: - - ActiveScp096Rage - - ActiveScp096HeatingUp - - ActiveScp096WithoutFace🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d.yml` around lines 21 - 29, В файле роли ClassD удалите или отключите блок ScpHolder, который даёт low-access ролям возможность удерживать SCP-096: конкретно уберите holdableWhitelist с компонентом Scp096 (и связанные holdableBlacklist/ActiveScp096* записи) из class_d.yml; вместо этого добавьте/оставьте этот ScpHolder только в ролях командного состава и ОСН (где требуется механика), т.е. перенесите или создайте аналогичный блок ScpHolder с holdableWhitelist: components: - Scp096 в соответствующих ролях команд/ОСН и убедитесь, что class_d.yml больше не содержит компонента Scp096 в списках удержания.Resources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/heavy_containment_zone_officer.yml (1)
31-40:⚠️ Potential issue | 🟡 MinorДобавьте
# Sunrise-Editк добавленному блокуScpHolder.Блок всё ещё добавлен без fork-маркера на строке начала изменения.
Предлагаемая правка
- - type: ScpHolder + - type: ScpHolder # Sunrise-Edit holdableWhitelist: components: - Scp096As per coding guidelines,
**/*.{yaml,yml,ftl,py,sh}: Use '# Sunrise-Edit' marker format in YAML, FTL, Python, and Shell script files for Sunrise fork edits.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/heavy_containment_zone_officer.yml` around lines 31 - 40, The added ScpHolder block (the mapping starting with "type: ScpHolder" and containing holdableWhitelist and holdableBlacklist) is missing the required fork marker; insert a YAML comment line "# Sunrise-Edit" immediately above the ScpHolder block (i.e., directly before the "type: ScpHolder" key) so the change is marked as a Sunrise fork edit while leaving the existing keys (holdableWhitelist, holdableBlacklist, components, etc.) unchanged.Content.Shared/Movement/Pulling/Systems/PullingSystem.cs (1)
532-535:⚠️ Potential issue | 🔴 CriticalНе редиректите SCP-hold до общей валидации pull-flow.
Line 533 возвращает результат
TryRedirectPullToScpHold()доCanPull()и доPullAttemptEvent, поэтому SCP-hold путь обходитNeedsHands,CanInteract, container/buckle guards и veto-событияStartPullAttemptEvent/BeingPulledAttemptEvent/PullAttemptEvent. Подписчики pull-системы не смогут запретить действие; вынесите общие guards/events в общий helper или вызывайте их и для redirect-пути.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 the current code and only fix it if needed. In `@Content.Shared/Movement/Pulling/Systems/PullingSystem.cs` around lines 532 - 535, Вызов TryRedirectPullToScpHold возвращает результат до выполнения общих проверок/событий и тем самым обходит CanPull(), StartPullAttemptEvent/BeingPulledAttemptEvent/PullAttemptEvent и guards (NeedsHands, CanInteract, container/buckle), поэтому измените порядок/реализацию так, чтобы общие валидации и публикация стартовых veto-событий выполнялись перед перенаправлением: либо переместите вызовы CanPull() и генерацию StartPullAttemptEvent/BeingPulledAttemptEvent/PullAttemptEvent в общий helper и вызовите его и из основного пути, и из TryRedirectPullToScpHold, либо вызовите эти проверки/события сразу перед TryRedirectPullToScpHold и только затем применяйте redirect; используйте идентификаторы TryRedirectPullToScpHold, CanPull, StartPullAttemptEvent, BeingPulledAttemptEvent, PullAttemptEvent, NeedsHands и CanInteract для поиска мест в коде.Resources/Locale/en-US/_strings/_scp/holding/holding.ftl (1)
14-14:⚠️ Potential issue | 🟡 MinorПометьте подсказку управления как
OOC:.Фраза “Move or click this alert to try to break free” — это инструкция игроку, а не внутриигровое описание.
💬 Предлагаемая правка
-alerts-scp-held-desc = Someone is physically gripping you. Move or click this alert to try to break free. In a hard restraint you must endure the hold before the breakout starts. +alerts-scp-held-desc = Someone is physically gripping you. OOC: Move or click this alert to try to break free. In a hard restraint you must endure the hold before the breakout starts.As per coding guidelines, “Require explicit
OOC:markers for out-of-character hints, controls, or gameplay instructions.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Locale/en-US/_strings/_scp/holding/holding.ftl` at line 14, The string value for alerts-scp-held-desc contains an out-of-character control instruction; update the alerts-scp-held-desc message so the player instruction is explicitly marked with "OOC:" — e.g., prepend or insert "OOC:" before "Move or click this alert to try to break free" so the line reads as an in-world description followed by an OOC control hint, leaving the rest of the sentence unchanged.Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Restrictions.cs (2)
28-31:⚠️ Potential issue | 🟠 MajorНе пере-применяйте restriction во время удаления компонента.
ValidateActions()наComponentShutdownможет оставить action выключенным уже после удаленияScpHoldRestrictedComponent.🐛 Предлагаемая правка
- SubscribeLocalEvent<ScpHoldRestrictedComponent, ComponentShutdown>(OnRestrictRemove); SubscribeLocalEvent<ScpHoldRestrictedComponent, ActionAttemptEvent>(OnHoldRestrictedActionAttempt); } @@ - private void OnRestrictRemove(Entity<ScpHoldRestrictedComponent> ent, ref ComponentShutdown args) - { - ValidateActions(ent.AsNullable()); - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Restrictions.cs` around lines 28 - 31, Метод OnRestrictRemove(Entity<ScpHoldRestrictedComponent> ent, ref ComponentShutdown args) не должен вызывать ValidateActions() на ComponentShutdown — это пере-применяет restriction и может оставить действия отключёнными после удаления компонента. Уберите вызов ValidateActions(ent.AsNullable()) из OnRestrictRemove и переместите логику в обработчик удаления компонента (например, обработчик ComponentRemove/ComponentRemoved для Entity<ScpHoldRestrictedComponent>) или вызывайте ValidateActions только после окончательного удаления (не в ComponentShutdown), чтобы ValidateActions выполнялся когда компонент действительно удалён.
65-71:⚠️ Potential issue | 🟠 MajorНе вызывайте one-shot popup напрямую из shared predicted path.
ActionAttemptEventможет проходить через prediction replay, поэтому прямой_popup.PopupClient(...)рискует дать дублирующиеся popups.🐛 Предлагаемая правка
- _popup.PopupClient(Loc.GetString("scp-hold-action-restricted"), args.User, args.User); args.Cancelled = true; + Popup(args.User, "scp-hold-action-restricted");As per coding guidelines,
Content.Shared/**/*.cs: “Fail if shared predicted code adds predicted side effects such asPlayPvs,PopupEntity, or similar one-shot UX effects withoutIsFirstTimePredicted,ApplyingState, or a predicted helper such asPlayPredicted/PopupPredicted.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Restrictions.cs` around lines 65 - 71, OnHoldRestrictedActionAttempt currently calls _popup.PopupClient directly in a shared predicted path which can produce duplicate popups during prediction replay; change the code to only emit the one-shot UX when this is the first predicted execution by guarding with the prediction check (e.g., IsFirstTimePredicted on the ActionAttemptEvent) or by routing the notification through the predicted helper (e.g., use a PopupPredicted-style helper instead of _popup.PopupClient), keeping the existing cancellation behavior (args.Cancelled = true) and the IsHeldAtStage check intact.
🤖 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.Client/_Scp/Holding/ScpHoldingSystem.cs`:
- Around line 99-136: In OnUpdateHeldPredicted, fix the inconsistent indentation
of the if ((EntityUid) ent == local) block: remove the extra nesting level so
the block body aligns with the other if blocks in the method (adjust the lines
that set args.IsPredicted = true; return; inside that if), keeping the same
logic and using the existing ent, local, and args symbols.
In `@Content.Server/Movement/Systems/PullController.cs`:
- Around line 125-128: The fork markers used in PullController.cs are incorrect;
replace the `Fire edit start`/`Fire edit end` markers around the upstream edit
that routes cursor-move through _scpHolding.TryMoveHeldToCursor(player, coords)
with the repository-standard Sunrise markers (use `Sunrise-Edit` for file header
and `Sunrise edit start` / `Sunrise edit end` around the changed block) so the
edit is correctly identified as an upstream modification.
- Around line 2-4: В файле PullController.cs удалите неиспользуемые импорты:
уберите строки с using Content.Shared._Scp.Holding.Components и using
Content.Shared._Scp.Holding.Systems, оставив только using
Content.Server._Scp.Holding (поскольку код использует только ScpHoldingSystem);
после удаления пересоберите/проверьте отсутствие ошибок и выполните
форматирование файла.
In `@Content.Shared/_Scp/Holding/Compatibility/PullingSystem.ScpHolding.cs`:
- Around line 45-65: Update the XML doc for TryRedirectPullToScpHold to
explicitly state return semantics: "Returns true if the pull was
handled/consumed (including being rejected); the out parameter 'result'
indicates whether the redirect succeeded." Mention that this covers the code
paths that call _scpHolding.CanToggleHold, TryStopPull and
_scpHolding.TryToggleHold so callers understand that true does not necessarily
mean success. Optionally rename the out parameter 'result' to 'success' (or
rename the method to indicate "handled") for clarity; if renaming, update all
call sites (e.g., PullingSystem.TryStartPull) to use the new name.
In
`@Content.Shared/_Scp/Holding/Components/ActiveStateScpHoldableCursorMoveComponent.cs`:
- Around line 17-18: Поле Holder в компоненте
ActiveStateScpHoldableCursorMoveComponent должно быть nullable (EntityUid?
Holder) так как текущее значение EntityUid.Invalid может быть ошибочно принято
за валидную цель; измените объявление Holder на nullable, обновите места
использования (включая методы SetCursorMoveState и ClearCursorMoveState) чтобы
проверять Holder.HasValue перед доступом и распаковывать через Holder.Value там,
где требуется, и при очистке состояния удалять/обнулять компонент как сейчас.
In `@Content.Shared/_Scp/Holding/Components/ScpBreakoutAttemptComponent.cs`:
- Around line 9-10: ScpBreakoutAttemptComponent is a shared, networked component
and needs the AutoGenerateComponentState attribute to enable proper
serialization/deserialization and prediction rollback; add the
[AutoGenerateComponentState] attribute to the ScpBreakoutAttemptComponent
declaration (alongside existing [RegisterComponent, NetworkedComponent] and
[Access(typeof(SharedScpHoldingSystem))]) so the component state is
auto-generated for network syncing.
In `@Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Actions.cs`:
- Around line 23-46: The popup is showing the raw localization key because
PopupClient is being passed the literal "scp-hold-already-holding-other"; update
the PopupClient calls to pass the localized string instead (use
Loc.GetString("scp-hold-already-holding-other") where PopupClient is invoked in
TryToggleHold and the other PopupClient invocation later in this file) so the
user sees the translated message.
- Around line 246-267: In TryStartFullBreakout, when fullHeld.StartedAt ==
TimeSpan.Zero do not call Loc.GetString("scp-hold-breakout-too-early",
("seconds", 1)) because it misreports remaining time; instead show a distinct
"not ready" message (e.g. use a new localization key like
"scp-hold-breakout-not-ready") via _popup.PopupClient(Loc.GetString(...), held),
or alternatively compute remaining time from a valid start time if one can be
derived — update the branch that checks fullHeld.StartedAt to use the new key
(or computed remaining) while keeping the existing logic that handles the normal
breakoutAvailableAt path.
- Around line 325-335: The ApplyFullBreakoutHolderCooldown method uses
TimeSpan.FromTicks(hold.HoldActionCooldown.Ticks * 2) which is equivalent to
hold.HoldActionCooldown * 2; update the calculation to use
hold.HoldActionCooldown * 2 to simplify and clarify intent (leave the rest of
the logic — the TryComp check, cooldownEnd comparison against
hold.HoldAvailableAt, and the SetHoldAvailableAt((holderUid, hold), cooldownEnd)
call — unchanged).
In
`@Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.BreakoutAttempt.cs`:
- Around line 136-149: В методе ShowBreakoutAttemptFeedback вы вызываете
Comp<ScpHolderComponent>(holderUid) без проверки и это бросит, если конфигурация
была удалена; замените этот вызов на TryComp<ScpHolderComponent>(holderUid, out
var scpHolderComp) и, если TryComp вернул false, пропускайте этого holderUid
(как уже делаете при отсутствии ActiveScpHolderComponent), иначе берите
scpHolderComp.BreakoutAttemptAlertSettings и передавайте их в
_worldAlert.TrySpawnAlert(holderUid, ...); сохраните текущую финальную отправку
_worldAlert.TrySpawnAlert(held, holdable.BreakoutAttemptAlertSettings) без
изменений.
In `@Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.cs`:
- Around line 60-64: The override of Update in SharedScpHoldingSystem currently
calls only UpdateSharedState() and UpdateHeldStates() and omits calling
base.Update(frameTime); modify the Update(float frameTime) method to invoke
base.Update(frameTime) (either before or after the helper calls) to conform to
EntitySystem lifecycle conventions and ensure base class behavior is preserved.
In `@Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.CursorMove.cs`:
- Around line 41-82: The parameter quiet on CanMoveHeldToCursor is unused (only
_ = quiet;) — either remove the parameter and the discard or actually thread
quiet into the same user-facing checks/popups as CanToggleHold: update
CanMoveHeldToCursor to pass quiet into any validation methods or popup calls
(e.g., any checks that emit messages or the
TryClampHeldCursorMoveTargetCoordinates/_interaction checks) so the method can
suppress UI when requested, or remove quiet and the discard to avoid dead code
and update all call sites accordingly.
- Around line 270-285: В обработчике OnHolderMove текущая ранняя отрезка
учитывает только совпадение EntityId и точное равенство позиции, из‑за чего
смена родителя (re-parent) с той же позицией всё равно считается изменением;
исправьте условие так, чтобы также проверять родителя/корневой EntityId и
сравнивать позиции с допуском (используйте EntityCoordinates.TryDelta или
сравнение LengthSquared() с маленьким epsilon вместо точного 0f), и только если
и родитель совпадает и дельта позиции меньше epsilon — вернуть; в остальных
случаях продолжать и не вызывать ClearCursorMoveState для чистого re-parent-а.
- Around line 196-200: The code currently calls Dirty(held, cursorMove) after
updating fields on ActiveStateScpHoldableCursorMoveComponent (cursorMove.Holder,
cursorMove.TargetCoordinates, cursorMove.Active); replace those calls with
point-updates using DirtyField(held, cursorMove,
nameof(ActiveStateScpHoldableCursorMoveComponent.<Property>)) for each changed
property (i.e., call DirtyField(..., nameof(Holder)) after setting Holder,
DirtyField(..., nameof(TargetCoordinates)) after setting TargetCoordinates, and
DirtyField(..., nameof(Active)) after setting Active). Apply the same
replacement to the other similar update sites that set those properties on the
same component (the other blocks that currently call Dirty(...)).
In `@Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Hands.cs`:
- Around line 128-143: In EnsureHeldHandBlockers, after calling
_hands.DoPickup(held, emptyHand, virtualItem.Value, held.Comp) verify the pickup
actually filled the hand (use _hands.TryGetEmptyHand or the component/state that
indicates empty vs filled) and if the hand remains empty increment a local
failedAttempts counter and break (or bail out) once a small threshold is
reached; alternatively break immediately on failure to avoid repeatedly spawning
virtualItem. Reference EnsureHeldHandBlockers, _hands.TryGetEmptyHand,
_hands.DoPickup and virtualItem to locate and guard the loop.
In `@Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Lifecycle.cs`:
- Around line 79-86: In OnHolderShutdown, set ActiveScpHolderComponent.Target to
null with a corresponding dirty call so the network delta is sent; after
assigning Target = null invoke the existing Dirty helper (or DirtyField) for the
entity/component to mark the networked field dirty (use Dirty(ent, ent.Comp) or
DirtyField(ent, ent.Comp, nameof(ActiveScpHolderComponent.Target)) so the change
is propagated).
In `@Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.State.cs`:
- Around line 126-129: The code currently uses an else if that prevents removing
both components when a holder has both ActiveScpHolderComponent and
ActiveStateScpHolderSlowdownComponent; update the logic in the removal sites so
you call RemComp<ActiveScpHolderComponent>(holderUid) and
RemComp<ActiveStateScpHolderSlowdownComponent>(holderUid) from two separate if
checks using _activeHolderQuery.HasComp(holderUid) and
_activeHolderSlowdownStateQuery.HasComp(holderUid) respectively (replace the
else if with an independent if), and apply the same change in ClearHoldState so
both components are removed independently (do not rely on OnHolderShutdown
side-effects).
In `@Content.Shared/_Scp/Other/WorldAlert/WorldAlertSystem.cs`:
- Around line 10-13: Member block ordering is wrong: move the dependency fields
(_audio and _net with [Dependency]) to appear before the constant
DefaultLifetimeSeconds so that Dependencies block precedes Constants/static;
update the WorldAlertSystem class so the readonly fields with [Dependency] come
first, then keep DefaultLifetimeSeconds as the constants/static block.
- Around line 32-40: The current EnsureTimedDespawn method returns immediately
if a TimedDespawnComponent already exists, ignoring the incoming
lifetime/settings; change it so that if HasComp<TimedDespawnComponent>(uid) is
true you fetch the existing component (e.g. via Comp<TimedDespawnComponent>(uid)
or TryComp) and, if the lifetime parameter has a value, update that component's
Lifetime to lifetime.Value.TotalSeconds (otherwise leave it unchanged); if no
component exists keep the existing EnsureComp<TimedDespawnComponent>(uid) path
and set Lifetime to lifetime.HasValue ? (float)lifetime.Value.TotalSeconds :
DefaultLifetimeSeconds.
In `@Content.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.Holding.cs`:
- Around line 89-100: The branch handling holderCount <= 0 inside
GetHoldBreakoutDirection is unreachable; make holderCount a strict positive
parameter by removing the fallback branch (the MathF.Cos/MathF.Sin angle path
remains) and add a defensive check at the top of GetHoldBreakoutDirection (e.g.,
Debug.Assert(holderCount > 0) or throw
ArgumentOutOfRangeException("holderCount")) to document the contract; update
callers to ensure they only call GetHoldBreakoutDirection when holderCount > 0
(or adjust call sites to pass a positive holderCount) so the function no longer
contains the dead holderCount <= 0 branch.
In `@Content.Shared/Hands/EntitySystems/SharedHandsSystem.Drop.cs`:
- Around line 142-148: Замените блок с пометкой "Fire edit" на формат Sunrise
(используйте либо однострочный "Sunrise-Edit" либо "Sunrise edit start"/"Sunrise
edit end") и упростите проверку компонента: вместо TryComp(entity, out
VirtualItemComponent? _) используйте HasComp<VirtualItemComponent>(entity); при
совпадении продолжайте вызывать DoDrop(ent, handId, doDropInteraction: false) и
вернуть true (сохраняйте существующие вызовы DoDrop, ent и handId).
In `@Content.Shared/Movement/Pulling/Systems/PullingSystem.cs`:
- Around line 42-44: Замените все однострочные маркеры форка вида "Fire edit
start"/"Fire edit end" на "Sunrise-Edit" в этом файле; конкретно обновите блок
вокруг объявления public sealed partial class PullingSystem : EntitySystem (и
аналогичные блоки на участках примерно 65-67 и 532-535), сохранив остальную
обёртку/комментарии, но используя точно "Sunrise-Edit" как единую метку для
C#/C++ Sunrise форка.
In `@Resources/Locale/ru-RU/_strings/_scp/holding/holding.ftl`:
- Line 14: The string alerts-scp-held-desc contains an in-game control hint that
must be marked as out-of-character; update the alerts-scp-held-desc value so the
player instruction segment ("Двигайтесь или нажмите на этот статус-эффект, чтобы
попытаться вырваться...") is explicitly prefixed with "OOC:" (keeping the rest
of the sentence and meaning intact and preserving the note about full hold
requiring breaking the grab).
In `@Resources/Prototypes/_Scp/Entities/Mobs/Player/Scp/Main/scp096.yml`:
- Around line 118-121: Сделайте удержание SCP‑096 доступным для роли: в сущности
SCP‑096 (символ ScpHoldable в scp096.yml) добавьте явный holderWhitelist:
[Scp096] чтобы переопределить роль SeniorExternalAdministrativeZoneOfficer с
ScpHolder.holdableBlacklist: [Scp]; альтернативно можно изменить компонент
ScpHolder у SeniorExternalAdministrativeZoneOfficer, убрав или скорректировав
holdableBlacklist так, чтобы Scp096 не блокировался — используйте имена
компонентов ScpHoldable, holderWhitelist, ScpHolder, holdableBlacklist и
идентификатор Scp096 для локализации и внесения правки.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/field_medical_specialist.yml`:
- Around line 34-40: The new ScpHolder block added to the YAML must be marked as
a Sunrise fork edit: insert the Sunrise marker (use the "# Sunrise-Edit"
comment) adjacent to the added block so the change to the existing prototype is
explicitly flagged; locate the ScpHolder stanza (keys: ScpHolder,
holdableWhitelist, holdableBlacklist, components/HumanoidAppearance,
components/Scp) and add the "# Sunrise-Edit" marker in the same YAML scope above
or immediately next to that block.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/senior_external_administrative_zone_officer.yml`:
- Around line 31-37: Blacklist "Scp" in the ScpHolder entry blocks holding
SCP-096 and prevents the OCN squad feature; either remove "Scp" from
holdableBlacklist.components in the ScpHolder block so SCPs can be held, or add
an exception on the SCP side by updating the ScpHoldable logic (e.g., implement
a holderWhitelist on the ScpHoldable for SCP-096 / its entity ID) so that
SCP-096 is allowed to be held despite the global blacklist; locate the ScpHolder
block (holdableWhitelist / holdableBlacklist) and the ScpHoldable implementation
to apply the chosen fix.
In `@Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_cook.yml`:
- Around line 26-34: В файле class_d_cook.yml удалите блок, который добавляет
возможность удержания: весь узел с ключом ScpHolder (включая поля
holdableWhitelist и holdableBlacklist) для роли ClassDCook; т.е. уберите
упоминание ScpHolder и связанные подключи (Scp096, ActiveScp096Rage,
ActiveScp096HeatingUp, ActiveScp096WithoutFace) из определения ClassDCook, и
проверьте, что нет других ссылок на ScpHolder внутри ClassDCook, чтобы
гарантировать, что роль больше не получает механики удержания.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/heavy_containment_zone_commandant.yml`:
- Around line 31-40: The newly added ScpHolder block (the mapping starting with
"type: ScpHolder" and its keys holdableWhitelist/holdableBlacklist and nested
components like Scp096, ClassDAppearance, ActiveScp096Rage,
ActiveScp096HeatingUp, ActiveScp096WithoutFace) must be marked as a Sunrise fork
edit; insert the single-line comment "# Sunrise-Edit" immediately above the
ScpHolder block so the entire added block is clearly annotated per the Sunrise
marker convention for YAML files.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/junior_heavy_containment_zone_officer.yml`:
- Around line 30-39: Добавьте маркер fork-редакта "# Sunrise-Edit"
непосредственно перед вновь добавленным блоком ScpHolder (блок, начинающийся с
ключа ScpHolder и содержащий holdableWhitelist/holdableBlacklist и компоненты
Scp096, ClassDAppearance, ActiveScp096Rage, ActiveScp096HeatingUp,
ActiveScp096WithoutFace), чтобы пометить Sunrise-правку в YAML по правилам;
просто вставьте строку с точным комментарием "# Sunrise-Edit" перед этим блока.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/senior_heavy_containment_zone_officer.yml`:
- Around line 31-40: The newly added ScpHolder block (the block with keys
ScpHolder, holdableWhitelist, holdableBlacklist and components like Scp096,
ClassDAppearance, ActiveScp096Rage, ActiveScp096HeatingUp,
ActiveScp096WithoutFace) needs the Sunrise fork marker: add a line with exactly
"# Sunrise-Edit" immediately above the ScpHolder mapping so the inserted YAML
block is clearly marked as a Sunrise edit per repository guidelines.
---
Duplicate comments:
In `@Content.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Restrictions.cs`:
- Around line 28-31: Метод OnRestrictRemove(Entity<ScpHoldRestrictedComponent>
ent, ref ComponentShutdown args) не должен вызывать ValidateActions() на
ComponentShutdown — это пере-применяет restriction и может оставить действия
отключёнными после удаления компонента. Уберите вызов
ValidateActions(ent.AsNullable()) из OnRestrictRemove и переместите логику в
обработчик удаления компонента (например, обработчик
ComponentRemove/ComponentRemoved для Entity<ScpHoldRestrictedComponent>) или
вызывайте ValidateActions только после окончательного удаления (не в
ComponentShutdown), чтобы ValidateActions выполнялся когда компонент
действительно удалён.
- Around line 65-71: OnHoldRestrictedActionAttempt currently calls
_popup.PopupClient directly in a shared predicted path which can produce
duplicate popups during prediction replay; change the code to only emit the
one-shot UX when this is the first predicted execution by guarding with the
prediction check (e.g., IsFirstTimePredicted on the ActionAttemptEvent) or by
routing the notification through the predicted helper (e.g., use a
PopupPredicted-style helper instead of _popup.PopupClient), keeping the existing
cancellation behavior (args.Cancelled = true) and the IsHeldAtStage check
intact.
In `@Content.Shared/Movement/Pulling/Systems/PullingSystem.cs`:
- Around line 532-535: Вызов TryRedirectPullToScpHold возвращает результат до
выполнения общих проверок/событий и тем самым обходит CanPull(),
StartPullAttemptEvent/BeingPulledAttemptEvent/PullAttemptEvent и guards
(NeedsHands, CanInteract, container/buckle), поэтому измените порядок/реализацию
так, чтобы общие валидации и публикация стартовых veto-событий выполнялись перед
перенаправлением: либо переместите вызовы CanPull() и генерацию
StartPullAttemptEvent/BeingPulledAttemptEvent/PullAttemptEvent в общий helper и
вызовите его и из основного пути, и из TryRedirectPullToScpHold, либо вызовите
эти проверки/события сразу перед TryRedirectPullToScpHold и только затем
применяйте redirect; используйте идентификаторы TryRedirectPullToScpHold,
CanPull, StartPullAttemptEvent, BeingPulledAttemptEvent, PullAttemptEvent,
NeedsHands и CanInteract для поиска мест в коде.
In `@Resources/Locale/en-US/_strings/_scp/holding/holding.ftl`:
- Line 14: The string value for alerts-scp-held-desc contains an
out-of-character control instruction; update the alerts-scp-held-desc message so
the player instruction is explicitly marked with "OOC:" — e.g., prepend or
insert "OOC:" before "Move or click this alert to try to break free" so the line
reads as an in-world description followed by an OOC control hint, leaving the
rest of the sentence unchanged.
In `@Resources/Prototypes/_Scp/Roles/Jobs/Administration/security_commander.yml`:
- Around line 36-39: Расширьте конфигурацию ScpHolder для роли Security
Commander: вместо единственного элемента в holdableWhitelist (ClassDAppearance)
добавьте также HumanoidAppearance и добавьте соответствующий blacklist (Scp) по
аналогии с реализацией в SeniorExternalAdministrativeZoneOfficer; убедитесь, что
ключи holdableWhitelist и blacklist правильно указаны и соответствуют остальной
структуре файла.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/junior_external_administrative_zone_officer.yml`:
- Around line 30-36: Добавленный блок ScpHolder не содержит обязательного
маркера Sunrise и должен быть помечен; перед строкой с "type: ScpHolder"
вставьте строку с комментарием "# Sunrise-Edit" (в том же YAML-уровне отступа),
чтобы отметить правку по правилам для файлов YAML/FTL/PY/SH; проверьте блокы с
идентификаторами ScpHolder, holdableWhitelist и holdableBlacklist чтобы маркер
однозначно относится к этому добавленному прототипу.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_botanist.yml`:
- Around line 26-34: ClassDBotanist currently gets Scp096 via the ScpHolder
block — remove Scp096 from its holdableWhitelist (or remove the ScpHolder entry
entirely for ClassDBotanist) so the role cannot hold SCP-096; specifically edit
the ClassDBotanist definition to either delete the ScpHolder section or ensure
holdableWhitelist does not list Scp096 (or add Scp096 to holdableBlacklist) to
prevent granting hold ability.
In `@Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_janitor.yml`:
- Around line 23-31: В роли ClassDJanitor удалите или отключите блок ScpHolder
(т.е. удалите ключи type: ScpHolder и связанные
holdableWhitelist/holdableBlacklist), чтобы ClassDJanitor больше не имел
возможности участвовать в удержании SCP-096; найдите секцию с именем
ClassDJanitor и ключами ScpHolder / holdableWhitelist / holdableBlacklist и
удалите её целиком или замените на разрешённый для low-access набор, если нужно
сохранить структуру.
In `@Resources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d.yml`:
- Around line 21-29: В файле роли ClassD удалите или отключите блок ScpHolder,
который даёт low-access ролям возможность удерживать SCP-096: конкретно уберите
holdableWhitelist с компонентом Scp096 (и связанные
holdableBlacklist/ActiveScp096* записи) из class_d.yml; вместо этого
добавьте/оставьте этот ScpHolder только в ролях командного состава и ОСН (где
требуется механика), т.е. перенесите или создайте аналогичный блок ScpHolder с
holdableWhitelist: components: - Scp096 в соответствующих ролях команд/ОСН и
убедитесь, что class_d.yml больше не содержит компонента Scp096 в списках
удержания.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/heavy_containment_zone_officer.yml`:
- Around line 31-40: The added ScpHolder block (the mapping starting with "type:
ScpHolder" and containing holdableWhitelist and holdableBlacklist) is missing
the required fork marker; insert a YAML comment line "# Sunrise-Edit"
immediately above the ScpHolder block (i.e., directly before the "type:
ScpHolder" key) so the change is marked as a Sunrise fork edit while leaving the
existing keys (holdableWhitelist, holdableBlacklist, components, etc.)
unchanged.
In `@Resources/Prototypes/Actions/types.yml`:
- Around line 264-265: В блоке, где определён type ScpHoldRestricted (the YAML
mapping with "type: ScpHoldRestricted" and "stage: Soft"), замените комментарий
"# Fire edit - block combat mode while held" на маркер Sunrise согласно гайду —
используйте именно "# Sunrise-Edit" (или при многострочных изменениях
обрамляющие "Sunrise edit start"/"Sunrise edit end"), чтобы правка помечалась
как upstream Sunrise-изменение.
In `@Resources/Prototypes/Entities/Mobs/Species/base.yml`:
- Around line 280-282: Remove the unintended ScpHolder entry from the
BaseMobSpeciesOrganic prototype (the line with "type: ScpHolder" and its TODO)
so holding mechanics are not granted to all organics; leave or keep "type:
ScpHoldable" only if that is required for the specific role mechanics. Also
replace the upstream fork markers "# Fire start" / "# Fire end" with the
required Sunrise markers: add a single "# Sunrise-Edit" header and wrap the
local modification block with "# Sunrise edit start" and "# Sunrise edit end"
(use the Sunrise prefix for any project-specific markers/folders) to mark the
unavoidable upstream edit. Ensure references to ScpHolder and
BaseMobSpeciesOrganic are the targets of the change.
🪄 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: 309a468f-f830-4c7f-af77-04c92ded739a
📒 Files selected for processing (61)
Content.Client/_Scp/Holding/ScpHoldingSystem.csContent.IntegrationTests/Tests/_Scp/ScpHeadsetEncryptionKeysTest.csContent.Server/Movement/Systems/PullController.csContent.Server/_Scp/Holding/ScpHoldingSystem.csContent.Shared/Hands/EntitySystems/SharedHandsSystem.Drop.csContent.Shared/Interaction/SharedInteractionSystem.Blocking.csContent.Shared/Movement/Pulling/Systems/PullingSystem.csContent.Shared/_Scp/Holding/Compatibility/PullingSystem.ScpHolding.csContent.Shared/_Scp/Holding/Components/ActiveScpHoldableComponent.csContent.Shared/_Scp/Holding/Components/ActiveScpHolderComponent.csContent.Shared/_Scp/Holding/Components/ActiveStateScpHoldableCursorMoveComponent.csContent.Shared/_Scp/Holding/Components/ActiveStateScpHoldableFullHoldComponent.csContent.Shared/_Scp/Holding/Components/ActiveStateScpHolderSlowdownComponent.csContent.Shared/_Scp/Holding/Components/ScpBreakoutAttemptComponent.csContent.Shared/_Scp/Holding/Components/ScpHeldHandBlockerComponent.csContent.Shared/_Scp/Holding/Components/ScpHoldHandBlockerComponent.csContent.Shared/_Scp/Holding/Components/ScpHoldImmuneComponent.csContent.Shared/_Scp/Holding/Components/ScpHoldRestrictedComponent.csContent.Shared/_Scp/Holding/Components/ScpHoldableComponent.csContent.Shared/_Scp/Holding/Components/ScpHolderComponent.csContent.Shared/_Scp/Holding/ScpHoldStage.csContent.Shared/_Scp/Holding/ScpHoldingEvents.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Actions.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.BreakoutAttempt.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.CursorMove.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Drag.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Hands.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Lifecycle.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.Restrictions.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.State.csContent.Shared/_Scp/Holding/Systems/SharedScpHoldingSystem.csContent.Shared/_Scp/Other/WorldAlert/WorldAlertSettings.csContent.Shared/_Scp/Other/WorldAlert/WorldAlertSystem.csContent.Shared/_Scp/Scp096/Main/Components/Scp096Component.csContent.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.Holding.csContent.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.Rage.csContent.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.WithoutFace.csContent.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.csResources/Locale/en-US/_strings/_scp/holding/holding.ftlResources/Locale/ru-RU/_strings/_scp/holding/holding.ftlResources/Prototypes/Actions/types.ymlResources/Prototypes/Entities/Mobs/Species/base.ymlResources/Prototypes/_Scp/Actions/scp096.ymlResources/Prototypes/_Scp/Alerts/holding.ymlResources/Prototypes/_Scp/Entities/Effects/world_alerts.ymlResources/Prototypes/_Scp/Entities/Mobs/Player/Scp/Main/scp096.ymlResources/Prototypes/_Scp/Entities/StatusEffects/holding.ymlResources/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/field_medical_specialist.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/LowAccessPersonnel/class_d.ymlResources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_botanist.ymlResources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_cook.ymlResources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_janitor.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 (1)
- Content.IntegrationTests/Tests/_Scp/ScpHeadsetEncryptionKeysTest.cs
Comment on lines
+99
to
+136
| private void OnUpdateHeldPredicted(Entity<ActiveScpHoldableComponent> ent, ref UpdateIsPredictedEvent args) | ||
| { | ||
| if (_player.LocalEntity is not { Valid: true } local) | ||
| return; | ||
|
|
||
| if ((EntityUid) ent == local) | ||
| { | ||
| args.IsPredicted = true; | ||
| return; | ||
| } | ||
|
|
||
| if (HasComp<ActiveStateScpHoldableCursorMoveComponent>(ent)) | ||
| { | ||
| args.BlockPrediction = true; | ||
| return; | ||
| } | ||
|
|
||
| if (_activeHolderQuery.TryComp(local, out var localHolder)) | ||
| { | ||
| if (localHolder.Target == ent) | ||
| { | ||
| args.IsPredicted = true; | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| foreach (var holder in ent.Comp.Holders) | ||
| { | ||
| if (holder != local) | ||
| continue; | ||
|
|
||
| args.IsPredicted = true; | ||
| return; | ||
| } | ||
|
|
||
| if (ent.Comp.Holders.Count > 0) | ||
| args.BlockPrediction = true; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Некорректный отступ в OnUpdateHeldPredicted.
Блок if ((EntityUid) ent == local) имеет лишний уровень отступа у тела, что рассогласуется со стилем остального файла и сбивает чтение. Нитпик.
🛠️ Предлагаемая правка
- if ((EntityUid) ent == local)
- {
- args.IsPredicted = true;
- return;
- }
+ if ((EntityUid) ent == local)
+ {
+ args.IsPredicted = true;
+ return;
+ }📝 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.
Suggested change
| private void OnUpdateHeldPredicted(Entity<ActiveScpHoldableComponent> ent, ref UpdateIsPredictedEvent args) | |
| { | |
| if (_player.LocalEntity is not { Valid: true } local) | |
| return; | |
| if ((EntityUid) ent == local) | |
| { | |
| args.IsPredicted = true; | |
| return; | |
| } | |
| if (HasComp<ActiveStateScpHoldableCursorMoveComponent>(ent)) | |
| { | |
| args.BlockPrediction = true; | |
| return; | |
| } | |
| if (_activeHolderQuery.TryComp(local, out var localHolder)) | |
| { | |
| if (localHolder.Target == ent) | |
| { | |
| args.IsPredicted = true; | |
| return; | |
| } | |
| } | |
| foreach (var holder in ent.Comp.Holders) | |
| { | |
| if (holder != local) | |
| continue; | |
| args.IsPredicted = true; | |
| return; | |
| } | |
| if (ent.Comp.Holders.Count > 0) | |
| args.BlockPrediction = true; | |
| } | |
| private void OnUpdateHeldPredicted(Entity<ActiveScpHoldableComponent> ent, ref UpdateIsPredictedEvent args) | |
| { | |
| if (_player.LocalEntity is not { Valid: true } local) | |
| return; | |
| if ((EntityUid) ent == local) | |
| { | |
| args.IsPredicted = true; | |
| return; | |
| } | |
| if (HasComp<ActiveStateScpHoldableCursorMoveComponent>(ent)) | |
| { | |
| args.BlockPrediction = true; | |
| return; | |
| } | |
| if (_activeHolderQuery.TryComp(local, out var localHolder)) | |
| { | |
| if (localHolder.Target == ent) | |
| { | |
| args.IsPredicted = true; | |
| return; | |
| } | |
| } | |
| foreach (var holder in ent.Comp.Holders) | |
| { | |
| if (holder != local) | |
| continue; | |
| args.IsPredicted = true; | |
| return; | |
| } | |
| if (ent.Comp.Holders.Count > 0) | |
| args.BlockPrediction = true; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Client/_Scp/Holding/ScpHoldingSystem.cs` around lines 99 - 136, In
OnUpdateHeldPredicted, fix the inconsistent indentation of the if ((EntityUid)
ent == local) block: remove the extra nesting level so the block body aligns
with the other if blocks in the method (adjust the lines that set
args.IsPredicted = true; return; inside that if), keeping the same logic and
using the existing ent, local, and args symbols.
Comment on lines
+2
to
+4
| using Content.Server._Scp.Holding; | ||
| using Content.Shared._Scp.Holding.Components; | ||
| using Content.Shared._Scp.Holding.Systems; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP '\b(ScpHoldableComponent|ScpHolderComponent|SharedScpHoldingSystem)\b' Content.Server/Movement/Systems/PullController.csRepository: space-sunrise/project-fire
Length of output: 52
🏁 Script executed:
cat -n Content.Server/Movement/Systems/PullController.cs | head -100Repository: space-sunrise/project-fire
Length of output: 5509
🏁 Script executed:
rg -n 'ScpHoldingSystem' Content.Server/Movement/Systems/PullController.csRepository: space-sunrise/project-fire
Length of output: 147
🏁 Script executed:
rg -n '\b[A-Z][a-zA-Z]*Component\b|\bSharedScpHoldingSystem\b' Content.Server/Movement/Systems/PullController.cs | head -50Repository: space-sunrise/project-fire
Length of output: 2110
🏁 Script executed:
fd -e cs Content.Shared/_Scp/Holding/Components/ | head -20 | xargs -I {} bash -c 'echo "=== {} ===" && grep "^public.*class\|^public.*component\|^public.*record" {}'Repository: space-sunrise/project-fire
Length of output: 531
🏁 Script executed:
fd . Content.Shared/_Scp/Holding/Components/ -e cs | xargs -I {} bash -c 'echo "=== {} ===" && grep "^public.*class\|^public.*component\|^public.*record" {}'Repository: space-sunrise/project-fire
Length of output: 1842
🏁 Script executed:
fd . Content.Shared/_Scp/Holding/Systems/ -e cs | xargs -I {} bash -c 'echo "=== {} ===" && grep "^public.*class\|^public.*system" {}'Repository: space-sunrise/project-fire
Length of output: 1266
Удалите неиспользуемые импорты из строк 3–4.
Файл использует только ScpHoldingSystem из Content.Server._Scp.Holding (строка 65). Импорты Content.Shared._Scp.Holding.Components и Content.Shared._Scp.Holding.Systems не задействованы и загрязняют upstream-файл.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Server/Movement/Systems/PullController.cs` around lines 2 - 4, В
файле PullController.cs удалите неиспользуемые импорты: уберите строки с using
Content.Shared._Scp.Holding.Components и using
Content.Shared._Scp.Holding.Systems, оставив только using
Content.Server._Scp.Holding (поскольку код использует только ScpHoldingSystem);
после удаления пересоберите/проверьте отсутствие ошибок и выполните
форматирование файла.
Comment on lines
+45
to
+65
| if (!_scpHolding.CanToggleHold(holder, | ||
| pullableUid, | ||
| ignoreHandAvailability: pullerComp.Pulling != null, | ||
| checkAttempt: true)) | ||
| return true; | ||
|
|
||
| if (pullerComp.Pulling is { } currentPullUid && | ||
| _pullableQuery.TryComp(currentPullUid, out var currentPull) && | ||
| !TryStopPull(currentPullUid, currentPull, pullerUid)) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| if (pullableComp.Puller != null && | ||
| !TryStopPull(pullableUid, pullableComp, pullableComp.Puller)) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| result = _scpHolding.TryToggleHold(holder, pullableUid, attemptChecked: true); | ||
| return true; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Поведение «redirect=true, result=false» неочевидно — добавьте комментарий или верните false явно.
При неуспешном CanToggleHold (строки 45‑49) или при неудачной остановке существующего pull (строки 51‑62) метод возвращает true, и вызывающий код в PullingSystem.TryStartPull пропускает обычный pull‑путь. По смыслу это, видимо, и нужно (SCP‑holdable нельзя тащить ванильным pull), но с точки зрения читаемости имя TryRedirectPullToScpHold намекает «вернул true, значит redirect выполнен». Стоит либо:
- задокументировать инвариант в xmldoc метода («
trueозначает что pull для SCP‑holdable обработан/проглочен, в т.ч. отказом»), - либо явно различать «handled (consume input)» и «success» — например, оставить
resultдля успеха и переименовать возвращаемое значение вhandled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Shared/_Scp/Holding/Compatibility/PullingSystem.ScpHolding.cs` around
lines 45 - 65, Update the XML doc for TryRedirectPullToScpHold to explicitly
state return semantics: "Returns true if the pull was handled/consumed
(including being rejected); the out parameter 'result' indicates whether the
redirect succeeded." Mention that this covers the code paths that call
_scpHolding.CanToggleHold, TryStopPull and _scpHolding.TryToggleHold so callers
understand that true does not necessarily mean success. Optionally rename the
out parameter 'result' to 'success' (or rename the method to indicate "handled")
for clarity; if renaming, update all call sites (e.g.,
PullingSystem.TryStartPull) to use the new name.
Comment on lines
+17
to
+18
| [AutoNetworkedField, ViewVariables] | ||
| public EntityUid Holder; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Рассмотрите nullable для Holder.
Holder объявлен как EntityUid (без nullable) со значением по умолчанию EntityUid.Invalid. Если компонент создаётся до первого валидного SetCursorMoveState, состояние Invalid легко спутать с валидной целью. Более идиоматично объявить EntityUid? Holder и проверять наличие через HasValue, тем более что ClearCursorMoveState всё равно удаляет компонент целиком.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Content.Shared/_Scp/Holding/Components/ActiveStateScpHoldableCursorMoveComponent.cs`
around lines 17 - 18, Поле Holder в компоненте
ActiveStateScpHoldableCursorMoveComponent должно быть nullable (EntityUid?
Holder) так как текущее значение EntityUid.Invalid может быть ошибочно принято
за валидную цель; измените объявление Holder на nullable, обновите места
использования (включая методы SetCursorMoveState и ClearCursorMoveState) чтобы
проверять Holder.HasValue перед доступом и распаковывать через Holder.Value там,
где требуется, и при очистке состояния удалять/обнулять компонент как сейчас.
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
Добавил систему удержания. Она позволяет двум людям схватить человека и не дать ему двигаться некоторое время.
Ее получит Комендантский отряд для всего персонала и отряд ОСН для SCP-096 с особыми условиями для процедуры лечения.
Ссылка на багрепорт/Предложение | Related Issue/Bug Report
Closes #824
Медиа (Видео/Скриншоты) | Media (Video/Screenshots)
Changelog
🆑 ThereDrD
Summary by CodeRabbit
Новые функции