Skip to content

Elevator fix 2.0#1067

Open
arthu241243 wants to merge 9 commits into
space-sunrise:masterfrom
arthu241243:Elevator-fix-2.0
Open

Elevator fix 2.0#1067
arthu241243 wants to merge 9 commits into
space-sunrise:masterfrom
arthu241243:Elevator-fix-2.0

Conversation

@arthu241243

@arthu241243 arthu241243 commented Jul 1, 2026

Copy link
Copy Markdown

Краткое описание | Short description

Теперь лифт точно-преточно работает, и более менее оптимизирован.

Ссылка на багрепорт/Предложение | Related Issue/Bug Report

Медиа (Видео/Скриншоты) | Media (Video/Screenshots)

Changelog

🆑 Fire_Helper/Патріот. Neyran

  • fix: Лифт теперь рабочий, баги с переносом человека и вещей исправлены

Summary by CodeRabbit

  • New Features

    • Добавлены новые элементы управления лифтами: кнопки вызова, отправки на этаж и перемещения между этажами.
    • Появились дополнительные возможности взаимодействия с лифтами через админские действия для быстрого перемещения и проверки.
  • Bug Fixes

    • Улучшена надежность движения лифта и обновления его состояния.
    • Исправлена синхронизация этажей и кнопок, чтобы изменения применялись сразу и корректно отображались.

@arthu241243

Copy link
Copy Markdown
Author

@WardexOfficial
Не дай кролику настигнуть меня

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Компоненты лифта (ComplexElevatorComponent, ElevatorButtonComponent, ElevatorDoorComponent, ElevatorPointComponent) перенесены из Content.Server в Content.Shared с сетевой синхронизацией. Логика движения лифта переведена на фазовый автомат через новый ElevatorMovingComponent, переписаны телепортация и урон при прибытии, добавлены debug-verb'ы для администраторов и вызовы Dirty в командах управления этажами.

Changes

Переработка системы лифта

Layer / File(s) Summary
Сетевые контракты компонентов лифта
Content.Shared/_Scp/ComplexElevator/ComplexElevatorComponent.cs, Content.Shared/_Scp/ComplexElevator/ElevatorButtonComponent.cs, Content.Server/_Scp/ComplexElevator/ElevatorButtonComponent.cs, Content.Server/_Scp/ComplexElevator/ElevatorPointComponent.cs, Content.Shared/_Scp/ComplexElevator/ElevatorDoorComponent.cs, Content.Shared/_Scp/ComplexElevator/ElevatorPointComponent.cs, Content.Server/_Scp/ComplexElevator/ElevatorMovingComponent.cs
Компоненты лифта перенесены в Content.Shared с атрибутами NetworkedComponent/AutoGenerateComponentState и AutoNetworkedField-полями; добавлены новые поля (TeleportBuckled, TeleportPulled, CrushEntitiesOnArrival, CrushDamage), удалено поле IsMoving; добавлен новый ElevatorMovingComponent с фазами DoorClosing/WaitingForSend/Travelling.
Фазовый автомат движения и телепортация
Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs
Update() переписан на итерацию по ElevatorMovingComponent с фазовыми переходами; добавлены индексы _elevatorIndex/_pointIndex; TeleportToFloor и CollectEntitiesOnElevator переписаны на AABB-поиск и относительные координаты; KillEntitiesInTargetArea и ClearGasInTargetArea обновлены под новые флаги, добавлен IsBlacklisted; HandleButtonPress, GetNextFloor, MoveToFloor и UpdateButtonLights переведены на проверку ElevatorMovingComponent.
Debug-команды для администраторов
Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.Debug.cs
Добавлен IAdminManager и обработчики GetVerbsEvent, добавляющие для админов verb'ы отправки лифта на этаж и мгновенной телепортации с побочными эффектами.
Синхронизация состояния в командах управления этажами
Content.Server/_Scp/ComplexElevator/ElevatorCommands.cs
После операций add/remove/move над списком этажей добавлены вызовы Dirty для сетевой синхронизации компонента.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ElevatorButton
  participant ComplexElevatorSystem
  participant ElevatorMovingComponent
  participant ElevatorDoorComponent
  participant AtmosphereSystem
  participant DamageableSystem

  ElevatorButton->>ComplexElevatorSystem: HandleButtonPress
  ComplexElevatorSystem->>ElevatorMovingComponent: создать/обновить TargetFloor и Phase=DoorClosing
  ComplexElevatorSystem->>ElevatorDoorComponent: закрыть двери
  ComplexElevatorSystem->>AtmosphereSystem: ClearGasInTargetArea
  ComplexElevatorSystem->>DamageableSystem: KillEntitiesInTargetArea
  ComplexElevatorSystem->>ComplexElevatorSystem: TeleportToFloor
  ComplexElevatorSystem->>ElevatorMovingComponent: удалить компонент
Loading

Suggested labels: 🛠️ Есть C# 🛠️


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Ss14 C# Rules ❌ Error В новых компонентах нет XML-doc для публичных полей, а в Update() лифта есть new List/HashSet и LINQ в hot-path. Добавьте /// <summary> ко всем публичным полям/enum и уберите аллокации/LINQ из Update(), переиспользуя буфер HashSet.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive Заголовок связан с изменениями, но слишком общий и не объясняет, что именно исправлено в лифте. Сделайте заголовок конкретнее, например: «Refactor complex elevator movement and shared components».
✅ Passed checks (7 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Ss14 Bridge Sync ✅ Passed В diff PR нет изменений в .agent/.agents/.claude/.cursor/.github; затронуты только elevator-файлы, так что bridge-синхронизация не нарушена.
Ss14 Yaml/Ftl Rules ✅ Passed В PR по сводке изменены только .cs-файлы; .yml/.yaml/.ftl не затронуты, значит этот чек неприменим.
Ss14 Prototype ↔ Ftl Parity ✅ Passed PR меняет только Content.Server/Shared; файлов Resources/Prototypes, Resources/migration.yml и Resources/Locale/*.ftl нет, значит чек неприменим.
Ss14 Prediction Safety ✅ Passed В shared-коде лишь сетевые компоненты/данные; предиктивных сайд-эффектов, random или client-side удаления сущностей не добавлено.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Content.Server/_Scp/ComplexElevator/ComplexElevatorComponent.cs`:
- Around line 5-8: Move ComplexElevatorComponent out of the server-only assembly
into the actual shared location so the type is truly available to both client
and server, and keep its namespace aligned with the shared assembly. Also
replace the mutable List<string> used by Floors under [AutoNetworkedField] with
a clone-safe networked representation or another synchronization approach in
ComplexElevatorComponent so state replication/prediction does not depend on
shared mutable collection state.

In `@Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs`:
- Around line 253-270: `HandleButtonPress()` is doing routing, validation,
execution, and delay setup in one place, but elevator interactions should follow
the OnEvent -> TryDo -> CanDo -> Do pattern. Refactor `ComplexElevatorSystem` by
introducing a public `TryHandleButtonPress(...)` that the event handler calls, a
separate `Can...(...)` check for the moving-state/button-type conditions, and a
dedicated `Do...(...)` method for `MoveToFloor`/`MoveUp`/`MoveDown` plus
`SetButtonDelay`. Keep `HandleButtonPress(...)` as a thin router or replace it
with the new `Try...` entrypoint so each responsibility is split cleanly.
- Around line 89-90: The Update() hot path in ComplexElevatorSystem currently
allocates a new List<EntityUid> every tick for toRemove, which creates per-frame
garbage. Move this buffer to a reusable field on ComplexElevatorSystem, clear it
at the start of Update(), and reuse it in the
EntityQueryEnumerator<ElevatorMovingComponent, ComplexElevatorComponent>() loop
instead of constructing a new list each call.
- Around line 113-117: The elevator flow in ComplexElevatorSystem should not
advance to WaitingForSend unless TryCloseDoorsForFloor actually succeeds.
Capture the boolean result from TryCloseDoorsForFloor in the movement handling
logic, and only set moving.Phase to ElevatorMovementPhase.WaitingForSend and
update MovementStartTime when the door closure returns true; otherwise leave the
elevator in the current state and do not Dirty the movement as ready to send.
- Around line 237-248: The teleport placement in ComplexElevatorSystem uses
manual world-position subtraction before SetCoordinates(), which mixes
coordinate spaces. Update the logic in the entity loop to derive destCoords
through the transform system’s parent-aware conversion (for the destParent)
instead of computing entWorldPos -
_transform.GetWorldPosition(destParent.Value). Keep the fix localized around
targetWorldPos, destParent, and _transform.SetCoordinates so rotated or
nonstandard parents resolve correctly.

In `@Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.Debug.cs`:
- Around line 14-18: Move the two SubscribeLocalEvent registrations from
InitializeDebug() into the main Initialize() method on
ComplexElevatorSystem.Debug, since *System*.cs should register subscriptions
only in Initialize(). Remove InitializeDebug() entirely after relocating the
AddAdminElevatorVerbs and AddAdminPointVerbs subscriptions.
- Around line 66-73: The debug instant-teleport path in
ComplexElevatorSystem.Debug should clear any active elevator motion before
changing floors. Update the Teleport here instantly action inside the Act block
to remove or reset the existing ElevatorMovingComponent on the elevator entity
before setting CurrentFloor, Dirty, and calling TeleportToFloor. Use the
existing debug helpers in ComplexElevatorSystem.Debug.cs to locate the flow and
ensure Update() cannot resume the old route afterward.
🪄 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: 9a6fa3c9-3836-43c5-bfef-9982f80119aa

📥 Commits

Reviewing files that changed from the base of the PR and between 38746af and 1f64c37.

📒 Files selected for processing (8)
  • Content.Server/_Scp/ComplexElevator/ComplexElevatorComponent.cs
  • Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.Debug.cs
  • Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs
  • Content.Server/_Scp/ComplexElevator/ElevatorButtonComponent.cs
  • Content.Server/_Scp/ComplexElevator/ElevatorCommands.cs
  • Content.Server/_Scp/ComplexElevator/ElevatorDoorComponent.cs
  • Content.Server/_Scp/ComplexElevator/ElevatorMovingComponent.cs
  • Content.Server/_Scp/ComplexElevator/ElevatorPointComponent.cs

Comment thread Content.Shared/_Scp/ComplexElevator/ComplexElevatorComponent.cs
Comment thread Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs
Comment thread Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs Outdated
Comment thread Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs
Comment thread Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs
Comment thread Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.Debug.cs
Comment thread Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.Debug.cs
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

RSI Diff Bot; head commit 8b9ed24 merging into 38746af
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/_Scp/Objects/Misc/scp_cage.rsi

State Old New Status
broken Added
door Added

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs`:
- Around line 217-233: The teleport loop in ComplexElevatorSystem currently
ignores the new ComplexElevatorComponent flags, so buckled entities are always
skipped and pull relations are always broken. Update the logic around
TryComp<BuckleComponent>, TryComp<PullableComponent>, and
TryComp<PullerComponent> to consult TeleportBuckled and TeleportPulled before
continuing or calling _pulling.TryStopPull, so entities are only excluded or
detached when the corresponding setting is disabled.
- Around line 184-186: Update GetButtonUseDelay in ComplexElevatorSystem so
IntermediateDelay is only counted when the elevator actually uses the
intermediate floor. When UseIntermediateFloor is false, exclude
elevator.Comp.IntermediateDelay from the button lockout; keep the existing
DoorCloseDelay, SendDelay, and the extra second as appropriate.
- Around line 258-270: The button handling in ComplexElevatorSystem currently
calls SetButtonDelay unconditionally after MoveToFloor, MoveUp, or MoveDown,
which can block the button even when no movement starts. Update the switch path
so SetButtonDelay is only applied when the selected movement method actually
initiates motion, using the return/result of MoveToFloor, MoveUp, and MoveDown
(or equivalent state checks) to guard the delay. Keep the fix localized around
the button action flow in ComplexElevatorSystem and the movement helpers so
failed requests do not trigger use-delay.

In `@Content.Server/_Scp/ComplexElevator/ElevatorMovingComponent.cs`:
- Around line 6-25: The new networked shared component lacks required XML
documentation for its public contract. Add /// <summary> comments to
ElevatorMovingComponent, each public field (TargetFloor, MovementStartTime,
Phase), and the ElevatorMovementPhase enum so the state and phase meanings are
documented for the networked API. Keep the summaries brief but explicit about
what each field represents and how the phase enum is used by
ElevatorMovingComponent.
🪄 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: d080f9ff-c95f-4e58-8be8-29c62eb30ef5

📥 Commits

Reviewing files that changed from the base of the PR and between 1f64c37 and d68cea9.

📒 Files selected for processing (8)
  • Content.Server/_Scp/ComplexElevator/ComplexElevatorComponent.cs
  • Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.Debug.cs
  • Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs
  • Content.Server/_Scp/ComplexElevator/ElevatorButtonComponent.cs
  • Content.Server/_Scp/ComplexElevator/ElevatorCommands.cs
  • Content.Server/_Scp/ComplexElevator/ElevatorDoorComponent.cs
  • Content.Server/_Scp/ComplexElevator/ElevatorMovingComponent.cs
  • Content.Server/_Scp/ComplexElevator/ElevatorPointComponent.cs

Comment thread Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs
Comment thread Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs Outdated
Comment thread Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs
Comment thread Content.Server/_Scp/ComplexElevator/ElevatorMovingComponent.cs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
Content.Shared/_Scp/ComplexElevator/ElevatorButtonComponent.cs (1)

7-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Добавьте XML-документацию для нового сетевого контракта.

Новый shared-компонент ElevatorButtonComponent и ElevatorButtonType не документированы, хотя это публичный сетевой контракт. As per coding guidelines, Each public field in a component must have XML documentation with a /// <summary> comment.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Content.Shared/_Scp/ComplexElevator/ElevatorButtonComponent.cs` around lines
7 - 26, Add XML documentation to the new public networked contract in
ElevatorButtonComponent and ElevatorButtonType: document the component class and
each public field/property-like member (ElevatorId, ButtonType, Floor) with a
/// <summary> comment, and also document the ElevatorButtonType enum and each
enum value so the shared contract matches the coding guidelines.

Source: Coding guidelines

Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs (2)

122-186: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Критично: побочные эффекты (очистка газа/крушение) применяются до подтверждения успеха телепортации; при провале нет выхода из состояния движения.

Во всех трёх переходах (WaitingForSend/промежуточный этаж, WaitingForSend/финальный этаж, Travelling) порядок такой:

ClearGasInTargetArea(uid, floorId);
KillEntitiesInTargetArea((uid, elevator), floorId, exempt);

if (TeleportToFloor(uid, floorId, elevator, toTeleport))
{
    ...
}

Если TeleportToFloor вернёт false (например, TryFindPoint не находит точку или destParent == null), moving.MovementStartTime не обновляется и moving.Phase не меняется — на следующем тике elapsed >= SendDelay/IntermediateDelay останется истинным, и блок выполнится СНОВА: газ и урон в целевой зоне будут применяться каждый тик бесконечно, а лифт навсегда «зависнет» в состоянии движения (кнопки заблокированы через HasComp<ElevatorMovingComponent>). В отличие от этого, фаза DoorClosing корректно обрабатывает провал через toRemove.Add(uid); continue;.

🐛 Предлагаемое исправление (по аналогии с фазой DoorClosing)
                     if (elevator.UseIntermediateFloor)
                     {
                         var toTeleport = CollectEntitiesOnElevator(uid);
                         var exempt = new HashSet<EntityUid>(toTeleport.Select(t => t.Item1));

                         ClearGasInTargetArea(uid, elevator.IntermediateFloorId);
                         KillEntitiesInTargetArea((uid, elevator), elevator.IntermediateFloorId, exempt);

                         if (TeleportToFloor(uid, elevator.IntermediateFloorId, elevator, toTeleport))
                         {
                             elevator.CurrentFloor = elevator.IntermediateFloorId;
                             Dirty(uid, elevator);

                             _audio.PlayPvs(elevator.TravelSound, uid);

                             moving.Phase = ElevatorMovementPhase.Travelling;
                             moving.MovementStartTime = _timing.CurTime;
                         }
+                        else
+                        {
+                            toRemove.Add(uid);
+                        }
                     }

(аналогично для второй ветки else и для фазы Travelling)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs` around lines
122 - 186, The elevator transition logic in ComplexElevatorSystem applies gas
clearing and entity killing before confirming TeleportToFloor succeeds, and it
does not exit the movement state on failure. In the WaitingForSend and
Travelling branches, move the side effects to run only after a successful
TeleportToFloor call, and if teleportation fails, handle it like DoorClosing by
removing the movement state for the elevator entity so it does not retry
forever. Use the existing helpers TeleportToFloor, ClearGasInTargetArea,
KillEntitiesInTargetArea, and the moving.Phase / toRemove flow to keep the
behavior consistent across all three transition paths.

89-197: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Критично: очистка toRemove выполняется внутри while, а не после него — modifying the same-typed query mid-iteration.

foreach (var movingUid in toRemove) (строки 189-196) находится внутри тела while (query.MoveNext(...)) — закрывающая скобка while только на строке 197. Из-за этого:

  1. RemComp<ElevatorMovingComponent>(movingUid) вызывается, пока тот же EntityQueryEnumerator<ElevatorMovingComponent, ComplexElevatorComponent> ещё активен и итерируется по сущностям с этим же компонентом — удаление компонента у одной сущности во время перечисления по её же типу небезопасно (инвалидация архетипа/пропуски/повторная обработка).
  2. Цикл выполняется на КАЖДОЙ итерации внешнего while, а не один раз после его завершения — при нескольких активных лифтах один и тот же toRemove будет повторно обрабатываться N раз (N = число ещё не пройденных сущностей).

As per path instructions, для *System.cs обязателен ss14-standard-optimizations/ss14-ecs-systems контекст, где итерация по EntityQueryEnumerator не должна пересекаться с мутацией того же набора компонентов без deferred-подхода.

🐛 Предлагаемое исправление
         while (query.MoveNext(out var uid, out var moving, out var elevator))
         {
             ...
-
-            foreach (var movingUid in toRemove)
-            {
-                RemComp<ElevatorMovingComponent>(movingUid);
-                if (TryComp<ComplexElevatorComponent>(movingUid, out var movingElevator))
-                {
-                    UpdateButtonLights((movingUid, movingElevator));
-                }
-            }
         }
+
+        foreach (var movingUid in toRemove)
+        {
+            RemComp<ElevatorMovingComponent>(movingUid);
+            if (TryComp<ComplexElevatorComponent>(movingUid, out var movingElevator))
+            {
+                UpdateButtonLights((movingUid, movingElevator));
+            }
+        }
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs` around lines 89
- 197, The cleanup loop in ComplexElevatorSystem.Update is running inside the
EntityQueryEnumerator iteration, which mutates ElevatorMovingComponent while the
same query is still active and also repeats the removals for every entity. Move
the toRemove processing block outside the while(query.MoveNext(...)) loop so all
RemComp<ElevatorMovingComponent> and UpdateButtonLights calls happen after
enumeration finishes, using the existing toRemove list and the Update method as
the main location to fix.

Source: Path instructions

Content.Shared/_Scp/ComplexElevator/ComplexElevatorComponent.cs (1)

49-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Добавьте XML-документацию для новых публичных полей.

TeleportBuckled, TeleportPulled, CrushEntitiesOnArrival, CrushDamage — новый публичный сетевой контракт компонента, но без /// <summary>. As per coding guidelines, Each public field in a component must have XML documentation with a /// <summary> comment. As per path instructions, Document public methods, public API points and DataField contracts via summary tags.

📝 Пример документации
+    /// <summary>
+    /// Переносить ли пристёгнутые (buckled) сущности вместе с лифтом.
+    /// </summary>
     [DataField]
     public bool TeleportBuckled = true;

+    /// <summary>
+    /// Переносить ли сущности, участвующие в pull-связи, вместе с лифтом.
+    /// </summary>
     [DataField]
     public bool TeleportPulled = true;

+    /// <summary>
+    /// Наносить ли урон сущностям, оказавшимся в зоне прибытия лифта.
+    /// </summary>
     [DataField]
     public bool CrushEntitiesOnArrival = true;

+    /// <summary>
+    /// Величина Blunt-урона, применяемого при прибытии, если <see cref="CrushEntitiesOnArrival"/> включено.
+    /// </summary>
     [DataField]
     public float CrushDamage = 2000f;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Content.Shared/_Scp/ComplexElevator/ComplexElevatorComponent.cs` around lines
49 - 60, Add XML <summary> documentation to each new public DataField in
ComplexElevatorComponent: TeleportBuckled, TeleportPulled,
CrushEntitiesOnArrival, and CrushDamage. Update the field declarations in the
component so each one has a concise summary describing its behavior and
contract, matching the existing public API documentation style used elsewhere in
the component.

Sources: Coding guidelines, Path instructions

♻️ Duplicate comments (1)
Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs (1)

213-295: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Дублирование логики сбора сущностей + TeleportBuckled/TeleportPulled по-прежнему не используются.

Новый CollectEntitiesOnElevator (274-295) дублирует фильтрацию buckled/blacklist из внутренней ветки TeleportToFloor (233-253), но не содержит логики остановки pull — то есть теперь в кодовой базе два почти одинаковых прохода по AABB с расхождением в деталях, что усложняет поддержку. Кроме того, поля elevatorComp.TeleportBuckled/TeleportPulled по-прежнему нигде не читаются ни в одном из двух мест — то же самое, что уже обсуждалось ранее без исправления.

Дублирование логики — новое наблюдение; неиспользуемые флаги — повтор ранее поднятого и не устранённого вопроса.

♻️ Предложение

Вынести общий приватный метод CollectTeleportCandidates(EntityUid uid, TransformComponent elevatorTransform, ComplexElevatorComponent elevatorComp), возвращающий List<(EntityUid, Vector2)> и учитывающий TeleportBuckled/TeleportPulled, и использовать его как из TeleportToFloor, так и из CollectEntitiesOnElevator.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs` around lines
213 - 295, The new entity-collection logic is duplicated between TeleportToFloor
and CollectEntitiesOnElevator, and the TeleportBuckled/TeleportPulled flags
still are not being used anywhere. Extract the shared AABB/entity filtering and
pull-stopping behavior into a single private helper such as
CollectTeleportCandidates that takes the elevator transform and
ComplexElevatorComponent, then call it from both TeleportToFloor and
CollectEntitiesOnElevator so the buckled/pulled rules are applied consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs`:
- Around line 128-171: The elevator movement path in
ComplexElevatorSystem.Update is allocating a new HashSet via LINQ each time a
phase transition runs, which is a hot-path issue. Replace the repeated new
HashSet<EntityUid>(toTeleport.Select(...)) calls with a reusable buffer field
(for example an exempt EntityUid set on the system) and populate it with an
explicit loop instead of Select. Apply the same pattern in both travel branches
and the intermediate-delay branch so CollectEntitiesOnElevator, TeleportToFloor,
and KillEntitiesInTargetArea can use the reused exemption set without per-tick
collection allocations.

In `@Content.Shared/_Scp/ComplexElevator/ElevatorDoorComponent.cs`:
- Around line 7-15: Add XML summary documentation to the public networked fields
in ElevatorDoorComponent: both ElevatorId and Floor need /// <summary> comments
to satisfy the component documentation guideline. Update the field declarations
in ElevatorDoorComponent so the public contract is documented consistently with
other components, without changing their DataField/AutoNetworkedField behavior.

In `@Content.Shared/_Scp/ComplexElevator/ElevatorPointComponent.cs`:
- Around line 7-12: Add XML documentation for the public FloorId field in
ElevatorPointComponent by including a /// <summary> comment directly above the
field declaration; keep the summary concise and descriptive so it matches the
component documentation standards for public fields.

---

Outside diff comments:
In `@Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs`:
- Around line 122-186: The elevator transition logic in ComplexElevatorSystem
applies gas clearing and entity killing before confirming TeleportToFloor
succeeds, and it does not exit the movement state on failure. In the
WaitingForSend and Travelling branches, move the side effects to run only after
a successful TeleportToFloor call, and if teleportation fails, handle it like
DoorClosing by removing the movement state for the elevator entity so it does
not retry forever. Use the existing helpers TeleportToFloor,
ClearGasInTargetArea, KillEntitiesInTargetArea, and the moving.Phase / toRemove
flow to keep the behavior consistent across all three transition paths.
- Around line 89-197: The cleanup loop in ComplexElevatorSystem.Update is
running inside the EntityQueryEnumerator iteration, which mutates
ElevatorMovingComponent while the same query is still active and also repeats
the removals for every entity. Move the toRemove processing block outside the
while(query.MoveNext(...)) loop so all RemComp<ElevatorMovingComponent> and
UpdateButtonLights calls happen after enumeration finishes, using the existing
toRemove list and the Update method as the main location to fix.

In `@Content.Shared/_Scp/ComplexElevator/ComplexElevatorComponent.cs`:
- Around line 49-60: Add XML <summary> documentation to each new public
DataField in ComplexElevatorComponent: TeleportBuckled, TeleportPulled,
CrushEntitiesOnArrival, and CrushDamage. Update the field declarations in the
component so each one has a concise summary describing its behavior and
contract, matching the existing public API documentation style used elsewhere in
the component.

In `@Content.Shared/_Scp/ComplexElevator/ElevatorButtonComponent.cs`:
- Around line 7-26: Add XML documentation to the new public networked contract
in ElevatorButtonComponent and ElevatorButtonType: document the component class
and each public field/property-like member (ElevatorId, ButtonType, Floor) with
a /// <summary> comment, and also document the ElevatorButtonType enum and each
enum value so the shared contract matches the coding guidelines.

---

Duplicate comments:
In `@Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs`:
- Around line 213-295: The new entity-collection logic is duplicated between
TeleportToFloor and CollectEntitiesOnElevator, and the
TeleportBuckled/TeleportPulled flags still are not being used anywhere. Extract
the shared AABB/entity filtering and pull-stopping behavior into a single
private helper such as CollectTeleportCandidates that takes the elevator
transform and ComplexElevatorComponent, then call it from both TeleportToFloor
and CollectEntitiesOnElevator so the buckled/pulled rules are applied
consistently.
🪄 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: 4787ec9b-8153-460f-bcfc-1886b18aab7e

📥 Commits

Reviewing files that changed from the base of the PR and between d68cea9 and 08a9f53.

📒 Files selected for processing (7)
  • Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs
  • Content.Server/_Scp/ComplexElevator/ElevatorDoorComponent.cs
  • Content.Server/_Scp/ComplexElevator/ElevatorMovingComponent.cs
  • Content.Shared/_Scp/ComplexElevator/ComplexElevatorComponent.cs
  • Content.Shared/_Scp/ComplexElevator/ElevatorButtonComponent.cs
  • Content.Shared/_Scp/ComplexElevator/ElevatorDoorComponent.cs
  • Content.Shared/_Scp/ComplexElevator/ElevatorPointComponent.cs
💤 Files with no reviewable changes (1)
  • Content.Server/_Scp/ComplexElevator/ElevatorDoorComponent.cs

Comment thread Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs
Comment thread Content.Shared/_Scp/ComplexElevator/ElevatorDoorComponent.cs
Comment thread Content.Shared/_Scp/ComplexElevator/ElevatorPointComponent.cs
@arthu241243

Copy link
Copy Markdown
Author

@WardexOfficial Джокер, иди сюда

@WardexOfficial

Copy link
Copy Markdown
Collaborator

Пока-что не уверен в принятии этого ПРа

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants