Децентрализация механик Scp173#984
Draft
WardexOfficial wants to merge 6 commits into
Draft
Conversation
Added ScpRestrictMovementOnVisibility component to SCP-173.
Removed unused movement event subscriptions and related methods for Scp173Component.
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughОбзорРефакторинг перемещает логику ограничения движения SCP-173 из специфичной системы в новую универсальную систему Изменения
Оцениваемые затраты на код-ревью🎯 3 (Moderate) | ⏱️ ~20 минут Стихотворение
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 3 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Content.Shared/_Scp/Other/ScpRestrictMovementOnVisibility/ScpRestrictMovementOnVisibilityComponent`:
- Around line 9-12: Поле ContainersMoveBlacklist не помечено как сериализуемое и
потому остаётся null при десериализации; добавьте атрибут [DataField] над
объявлением ContainersMoveBlacklist, точно так же как у ContainersMoveWhitelist,
чтобы YAML/префабные чёрные списки правильно загружались; проверьте в методе
SharedScpRestrictMovementOnVisibilitySystem.IsInContainer() / CheckBoth() что
тип и nullability поля совпадают с ContainersMoveWhitelist.
In
`@Content.Shared/_Scp/Other/ScpRestrictMovementOnVisibility/SharedScpRestrictMovementOnVisibilitySystem`:
- Around line 19-31: Initialize the private field _insideQuery in Initialize()
by assigning it the result of GetEntityQuery<InsideEntityStorageComponent>() so
IsInContainer() isn't using a null/uninitialized query; update the Initialize
method (where SubscribeLocalEvent calls are) to set _insideQuery =
GetEntityQuery<InsideEntityStorageComponent>() before any code paths that may
call IsInContainer().
🪄 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: 4873f7ef-233e-459c-a1c7-f0cef1b1c488
📒 Files selected for processing (4)
Content.Shared/_Scp/Other/ScpRestrictMovementOnVisibility/ScpRestrictMovementOnVisibilityComponentContent.Shared/_Scp/Other/ScpRestrictMovementOnVisibility/SharedScpRestrictMovementOnVisibilitySystemContent.Shared/_Scp/Scp173/SharedScp173System.csResources/Prototypes/_Scp/Entities/Mobs/Player/Scp/Main/scp173.yml
Collaborator
Author
|
@ThereDrD0 этот тоже закинуть бы в Драфт |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Content.Shared/_Scp/Other/ScpRestrictMovementOnVisibility/SharedRestrictMovementOnVisibilitySystem.cs`:
- Line 12: The file name does not match the public class name; rename the file
containing the class SharedScpRestrictMovementOnVisibilitySystem to
SharedScpRestrictMovementOnVisibilitySystem.cs so the C# file name matches the
public class name and follows project conventions.
- Around line 35-88: Change the event handler methods from public to private:
OnAttackAttempt, OnDirectionAttempt, OnMoveAttempt, OnMoveInput, and OnMove in
SharedRestrictMovementOnVisibilitySystem; these handlers are only subscribed via
SubscribeLocalEvent and not part of the public API, so update their access
modifiers to private (ensure no external code relies on them being public).
🪄 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: e9a8413d-d764-48ed-939d-98f9c77d30df
📒 Files selected for processing (2)
Content.Shared/_Scp/Other/ScpRestrictMovementOnVisibility/ScpRestrictMovementOnVisibilityComponent.csContent.Shared/_Scp/Other/ScpRestrictMovementOnVisibility/SharedRestrictMovementOnVisibilitySystem.cs
|
|
||
| namespace Content.Shared._Scp.Other.ScpRestrictMovementOnVisibility; | ||
|
|
||
| public sealed class SharedScpRestrictMovementOnVisibilitySystem : EntitySystem |
Contributor
There was a problem hiding this comment.
Имя файла не соответствует имени класса.
Файл называется SharedRestrictMovementOnVisibilitySystem.cs, а класс — SharedScpRestrictMovementOnVisibilitySystem. По конвенциям C# имя файла должно совпадать с именем класса.
🔧 Предлагаемое исправление
Переименуйте файл в SharedScpRestrictMovementOnVisibilitySystem.cs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Content.Shared/_Scp/Other/ScpRestrictMovementOnVisibility/SharedRestrictMovementOnVisibilitySystem.cs`
at line 12, The file name does not match the public class name; rename the file
containing the class SharedScpRestrictMovementOnVisibilitySystem to
SharedScpRestrictMovementOnVisibilitySystem.cs so the C# file name matches the
public class name and follows project conventions.
Comment on lines
+35
to
+88
| public void OnAttackAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref AttackAttemptEvent args) | ||
| { | ||
| if (IsInContainer(ent, out _)) | ||
| { | ||
| args.Cancel(); | ||
| return; | ||
| } | ||
|
|
||
| if (_watching.IsWatchedByAny(ent, useTimeCompensation: true)) | ||
| { | ||
| args.Cancel(); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| public void OnDirectionAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref ChangeDirectionAttemptEvent args) | ||
| { | ||
| // В контейнере можно двигаться | ||
| if (IsInContainer(ent, out _)) | ||
| return; | ||
|
|
||
| if (!_watching.IsWatchedByAny(ent, useTimeCompensation: true)) | ||
| return; | ||
|
|
||
| args.Cancel(); | ||
| } | ||
|
|
||
| public void OnMoveAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref UpdateCanMoveEvent args) | ||
| { | ||
| // В контейнере можно двигаться | ||
| if (IsInContainer(ent, out _)) | ||
| return; | ||
|
|
||
| if (!_watching.IsWatchedByAny(ent, useTimeCompensation: true)) | ||
| return; | ||
|
|
||
| args.Cancel(); | ||
| } | ||
|
|
||
| public void OnMoveInput(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref MoveInputEvent args) | ||
| { | ||
| // Метод подвязанный на MoveInputEvent так же нужен, вместе с методом на MoveEvent | ||
| // Этот метод исправляет проблему, когда сущность должен мочь двинуться, но ему об этом никто не сказал | ||
| // То есть последний вопрос от сущности МОГУ ЛИ Я ДВИНУТЬСЯ был когда он еще мог двинуться, через MoveEvent | ||
| // Потом он перестал мочь, и следственно больше НЕ МОЖЕТ задать вопрос, может они двинуться | ||
| // Это фикслось в игре сменой направления спрайта мышкой | ||
| // Но данный метод как раз будет спрашивать у сущности, может ли он сдвинуться, когда как раз не двигается | ||
| _blocker.UpdateCanMove(ent); | ||
| } | ||
|
|
||
| public void OnMove(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref MoveEvent args) | ||
| { | ||
| _blocker.UpdateCanMove(ent); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Обработчики событий должны быть private.
Методы OnAttackAttempt, OnDirectionAttempt, OnMoveAttempt, OnMoveInput, OnMove используются только для подписки через SubscribeLocalEvent и не являются частью публичного API системы.
♻️ Предлагаемое исправление
- public void OnAttackAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref AttackAttemptEvent args)
+ private void OnAttackAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref AttackAttemptEvent args)- public void OnDirectionAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref ChangeDirectionAttemptEvent args)
+ private void OnDirectionAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref ChangeDirectionAttemptEvent args)- public void OnMoveAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref UpdateCanMoveEvent args)
+ private void OnMoveAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref UpdateCanMoveEvent args)- public void OnMoveInput(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref MoveInputEvent args)
+ private void OnMoveInput(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref MoveInputEvent args)- public void OnMove(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref MoveEvent args)
+ private void OnMove(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref MoveEvent args)📝 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
| public void OnAttackAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref AttackAttemptEvent args) | |
| { | |
| if (IsInContainer(ent, out _)) | |
| { | |
| args.Cancel(); | |
| return; | |
| } | |
| if (_watching.IsWatchedByAny(ent, useTimeCompensation: true)) | |
| { | |
| args.Cancel(); | |
| return; | |
| } | |
| } | |
| public void OnDirectionAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref ChangeDirectionAttemptEvent args) | |
| { | |
| // В контейнере можно двигаться | |
| if (IsInContainer(ent, out _)) | |
| return; | |
| if (!_watching.IsWatchedByAny(ent, useTimeCompensation: true)) | |
| return; | |
| args.Cancel(); | |
| } | |
| public void OnMoveAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref UpdateCanMoveEvent args) | |
| { | |
| // В контейнере можно двигаться | |
| if (IsInContainer(ent, out _)) | |
| return; | |
| if (!_watching.IsWatchedByAny(ent, useTimeCompensation: true)) | |
| return; | |
| args.Cancel(); | |
| } | |
| public void OnMoveInput(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref MoveInputEvent args) | |
| { | |
| // Метод подвязанный на MoveInputEvent так же нужен, вместе с методом на MoveEvent | |
| // Этот метод исправляет проблему, когда сущность должен мочь двинуться, но ему об этом никто не сказал | |
| // То есть последний вопрос от сущности МОГУ ЛИ Я ДВИНУТЬСЯ был когда он еще мог двинуться, через MoveEvent | |
| // Потом он перестал мочь, и следственно больше НЕ МОЖЕТ задать вопрос, может они двинуться | |
| // Это фикслось в игре сменой направления спрайта мышкой | |
| // Но данный метод как раз будет спрашивать у сущности, может ли он сдвинуться, когда как раз не двигается | |
| _blocker.UpdateCanMove(ent); | |
| } | |
| public void OnMove(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref MoveEvent args) | |
| { | |
| _blocker.UpdateCanMove(ent); | |
| } | |
| private void OnAttackAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref AttackAttemptEvent args) | |
| { | |
| if (IsInContainer(ent, out _)) | |
| { | |
| args.Cancel(); | |
| return; | |
| } | |
| if (_watching.IsWatchedByAny(ent, useTimeCompensation: true)) | |
| { | |
| args.Cancel(); | |
| return; | |
| } | |
| } | |
| private void OnDirectionAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref ChangeDirectionAttemptEvent args) | |
| { | |
| // В контейнере можно двигаться | |
| if (IsInContainer(ent, out _)) | |
| return; | |
| if (!_watching.IsWatchedByAny(ent, useTimeCompensation: true)) | |
| return; | |
| args.Cancel(); | |
| } | |
| private void OnMoveAttempt(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref UpdateCanMoveEvent args) | |
| { | |
| // В контейнере можно двигаться | |
| if (IsInContainer(ent, out _)) | |
| return; | |
| if (!_watching.IsWatchedByAny(ent, useTimeCompensation: true)) | |
| return; | |
| args.Cancel(); | |
| } | |
| private void OnMoveInput(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref MoveInputEvent args) | |
| { | |
| // Метод подвязанный на MoveInputEvent так же нужен, вместе с методом на MoveEvent | |
| // Этот метод исправляет проблему, когда сущность должен мочь двинуться, но ему об этом никто не сказал | |
| // То есть последний вопрос от сущности МОГУ ЛИ Я ДВИНУТЬСЯ был когда он еще мог двинуться, через MoveEvent | |
| // Потом он перестал мочь, и следственно больше НЕ МОЖЕТ задать вопрос, может они двинуться | |
| // Это фикслось в игре сменой направления спрайта мышкой | |
| // Но данный метод как раз будет спрашивать у сущности, может ли он сдвинуться, когда как раз не двигается | |
| _blocker.UpdateCanMove(ent); | |
| } | |
| private void OnMove(Entity<ScpRestrictMovementOnVisibilityComponent> ent, ref MoveEvent args) | |
| { | |
| _blocker.UpdateCanMove(ent); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Content.Shared/_Scp/Other/ScpRestrictMovementOnVisibility/SharedRestrictMovementOnVisibilitySystem.cs`
around lines 35 - 88, Change the event handler methods from public to private:
OnAttackAttempt, OnDirectionAttempt, OnMoveAttempt, OnMoveInput, and OnMove in
SharedRestrictMovementOnVisibilitySystem; these handlers are only subscribed via
SubscribeLocalEvent and not part of the public API, so update their access
modifiers to private (ensure no external code relies on them being public).
Collaborator
|
Ты сам можешь закидывать и возвращать, кнопка есть |
Collaborator
Author
|
Заметка для меня: https://discord.com/channels/1051873590301184031/1498716212396429382/1498716212396429382 #НЕЗАБЫТЬ |
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
2 participants
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
Разбитие механик Scp173 на раздельные системы.
TODO
Summary by CodeRabbit
Изменения