Conversation
1. Перенос газов между этажами лифта. 2. Очистка промежуточного этажа от не нужных газов. 3. Поля для включения переноса газов а так же очистки промежуточного этажа от нежелательных газов. 4. метод SetTileMixtureв AtmosphereSystem.API.cs для установки газов на тайле. 5. Заменен звук звукового сигнала лифта при перегрузке или заблокированных дверях.
|
RSI Diff Bot; head commit 0f87458 merging into 507f0bf Resources/Textures/_Scp/Structures/Wallmounts/elevator_buttons.rsi
|
📝 WalkthroughWalkthroughДобавлены клиентские визуалы кнопки лифта (Appearance/RSI и визуализатор), новые enum'ы для состояний кнопки; значительно расширена серверная логика ComplexElevator (движение, телепорт, обработка дверей, газов, аварий, звук); введён маркер Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)Content.@(Shared|Server|Client)/**/*.cs📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🔇 Additional comments (4)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (5)
Resources/Textures/_Scp/Structures/Wallmounts/elevator_buttons.rsi/base.pngis excluded by!**/*.pngResources/Textures/_Scp/Structures/Wallmounts/elevator_buttons.rsi/elevator_elsewhere.pngis excluded by!**/*.pngResources/Textures/_Scp/Structures/Wallmounts/elevator_buttons.rsi/elevator_here.pngis excluded by!**/*.pngResources/Textures/_Scp/Structures/Wallmounts/elevator_buttons.rsi/elevator_moving.pngis excluded by!**/*.pngResources/Textures/_Scp/Structures/Wallmounts/elevator_buttons.rsi/off.pngis excluded by!**/*.png
📒 Files selected for processing (19)
Content.Client/_Scp/ComplexElevator/Visualizers/ElevatorButtonVisualizerSystem.cs(1 hunks)Content.Client/_Scp/ComplexElevator/Visualizers/ElevatorButtonVisualsComponent.cs(1 hunks)Content.Server/Atmos/EntitySystems/AtmosphereSystem.API.cs(1 hunks)Content.Server/Construction/ConstructionSystem.Initial.cs(3 hunks)Content.Server/_Scp/ComplexElevator/ComplexElevatorComponent.cs(1 hunks)Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs(12 hunks)Content.Shared/Construction/SharedConstructionSystem.cs(3 hunks)Content.Shared/RCD/Systems/RCDSystem.cs(2 hunks)Content.Shared/_Scp/ComplexElevator/SharedElevatorButtonVisuals.cs(1 hunks)Content.Shared/_Scp/Construction/ConstructionBlockerComponent.cs(1 hunks)Resources/Locale/en-US/_strings/construction/construction-system.ftl(1 hunks)Resources/Locale/en-US/_strings/rcd/components/rcd-component.ftl(1 hunks)Resources/Locale/ru-RU/_prototypes/_scp/entities/markers/buildblocker.ftl(1 hunks)Resources/Locale/ru-RU/_prototypes/_scp/entities/structures/machines/elevator.ftl(1 hunks)Resources/Locale/ru-RU/_strings/construction/construction-system.ftl(1 hunks)Resources/Locale/ru-RU/_strings/rcd/components/rcd-component.ftl(1 hunks)Resources/Prototypes/_Scp/Entities/Markers/construction_blockers.yml(1 hunks)Resources/Prototypes/_Scp/Entities/Structures/Machines/complex_elevator.yml(4 hunks)Resources/Textures/_Scp/Structures/Wallmounts/elevator_buttons.rsi/meta.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
Resources/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place game assets (textures, audio, etc.) under Resources
Files:
Resources/Locale/en-US/_strings/rcd/components/rcd-component.ftlResources/Locale/ru-RU/_strings/construction/construction-system.ftlResources/Locale/ru-RU/_strings/rcd/components/rcd-component.ftlResources/Locale/ru-RU/_prototypes/_scp/entities/markers/buildblocker.ftlResources/Prototypes/_Scp/Entities/Structures/Machines/complex_elevator.ymlResources/Textures/_Scp/Structures/Wallmounts/elevator_buttons.rsi/meta.jsonResources/Prototypes/_Scp/Entities/Markers/construction_blockers.ymlResources/Locale/ru-RU/_prototypes/_scp/entities/structures/machines/elevator.ftlResources/Locale/en-US/_strings/construction/construction-system.ftl
Content.@(Shared|Server|Client)/**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Organize C# code by component within Content.Shared, Content.Server, and Content.Client
Files:
Content.Server/Atmos/EntitySystems/AtmosphereSystem.API.csContent.Shared/_Scp/ComplexElevator/SharedElevatorButtonVisuals.csContent.Server/_Scp/ComplexElevator/ComplexElevatorComponent.csContent.Shared/_Scp/Construction/ConstructionBlockerComponent.csContent.Server/_Scp/ComplexElevator/ComplexElevatorSystem.csContent.Client/_Scp/ComplexElevator/Visualizers/ElevatorButtonVisualsComponent.csContent.Client/_Scp/ComplexElevator/Visualizers/ElevatorButtonVisualizerSystem.csContent.Server/Construction/ConstructionSystem.Initial.csContent.Shared/Construction/SharedConstructionSystem.csContent.Shared/RCD/Systems/RCDSystem.cs
Resources/Prototypes/**/*.{yml,yaml}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store prototype YAML files under Resources/Prototypes
Files:
Resources/Prototypes/_Scp/Entities/Structures/Machines/complex_elevator.ymlResources/Prototypes/_Scp/Entities/Markers/construction_blockers.yml
🔇 Additional comments (18)
Resources/Locale/ru-RU/_prototypes/_scp/entities/structures/machines/elevator.ftl (2)
19-19: Улучшение ясности названия кнопки.Изменение с "кнопка лифта" на "кнопка вызова лифта" делает название более конкретным и понятным. Это хорошо согласуется с добавлением визуальных состояний кнопок, описанных в целях PR.
22-25: Явные строки для направленных кнопок оправданы.Переход от использования наследования
{ ent-ElevatorButton }к явным строкам ("кнопка вызова лифта вверх" / "вниз") является правильным решением, поскольку:
- Эти кнопки не просто повторяют базовый текст — они содержат направленные квалификаторы
- Явные названия улучшают различимость в UI и логах
- Текст корректен с точки зрения русского языка
Resources/Locale/en-US/_strings/construction/construction-system.ftl (1)
2-2: Изменения выглядят корректно.Строка локализации соответствует функциональности блокировки строительства, добавленной в PR.
Resources/Locale/en-US/_strings/rcd/components/rcd-component.ftl (1)
31-31: LGTM!Английская локализация корректна и согласуется с новой функцией блокировки строительства.
Content.Server/Atmos/EntitySystems/AtmosphereSystem.API.cs (1)
229-241: LGTM!Метод
SetTileMixtureкорректно реализован. Логика валидации сетки, установки газовой смеси и обновления визуалов выглядит правильной.Content.Server/_Scp/ComplexElevator/ComplexElevatorComponent.cs (1)
42-54: LGTM!Добавленные поля конфигурации (
AlarmSound,MaxEntitiesToTeleport,TransferGases,ClearGases) корректно реализуют новую функциональность лифта, описанную в PR. Незначительная корректировкаDoorBlockCheckRange(0.6f → 0.59f) не вызывает опасений.Resources/Textures/_Scp/Structures/Wallmounts/elevator_buttons.rsi/meta.json (1)
1-101: LGTM!Метаданные RSI для спрайтов кнопок лифта корректно оформлены. Определены все необходимые состояния (
base,off,elevator_elsewhere,elevator_moving,elevator_here) с 4 направлениями, что согласуется с новой системой визуализации кнопок.Content.Shared/RCD/Systems/RCDSystem.cs (1)
16-16: LGTM!Правильное добавление using-директивы для нового компонента
ConstructionBlockerComponent.Content.Shared/_Scp/Construction/ConstructionBlockerComponent.cs (1)
1-11: LGTM!Правильная реализация компонента-маркера. Код следует стандартному паттерну для маркерных компонентов: атрибут
[RegisterComponent], наследование отComponent, понятная XML-документация и пустое тело класса.Content.Server/Construction/ConstructionSystem.Initial.cs (2)
5-5: LGTM!Правильное добавление необходимых using-директив для новых компонентов и систем.
Also applies to: 22-22
499-511: LGTM!Проверка блокировщиков строительства реализована корректно:
- Проверка выполняется рано в процессе валидации (до
InRangeUnobstructed)- Правильно получает закреплённые сущности на гриде
- Корректно вызывает
Cleanup()перед ранним выходом- Использует подходящий ключ локализации для отображения сообщения пользователю
Логика предотвращает строительство на тайлах с маркерами блокировки, что соответствует задаче PR.
Content.Shared/Construction/SharedConstructionSystem.cs (3)
1-1: LGTM!Правильное добавление using-директивы для поддержки новой функциональности блокировщиков строительства.
13-15: LGTM!Изменение видимости поля
_mapсprivateнаprotectedпозволяет производным классам использовать систему карт, что необходимо для новой логики проверки блокировщиков.
28-31: LGTM!Обновлённая логика
GetPredicateкорректно фильтрует закреплённые сущности:
- Получает все закреплённые сущности на координатах
- Исключает сущности с
ConstructionBlockerComponentиз набора игнорируемых- Это гарантирует, что блокировщики строительства будут учитываться при проверке препятствий
Изменение правильно интегрируется с новой системой блокировки строительства.
Resources/Prototypes/_Scp/Entities/Markers/construction_blockers.yml (1)
1-14: LGTM!Прототип сущности
BuildBlockerопределён корректно:
- Правильно наследуется от
MarkerBase- Включает необходимый компонент
ConstructionBlocker- Настройка спрайта с двумя слоями (база и стена) обеспечивает визуальную обратную связь
- Использование шейдера
unshadedподходит для маркерных объектовСтруктура соответствует руководству по прототипам.
Content.Shared/_Scp/ComplexElevator/SharedElevatorButtonVisuals.cs (1)
1-22: LGTM!Определения enum для визуализации кнопок лифта реализованы правильно:
ElevatorButtonVisualsиElevatorButtonStateпомечены[NetSerializable]для сетевой синхронизацииElevatorButtonLayersне имеет[NetSerializable], так как используется только на клиенте- Состояния кнопки (
ElevatorHere,ElevatorMoving,ElevatorElsewhere) чётко описывают положение лифта- Использование
byteв качестве базового типа оптимально для перечислений с небольшим количеством значенийКод следует стандартному паттерну для систем визуализации на основе внешнего вида.
Content.Client/_Scp/ComplexElevator/Visualizers/ElevatorButtonVisualsComponent.cs (1)
1-19: LGTM!Компонент визуализации кнопки лифта реализован корректно:
- Словарь
SpriteStateMapправильно связывает состояния лифта со строками состояний спрайтов- Использование
[DataField]позволяет переопределять маппинг через прототипы при необходимости- Атрибут
[ViewVariables(VVAccess.ReadOnly)]обеспечивает доступ для отладки- Инициализация словаря с понятными значениями облегчает поддержку
Компонент следует стандартному паттерну для клиентских визуализаторов.
Content.Client/_Scp/ComplexElevator/Visualizers/ElevatorButtonVisualizerSystem.cs (1)
1-19: LGTM!Система визуализации кнопки лифта реализована правильно:
- Проверки на
nullпредотвращают исключения при отсутствии спрайта- Использование
TryGetDataбезопасно обрабатывает случаи, когда состояние не установленоTryGetValueдля поиска в словаре предотвращает исключения при неизвестных состояниях- Обновление слоя через
SpriteSystem.LayerSetRsiStateсоответствует стандартному паттернуСистема корректно интегрируется с системой внешнего вида и компонентом визуализации.
Resources/Locale/ru-RU/_prototypes/_scp/entities/markers/buildblocker.ftl
Outdated
Show resolved
Hide resolved
Resources/Locale/ru-RU/_strings/construction/construction-system.ftl
Outdated
Show resolved
Hide resolved
Resources/Locale/ru-RU/_strings/rcd/components/rcd-component.ftl
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs (2)
187-188: Разрешите ровно лимит пассажиров.Проверка на
<блокирует лифт даже при точном попадании в лимит и противоречит описанию (“перегруз только при превышении”). Это замечание уже поднималось, но правка отсутствует.- return GetEntitiesInElevator(ent.Owner).Count(e => !HasComp<GhostComponent>(e)) < ent.Comp.MaxEntitiesToTeleport; + return GetEntitiesInElevator(ent.Owner).Count(e => !HasComp<GhostComponent>(e)) <= ent.Comp.MaxEntitiesToTeleport;
499-511: Проверьте наличиеAppearanceComponentпередSetData.Мы всё ещё вызываем
_appearance.SetDataдля каждой кнопки, включая те, у которых по прототипу нетAppearanceComponent, что спамит предупреждениями. Нужно добавить guard (как было предложено ранее) или расширить прототипы.- _appearance.SetData(buttonUid, ElevatorButtonVisuals.ButtonState, state); + if (TryComp<AppearanceComponent>(buttonUid, out _)) + _appearance.SetData(buttonUid, ElevatorButtonVisuals.ButtonState, state);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs(12 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
Content.@(Shared|Server|Client)/**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Organize C# code by component within Content.Shared, Content.Server, and Content.Client
Files:
Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs (2)
139-143: 🔴 КРИТИЧЕСКАЯ ПРОБЛЕМА: Отсутствует параметрshouldKill— проект не скомпилируется.Сигнатура метода принимает только два параметра, но в строке 109 (
StartMovement) метод вызывается БЕЗ третьего параметраshouldKill = false. Однако безусловный вызовKillEntitiesInTargetAreaна строке 141 всё равно убивает всех пассажиров при перемещении на промежуточный этаж, что противоречит логике системы.Требуется:
- Добавить параметр
shouldKillв сигнатуру- Условно вызывать
KillEntitiesInTargetAreaтолько приshouldKill == true- private void MoveToFloorImmediate(Entity<ComplexElevatorComponent> ent, string floorId) + private void MoveToFloorImmediate(Entity<ComplexElevatorComponent> ent, string floorId, bool shouldKill = true) { - KillEntitiesInTargetArea(ent, floorId); - TeleportToFloor(ent, floorId); + if (shouldKill) + KillEntitiesInTargetArea(ent, floorId); + + TeleportToFloor(ent, floorId); }И обновите вызов в
StartMovement(строка 109):- MoveToFloorImmediate(ent, ent.Comp.IntermediateFloorId); + MoveToFloorImmediate(ent, ent.Comp.IntermediateFloorId, shouldKill: false);
491-511: Отсутствует проверкаAppearanceComponentпередSetData.Метод
UpdateButtonLightsвызывает_appearance.SetDataдля всех кнопок лифта, включаяElevatorButtonUp/Down, у которых по прототипу может отсутствоватьAppearanceComponent. Это приведёт к предупреждениям в логе или исключениям. Добавьте проверку наличия компонента перед вызовом.ElevatorButtonState state = ElevatorButtonState.ElevatorElsewhere; if (buttonComp.ButtonType == ElevatorButtonType.CallButton) { if (elevator.Comp.IsMoving) state = ElevatorButtonState.ElevatorMoving; else if (buttonComp.Floor == elevator.Comp.CurrentFloor) state = ElevatorButtonState.ElevatorHere; else state = ElevatorButtonState.ElevatorElsewhere; } - _appearance.SetData(buttonUid, ElevatorButtonVisuals.ButtonState, state); + if (TryComp<AppearanceComponent>(buttonUid, out _)) + _appearance.SetData(buttonUid, ElevatorButtonVisuals.ButtonState, state); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs(12 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
Content.@(Shared|Server|Client)/**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Organize C# code by component within Content.Shared, Content.Server, and Content.Client
Files:
Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs
🔇 Additional comments (4)
Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs (4)
49-57: LGTM: Безопасная обёртка для таймеров.Метод
SpawnCheckedTimerкорректно проверяет существование сущности и флагIsMovingперед выполнением действия, что предотвращает выполнение действий на удалённых или остановленных лифтах.
175-188: LGTM: Проверка лимита сущностей исправлена.Строка 187 теперь использует
<=вместо строгого<, что позволяет лифту двигаться при точном соответствии лимитуMaxEntitiesToTeleport. Это соответствует описанию функциональности в PR.
446-446: Изменение флаговLookupFlagsможет повлиять на блокировку дверей.Добавление флага
LookupFlags.Sensorsв строке 446 изменяет поведение проверки блокировки дверей. Теперь сенсоры будут учитываться при определении, заблокирована ли дверь. Убедитесь, что это ожидаемое поведение и сенсоры не должны игнорироваться при проверке.
211-252: Потенциальная проблема со значениемCurrentFloorпри обработке газов.Строка 240 использует
elevatorComp.CurrentFloorдля определения, находится ли лифт на промежуточном этаже. Однако на момент вызоваTeleportToFloorзначениеCurrentFloorещё может быть старым (до обновления вStartMovementилиPerformIntermediateMovementCheck). Это может привести к некорректному выбору газовой смеси.Рекомендация: передавайте флаг
isFromIntermediateилиsourceFloorIdкак параметр вTeleportToFloor, чтобы избежать зависимости от состоянияCurrentFloor.-private void TeleportToFloor(EntityUid uid, string floorId) +private void TeleportToFloor(EntityUid uid, string floorId, string? sourceFloorId = null)И в строке 240:
-var replacementMixture = CreateReplacementMixture(elevatorComp.CurrentFloor == elevatorComp.IntermediateFloorId); +var replacementMixture = CreateReplacementMixture(sourceFloorId == elevatorComp.IntermediateFloorId);Проверьте логику обновления
CurrentFloorи порядок вызовов вStartMovementиPerformIntermediateMovementCheck.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs (2)
142-146: Критично: вернитеshouldKillи применяйте по назначению.Безусловное убийство на промежуточной площадке ломает UX и повторяет ранее поднятую проблему. Метод должен учитывать флаг.
Примените правку:
- private void MoveToFloorImmediate(Entity<ComplexElevatorComponent> ent, string floorId) + private void MoveToFloorImmediate(Entity<ComplexElevatorComponent> ent, string floorId, bool shouldKill = true) { - KillEntitiesInTargetArea(ent, floorId); - TeleportToFloor(ent, floorId); + if (shouldKill) + KillEntitiesInTargetArea(ent, floorId); + + TeleportToFloor(ent, floorId); }И обновите вызовы (в этом же файле):
- В StartMovement:
MoveToFloorImmediate(ent, ent.Comp.IntermediateFloorId, shouldKill: false);- В PerformIntermediateMovementCheck:
MoveToFloorImmediate(ent, targetFloor, shouldKill: true);
495-516: Проверяйте наличие Appearance перед SetData.Некоторые кнопки не имеют AppearanceComponent; вызов приведёт к ворнингам. Добавьте guard.
- _appearance.SetData(buttonUid, ElevatorButtonVisuals.ButtonState, state); + if (TryComp<AppearanceComponent>(buttonUid, out _)) + _appearance.SetData(buttonUid, ElevatorButtonVisuals.ButtonState, state);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs(12 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
Content.@(Shared|Server|Client)/**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Organize C# code by component within Content.Shared, Content.Server, and Content.Client
Files:
Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs
🔇 Additional comments (3)
Content.Server/_Scp/ComplexElevator/ComplexElevatorSystem.cs (3)
179-192: Емкость лифта: фикс подтверждён.Сравнение изменено на
<=, теперь точное попадание в лимит разрешено. OK.
72-95: Замена magic numbers на Gas enum — хорошо.Повышает устойчивость к изменениям enum и читабельность. Значения по умолчанию и температура заданы корректно.
215-256: Возможная ошибка системы координат при работе с тайлами атмосферы.Индексы тайлов берутся из мировых X/Y (через AABB и каст к int). Для SetTileMixture требуются координаты тайлов в системе грида. На смещённых/повёрнутых гридах и при нецелых оффсетах это даст неправильные ячейки.
Предлагаю:
- Конвертировать мировые координаты в координаты тайлов грида через Map/Transform API (получить локальные координаты на grid и затем Vector2i тайла).
- Избегать
(int)(x + offset.X); использовать корректное округление после проекции в grid‑space.Проверьте корректность с тестом: переместить лифт между разными позициями грида (не (0,0)), сравнить набор затронутых тайлов до/после.
|
Этот PR содержит конфликты, пожалуйста, разрешите их, прежде чем мы сможем его оценить. |
1 participant
Краткое описание
Доработки лифтов
Сссылка на багрепорт/ТЗ/Предложение
Медиа(Видео/Скриншоты)
Changelog
🆑 Neyran
Summary by CodeRabbit