Манипулятор для пИИ - #1179
Conversation
github-actions
Bot
added
Status: Needs Review
Changes: C#
Changes: Sprite
Changes: YML
labels
|
RSI Diff Bot; head commit 2b27ddf merging into ff38ac4 Resources/Textures/_White/Interface/Actions/pai.rsi
Resources/Textures/_White/Objects/Specific/pai_manipulator.rsi
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughДобавлены сетевое состояние и события манипулятора PAI, его прототип и ресурсы, клиентская обработка ввода, а также серверная логика создания, захвата, перемещения, взаимодействия и возврата. ChangesМанипулятор PAI
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/_White/PAI/Systems/ManipulatorSystem.cs`:
- Around line 21-30: Update ManipulatorSystem.Initialize to subscribe to the pAI
component’s ComponentShutdown event, then add a handler that calls Detach and
removes the active manipulator entity when the pAI is destroyed. Ensure any
currently grabbed item is detached before deleting the manipulator.
- Around line 122-130: Update OnInteract to set args.Handled after a valid
grabbed entity is successfully passed to _interact.UseInHandInteraction, while
preserving the early return when no entity is grabbed, so the configured
useDelay is applied.
- Line 48: Update the manipulator spawn in the relevant system method to pass
map coordinates from _transform.GetMapCoordinates(uid) instead of
Transform(uid).Coordinates, ensuring the spawned manipulator is created on the
map rather than inside the PAI’s current container.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1bd9659a-2303-4040-a304-4bc43ba8ccd9
⛔ Files ignored due to path filters (6)
Resources/Textures/_White/Interface/Actions/pai.rsi/grab.pngis excluded by!**/*.pngResources/Textures/_White/Interface/Actions/pai.rsi/interact.pngis excluded by!**/*.pngResources/Textures/_White/Interface/Actions/pai.rsi/move.pngis excluded by!**/*.pngResources/Textures/_White/Interface/Actions/pai.rsi/toggle.pngis excluded by!**/*.pngResources/Textures/_White/Objects/Specific/pai_manipulator.rsi/icon.pngis excluded by!**/*.pngResources/Textures/_White/Objects/Specific/pai_manipulator.rsi/rope.pngis excluded by!**/*.png
📒 Files selected for processing (11)
Content.Server/_White/PAI/Systems/ManipulatorSystem.csContent.Shared/PAI/PAIComponent.csContent.Shared/PAI/SharedPAISystem.csContent.Shared/_White/PAI/Components/ManipulatorComponent.csContent.Shared/_White/PAI/Components/UsedByManipulatorComponent.csContent.Shared/_White/PAI/Events/ManipulatorEvent.csResources/Prototypes/Entities/Objects/Fun/pai.ymlResources/Prototypes/_White/Actions/pai.ymlResources/Prototypes/_White/PAI/manipulator.ymlResources/Textures/_White/Interface/Actions/pai.rsi/meta.jsonResources/Textures/_White/Objects/Specific/pai_manipulator.rsi/meta.json
| if (comp.IsReturning) | ||
| return; | ||
|
|
||
| comp.Manipulator = SpawnAtPosition(comp.ManipulatorProto, Transform(uid).Coordinates); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'ManipulatorSystem.cs' . || true
echo "== file outline =="
if [ -f Content.Server/_White/PAI/Systems/ManipulatorSystem.cs ]; then
ast-grep outline Content.Server/_White/PAI/Systems/ManipulatorSystem.cs || true
echo "== relevant lines =="
cat -n Content.Server/_White/PAI/Systems/ManipulatorSystem.cs | sed -n '1,140p'
fi
echo "== transform/mapping symbols usage =="
rg -n "GetMapCoordinates|Transform\\(uid\\)|SpawnAtPosition|ManipulatorProto|Manipulator = " Content.Server Content.Server/_White/PAI -g '*.cs' | head -200Repository: WWhiteDreamProject/wwdpublic
Length of output: 32725
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inspect other manipulator paths =="
cat -n Content.Server/_White/PAI/Systems/ManipulatorSystem.cs | sed -n '130,215p'
echo "== inspect pAI components and prototypes =="
rg -n "ManipulatorComponent|ManuallyAttachTo|IsReturning|TargetWorldPos|GetManuallyAttachedTo|AttachedEntity|ManipulatorProto|Manipulator" Content.Server/_White/PAI Content.Shared/_White/PAI -g '*.cs' | sed -n '1,240p'
echo "== inspect shared transform map-coordinate semantics in repository comments =="
rg -n "GetMapCoordinates|ToMapCoordinates|ManuallyAttachTo|AttachedEntity" Robust.Client/Robust.Server -g '*.cs' 2>/dev/null | head -120 || trueRepository: WWhiteDreamProject/wwdpublic
Length of output: 10471
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate entity coordinates and transform methods =="
rg -n "struct EntityCoordinates|class EntityCoordinates|partial class EntityCoordinates|GetMapCoordinates\\(|ToMapCoordinates\\(|GetParentCoordinates\\(" Robust.Server Robust.Shared -g '*.cs' 2>/dev/null | head -200 || true
echo "== read focused definitions if present =="
rg -l "struct EntityCoordinates|class EntityCoordinates|GetMapCoordinates\\(|ToMapCoordinates\\(|GetMoverCoordinates\\(|GetParentCoordinates\\(" Robust.Server Robust.Shared Robust.Client -g '*.cs' 2>/dev/null | head -50Repository: WWhiteDreamProject/wwdpublic
Length of output: 262
Создавайте манипулятор на карте, а не в текущем контейнере pAI.
Transform(uid).Coordinates сохраняет родителя устройства, поэтому при pAI в руке или рюкзаке манипулятор спавнится внутри этого контейнера, а не на карте рядом с владельцем. Другие части логики уже предполагают map-координаты pAI — для spawn используйте _transform.GetMapCoordinates(uid).
Предлагаемое исправление
- comp.Manipulator = SpawnAtPosition(comp.ManipulatorProto, Transform(uid).Coordinates);
+ comp.Manipulator = SpawnAtPosition(comp.ManipulatorProto, _transform.GetMapCoordinates(uid));📝 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.
| comp.Manipulator = SpawnAtPosition(comp.ManipulatorProto, Transform(uid).Coordinates); | |
| comp.Manipulator = SpawnAtPosition(comp.ManipulatorProto, _transform.GetMapCoordinates(uid)); |
🤖 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/_White/PAI/Systems/ManipulatorSystem.cs` at line 48, Update
the manipulator spawn in the relevant system method to pass map coordinates from
_transform.GetMapCoordinates(uid) instead of Transform(uid).Coordinates,
ensuring the spawned manipulator is created on the map rather than inside the
PAI’s current container.
|
Мне кажется забракуют.... |
RedFoxIV
left a comment
RedFoxIV
left a comment
There was a problem hiding this comment.
в текущем варианте это неликвид, к сожалению
идея интересная, но блять, движение манипулятора через экшены?
надо переделать это чудо природы на использование мышки и стандартных клавиш - авось, можно будет ещё пару интересных фич приделать, по типу ползанья по полу
| [DataField] | ||
| public MapCoordinates? TargetWorldPos; |
| public bool IsActive = false; | ||
|
|
||
| [DataField, ViewVariables(VVAccess.ReadOnly)] | ||
| public bool IsGrabbin = false; |
There was a problem hiding this comment.
| public bool IsGrabbin = false; | |
| public bool IsGrabbing = false; |
| [DataField, ViewVariables(VVAccess.ReadOnly)] | ||
| public bool IsGrabbin = false; |
| if (comp.IsActive) | ||
| { | ||
| Detach(comp); | ||
|
|
||
| comp.IsActive = false; | ||
| comp.IsReturning = true; | ||
| comp.TargetWorldPos = _transform.GetMapCoordinates(uid); | ||
| args.Handled = true; | ||
| } | ||
| else | ||
| { | ||
| if (comp.IsReturning) | ||
| return; | ||
|
|
||
| comp.Manipulator = SpawnAtPosition(comp.ManipulatorProto, Transform(uid).Coordinates); | ||
| var man = comp.Manipulator; | ||
| var visuals = EnsureComp<JointVisualsComponent>(man.Value); | ||
|
|
||
| visuals.Sprite = comp.JointSpite; | ||
| visuals.OffsetA = new Vector2(0f, 0f); | ||
| visuals.Target = GetNetEntity(uid); | ||
| Dirty(man.Value, visuals); | ||
|
|
||
| comp.IsActive = true; | ||
| args.Handled = true; | ||
| } |
There was a problem hiding this comment.
| if (comp.IsActive) | |
| { | |
| Detach(comp); | |
| comp.IsActive = false; | |
| comp.IsReturning = true; | |
| comp.TargetWorldPos = _transform.GetMapCoordinates(uid); | |
| args.Handled = true; | |
| } | |
| else | |
| { | |
| if (comp.IsReturning) | |
| return; | |
| comp.Manipulator = SpawnAtPosition(comp.ManipulatorProto, Transform(uid).Coordinates); | |
| var man = comp.Manipulator; | |
| var visuals = EnsureComp<JointVisualsComponent>(man.Value); | |
| visuals.Sprite = comp.JointSpite; | |
| visuals.OffsetA = new Vector2(0f, 0f); | |
| visuals.Target = GetNetEntity(uid); | |
| Dirty(man.Value, visuals); | |
| comp.IsActive = true; | |
| args.Handled = true; | |
| } | |
| if (comp.IsActive) | |
| { | |
| Detach(comp); | |
| comp.IsActive = false; | |
| comp.IsReturning = true; | |
| comp.TargetWorldPos = _transform.GetMapCoordinates(uid); | |
| args.Handled = true; | |
| return; | |
| } | |
| if (comp.IsReturning) | |
| return; | |
| comp.Manipulator = SpawnAtPosition(comp.ManipulatorProto, Transform(uid).Coordinates); | |
| var man = comp.Manipulator; | |
| var visuals = EnsureComp<JointVisualsComponent>(man.Value); | |
| visuals.Sprite = comp.JointSpite; | |
| visuals.OffsetA = new Vector2(0f, 0f); | |
| visuals.Target = GetNetEntity(uid); | |
| Dirty(man.Value, visuals); | |
| comp.IsActive = true; | |
| args.Handled = true; | |
| private void OnMove(EntityUid uid, ManipulatorComponent comp, ManipulatorMoveActionEvent args) | ||
| { | ||
| if (!comp.IsActive || comp.Manipulator == null || comp.IsReturning) | ||
| return; | ||
|
|
||
| comp.TargetWorldPos = _transform.ToMapCoordinates(args.Target); | ||
| args.Handled = true; | ||
| } |
| continue; | ||
|
|
||
| if (!EntityManager.EntityExists(entity)) | ||
| return; |
There was a problem hiding this comment.
| return; | |
| continue; |
| [DataField, ViewVariables] | ||
| public EntityUid? Manipulator; |
| [DataField, ViewVariables] | ||
| public EntityUid? GrabbedEntity; |
| [DataField] | ||
| public bool IsReturning = false; |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.Shared/_White/PAI/Systems/ManipulatorSystem.cs`:
- Around line 132-149: Обновите Detach и ветку проверки недействительного
манипулятора в Update, вынеся единый cleanup для потерянных Manipulator или
GrabbedEntity. Cleanup должен снимать UsedByManipulatorComponent, если сущность
существует, и безусловно сбрасывать IsGrabbing, GrabbedEntity, Manipulator,
IsActive, IsReturning и TargetWorldPos; не допускайте пропуска очистки при
null-ссылках или отсутствии PhysicsComponent.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 703f4c27-5a96-49d1-be5c-9575a5d157f7
📒 Files selected for processing (2)
Content.Shared/_White/PAI/Components/ManipulatorComponent.csContent.Shared/_White/PAI/Systems/ManipulatorSystem.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Content.Shared/_White/PAI/Components/ManipulatorComponent.cs
| private void Detach(ManipulatorComponent comp) | ||
| { | ||
| if (comp.Manipulator == null) | ||
| return; | ||
|
|
||
| if (comp.GrabbedEntity != null && EntityManager.EntityExists(comp.GrabbedEntity)) | ||
| { | ||
| var ent = comp.GrabbedEntity.Value; | ||
| var manipulatorCoords = _transform.GetMoverCoordinates(comp.Manipulator.Value); | ||
|
|
||
| _transform.AttachToGridOrMap(ent, Transform(ent)); | ||
| _transform.SetCoordinates(ent, manipulatorCoords); | ||
|
|
||
| RemComp<UsedByManipulatorComponent>(ent); | ||
| } | ||
|
|
||
| comp.GrabbedEntity = null; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Сбрасывайте состояние при потере манипулятора или предмета.
Detach не сбрасывает IsGrabbing во всех ветках, а Update просто пропускает манипулятор без PhysicsComponent, сохраняя ненулевой Manipulator. После этого следующий toggle переводит pAI в IsReturning, но возврат никогда не завершается; grab-toggle также остаётся в ветке отпускания. Вынесите единый cleanup для недействительных ссылок и безусловно сбрасывайте IsGrabbing, GrabbedEntity, Manipulator, IsActive, IsReturning и TargetWorldPos, снимая маркер захвата, если сущность ещё существует.
Also applies to: 178-184
🤖 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/_White/PAI/Systems/ManipulatorSystem.cs` around lines 132 -
149, Обновите Detach и ветку проверки недействительного манипулятора в Update,
вынеся единый cleanup для потерянных Manipulator или GrabbedEntity. Cleanup
должен снимать UsedByManipulatorComponent, если сущность существует, и
безусловно сбрасывать IsGrabbing, GrabbedEntity, Manipulator, IsActive,
IsReturning и TargetWorldPos; не допускайте пропуска очистки при null-ссылках
или отсутствии PhysicsComponent.
|
ну вот, ЛКМ это двигать, Q это подобрать, E это активация |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Content.Shared/_White/PAI/Systems/ManipulatorSystem.cs (2)
111-129: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Detachне сбрасываетIsGrabbing— манипулятор навсегда теряет возможность что-либо захватить после первого отпускания.
Detachсбрасывает толькоcomp.GrabbedEntity, но неcomp.IsGrabbing. ПосколькуManipulatorComponentне пересоздаётся при выключении/включении манипулятора, после первого цикла "схватить → Q для отпускания"IsGrabbingнавсегда остаётсяtrue:OnGrabпри каждом следующем нажатии Q будет уходить в веткуDetach(уже no-op, так какGrabbedEntity == null) и никогда не даст захватить новый предмет. Это тот же незакрытый (без ✅) комментарий из прошлого ревью про сброс состояния; также применимо к ветке вUpdate(151-199), где отсутствиеPhysicsComponentу манипулятора пропускает итерацию без сбросаManipulator/IsReturning.🐛 Предлагаемый фикс
comp.GrabbedEntity = null; + comp.IsGrabbing = false; Dirty(uid, comp);Also applies to: 151-199
🤖 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/_White/PAI/Systems/ManipulatorSystem.cs` around lines 111 - 129, Обновите состояние в Detach: после отпускания сущности сбрасывайте comp.IsGrabbing вместе с comp.GrabbedEntity, чтобы следующий вызов OnGrab мог захватить новый объект. Также в ветке Update, где отсутствует PhysicsComponent у манипулятора, сбрасывайте связанные состояния Manipulator и IsReturning перед пропуском итерации.
131-143: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winНе хватает
Dirtyпосле изменения сетевых полей вOnParentChanged.
man.GrabbedEntityиman.IsGrabbingпомеченыAutoNetworkedField, но после сброса здесь не вызываетсяDirty(comp.ManipulatorOwner, man), в отличие отDetachиOnToggle. Клиенты не получат актуальное состояние захвата после того, как предмет был отсоединён внешним изменением родителя.🔧 Предлагаемый фикс
man.GrabbedEntity = null; man.IsGrabbing = false; + Dirty(comp.ManipulatorOwner, man); }🤖 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/_White/PAI/Systems/ManipulatorSystem.cs` around lines 131 - 143, В методе OnParentChanged после сброса man.GrabbedEntity и man.IsGrabbing вызови Dirty(comp.ManipulatorOwner, man), чтобы изменения сетевых полей были отправлены клиентам; сохрани существующие проверки и удаление UsedByManipulatorComponent.
🤖 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.Client/_White/PAI/ManipulatorSystem.cs`:
- Around line 21-26: Добавьте переопределение Shutdown для отписки обработчика
OnKey от _input.FirstChanceOnKeyEvent перед завершением системы; сохраните
корректный вызов базового Shutdown, чтобы повторная инициализация не накапливала
подписчиков.
In `@Content.Shared/_White/PAI/Systems/ManipulatorSystem.cs`:
- Around line 87-97: В методе OnMove проверяйте, что msg.Coords находится на той
же карте, что и ent, и ограничивайте целевую позицию максимальной допустимой
дистанцией от владельца перед присваиванием man.TargetWorldPos. Отклоняйте
координаты с другим MapId и нормализуйте слишком удалённые координаты до границы
допустимого радиуса.
- Around line 99-109: Добавьте защиту от частых вызовов в OnInteract: ограничьте
обработку ManipulatorInteractEvent задержкой, эквивалентной прежнему useDelay,
прежде чем вызывать _interact.UseInHandInteraction. Отклоняйте события до
истечения задержки и обновляйте таймер только при принятом взаимодействии,
сохранив существующие проверки сущности и GrabbedEntity.
- Around line 61-85: Добавьте в OnGrab серверную валидацию target до вызова
_transform.SetParent: убедитесь, что NetEntity существует и разрешается в
сущность, находится в допустимом радиусе манипулятора, имеет ItemComponent и
BodyStatus.OnGround, не заанкорена и не находится в контейнере, а также не
является владельцем ПАИ или самой сущностью манипулятора. При любой непрошедшей
проверке немедленно отклоняйте захват, сохранив текущую логику только для
валидной цели.
---
Outside diff comments:
In `@Content.Shared/_White/PAI/Systems/ManipulatorSystem.cs`:
- Around line 111-129: Обновите состояние в Detach: после отпускания сущности
сбрасывайте comp.IsGrabbing вместе с comp.GrabbedEntity, чтобы следующий вызов
OnGrab мог захватить новый объект. Также в ветке Update, где отсутствует
PhysicsComponent у манипулятора, сбрасывайте связанные состояния Manipulator и
IsReturning перед пропуском итерации.
- Around line 131-143: В методе OnParentChanged после сброса man.GrabbedEntity и
man.IsGrabbing вызови Dirty(comp.ManipulatorOwner, man), чтобы изменения сетевых
полей были отправлены клиентам; сохрани существующие проверки и удаление
UsedByManipulatorComponent.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e98af917-9875-4697-a0fc-7009e202d03f
📒 Files selected for processing (8)
Content.Client/_White/PAI/ManipulatorSystem.csContent.Shared/PAI/PAIComponent.csContent.Shared/PAI/SharedPAISystem.csContent.Shared/_White/PAI/Components/ManipulatorComponent.csContent.Shared/_White/PAI/Events/ManipulatorEvent.csContent.Shared/_White/PAI/Systems/ManipulatorSystem.csResources/Prototypes/_White/Actions/pai.ymlResources/Textures/_White/Interface/Actions/pai.rsi/meta.json
💤 Files with no reviewable changes (3)
- Resources/Textures/_White/Interface/Actions/pai.rsi/meta.json
- Resources/Prototypes/_White/Actions/pai.yml
- Content.Shared/PAI/PAIComponent.cs
| public override void Initialize() | ||
| { | ||
| base.Initialize(); | ||
|
|
||
| _input.FirstChanceOnKeyEvent += OnKey; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Отписывайтесь от FirstChanceOnKeyEvent в Shutdown.
Подписка _input.FirstChanceOnKeyEvent += OnKey; не снимается — нет переопределения Shutdown(). При повторной инициализации системы (реконнект и т.п.) подписчики будут накапливаться, что приведёт к дублированию сетевых событий на одно нажатие клавиши.
🔧 Предлагаемый фикс
public override void Initialize()
{
base.Initialize();
_input.FirstChanceOnKeyEvent += OnKey;
}
+
+ public override void Shutdown()
+ {
+ _input.FirstChanceOnKeyEvent -= OnKey;
+
+ base.Shutdown();
+ }📝 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.
| public override void Initialize() | |
| { | |
| base.Initialize(); | |
| _input.FirstChanceOnKeyEvent += OnKey; | |
| } | |
| public override void Initialize() | |
| { | |
| base.Initialize(); | |
| _input.FirstChanceOnKeyEvent += OnKey; | |
| } | |
| public override void Shutdown() | |
| { | |
| _input.FirstChanceOnKeyEvent -= OnKey; | |
| base.Shutdown(); | |
| } |
🤖 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.Client/_White/PAI/ManipulatorSystem.cs` around lines 21 - 26,
Добавьте переопределение Shutdown для отписки обработчика OnKey от
_input.FirstChanceOnKeyEvent перед завершением системы; сохраните корректный
вызов базового Shutdown, чтобы повторная инициализация не накапливала
подписчиков.
| private void OnGrab(ManipulatorGrabEvent msg, EntitySessionEventArgs args) | ||
| { | ||
| if (args.SenderSession.AttachedEntity is not { } ent | ||
| || !TryComp<ManipulatorComponent>(ent, out var man)) | ||
| return; | ||
|
|
||
| if (!man.IsActive || man.Manipulator == null || man.IsReturning) | ||
| return; | ||
|
|
||
| if (man.IsGrabbing) | ||
| { | ||
| Detach(man, ent); | ||
| return; | ||
| } | ||
|
|
||
| var target = GetEntity(msg.Ent); | ||
|
|
||
| _transform.SetParent(target, man.Manipulator.Value); | ||
|
|
||
| man.GrabbedEntity = target; | ||
| var marker = EnsureComp<UsedByManipulatorComponent>(target); | ||
| marker.ManipulatorOwner = ent; | ||
|
|
||
| man.IsGrabbing = true; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Валидируйте цель захвата на сервере — сейчас клиент полностью контролирует, что реперентится.
OnGrab берёт msg.Ent от клиента и сразу вызывает _transform.SetParent(target, man.Manipulator.Value) без каких-либо серверных проверок: не проверяется существование сущности, дистанция до манипулятора, наличие ItemComponent, что цель не заанкорена, что это не сам манипулятор/владелец-пАИ и что цель не в контейнере. Клиентские фильтры в Content.Client/_White/PAI/ManipulatorSystem.cs (радиус 0.1, ItemComponent, BodyStatus.OnGround, не в контейнере) — это только UX-подсказка, модифицированный клиент может отправить ManipulatorGrabEvent напрямую с любым NetEntity, включая закреплённые механизмы, других игроков или сам пАИ, что приведёт к репарентингу произвольных сущностей в мире.
Продублируйте на сервере как минимум те же проверки (наличие сущности, дистанция до манипулятора, ItemComponent, BodyStatus.OnGround, отсутствие анкера, отсутствие в контейнере, цель ≠ манипулятор/владелец) перед SetParent.
🤖 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/_White/PAI/Systems/ManipulatorSystem.cs` around lines 61 - 85,
Добавьте в OnGrab серверную валидацию target до вызова _transform.SetParent:
убедитесь, что NetEntity существует и разрешается в сущность, находится в
допустимом радиусе манипулятора, имеет ItemComponent и BodyStatus.OnGround, не
заанкорена и не находится в контейнере, а также не является владельцем ПАИ или
самой сущностью манипулятора. При любой непрошедшей проверке немедленно
отклоняйте захват, сохранив текущую логику только для валидной цели.
| private void OnMove(ManipulatorMoveEvent msg, EntitySessionEventArgs args) | ||
| { | ||
| if (args.SenderSession.AttachedEntity is not { } ent | ||
| || !TryComp<ManipulatorComponent>(ent, out var man)) | ||
| return; | ||
|
|
||
| if (!man.IsActive || man.Manipulator == null || man.IsReturning) | ||
| return; | ||
|
|
||
| man.TargetWorldPos = msg.Coords; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Ограничьте TargetWorldPos дистанцией/картой владельца.
OnMove присваивает man.TargetWorldPos = msg.Coords без проверки, что координаты находятся на той же карте, что и владелец, и без ограничения максимальной дистанции. Модифицированный клиент может отправить произвольные координаты (в том числе с другим MapId), заставляя манипулятор двигаться в непредсказуемом направлении.
🤖 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/_White/PAI/Systems/ManipulatorSystem.cs` around lines 87 - 97,
В методе OnMove проверяйте, что msg.Coords находится на той же карте, что и ent,
и ограничивайте целевую позицию максимальной допустимой дистанцией от владельца
перед присваиванием man.TargetWorldPos. Отклоняйте координаты с другим MapId и
нормализуйте слишком удалённые координаты до границы допустимого радиуса.
| private void OnInteract(ManipulatorInteractEvent msg, EntitySessionEventArgs args) | ||
| { | ||
| if (args.SenderSession.AttachedEntity is not { } ent | ||
| || !TryComp<ManipulatorComponent>(ent, out var man)) | ||
| return; | ||
|
|
||
| if (man.GrabbedEntity == null) | ||
| return; | ||
|
|
||
| _interact.UseInHandInteraction(ent, man.GrabbedEntity.Value, false, false, true); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Нет анти-спам защиты в OnInteract.
Раньше задержка обеспечивалась через useDelay на action-обработчике; теперь это сырое сетевое событие без какого-либо ограничения частоты — модифицированный клиент может спамить ManipulatorInteractEvent каждый тик, вызывая UseInHandInteraction многократно быстрее, чем позволял старый механизм.
🤖 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/_White/PAI/Systems/ManipulatorSystem.cs` around lines 99 -
109, Добавьте защиту от частых вызовов в OnInteract: ограничьте обработку
ManipulatorInteractEvent задержкой, эквивалентной прежнему useDelay, прежде чем
вызывать _interact.UseInHandInteraction. Отклоняйте события до истечения
задержки и обновляйте таймер только при принятом взаимодействии, сохранив
существующие проверки сущности и GrabbedEntity.
|
Почему... Мне придется оплатить годовой сеанс психотерапии и прорабатывать увиденное |
|
тяжело |
|
обработка ввода нестандартная и грустная, валидация ивентов, прилетающих с клиента, отсутствует. |
4 participants
Описание PR
манипулятор для пИИ
манипулятор для персонального ИИ
манипулятор для персонального искусственного интеллекта
им можно водить туда сюда, хватать и перемещать, использовать и выкидывать
смысл слова МАНИПУЛЯТОР потерян
Медиа
https://youtu.be/A0O4awqkIKk <-- круто клавиши трогательность
Изменения
🆑