Skip to content

Розумний Револьвер#290

Draft
JustDecor wants to merge 3 commits intoSpaceStationUA:masterfrom
JustDecor:Revolver
Draft

Розумний Револьвер#290
JustDecor wants to merge 3 commits intoSpaceStationUA:masterfrom
JustDecor:Revolver

Conversation

@JustDecor
Copy link

@JustDecor JustDecor commented Feb 8, 2026

About the PR

це найкращий пістолет, який коли-небудь виготовлявся культовий однозарядний армійський пістолет...
Й ім'я його власника — Адамська, також званий, Револьвер Оцелот.
image

відео: https://discord.com/channels/1158407971936677960/1170867356621353051/1470106491649523833

Summary by CodeRabbit

  • New Features
    • Додано Smart Revolver — легендарний револьвер зі системою наведення і налаштовуваними рикошетами.
    • Додано екранний індикатор блокування цілі при прицілюванні.
    • Додано миттєве встановлення/очищення цілі правим кліком та дія для циклування цілей.
    • Додано рикошетні кулі з керуванням кількості відскоків і пошуком цілі.
    • Додано українські рядки інтерфейсу для функцій револьвера.

@coderabbitai
Copy link

coderabbitai bot commented Feb 8, 2026

Walkthrough

Додаються клієнтський оверлей і клієнтська система для вибору цілі револьвера, серверна система SmartRevolver із мережевим повідомленням, компонент SmartRevolver, компонент і система рикошетного снаряду з плануванням траєкторії, а також прототипи зброї, боєприпасів і дія циклування цілей.

Changes

Cohort / File(s) Summary
Client overlay & input
Content.Pirate.Client/_JustDecor/Weapons/SmartRevolver/SmartRevolverOverlay.cs, Content.Pirate.Client/_JustDecor/Weapons/SmartRevolver/SmartRevolverSystem.cs
Додано екранний оверлей для індикації обраної цілі та клієнтську систему, яка прив'язує правий клік до встановлення/очищення цілі й надсилає SmartRevolverSetTargetMessage.
Server SmartRevolver logic
Content.Pirate.Shared/_JustDecor/Weapons/SmartRevolver/SmartRevolverSystem.cs, Content.Pirate.Shared/_JustDecor/Weapons/SmartRevolver/SmartRevolverComponent.cs, Content.Pirate.Shared/_JustDecor/Weapons/SmartRevolver/SmartRevolverMessages.cs
Додано серверну систему для керування встановленням/циклом цілей, обробки клієнтських повідомлень, компонент SmartRevolver з параметрами таргетингу та мережеве повідомлення SmartRevolverSetTargetMessage.
Ricochet projectile feature
Content.Pirate.Shared/_JustDecor/Weapons/Ranged/RicochetProjectileComponent.cs, Content.Pirate.Shared/_JustDecor/Weapons/Ranged/RicochetProjectileSystem.cs
Додано компонент і систему рикошетного снаряду: планування траєкторії, логіка відскоків, гомінг/наведення, обробка колізій і застосування пошкоджень.
Prototypes — actions, ammo, weapon
Resources/Prototypes/_Pirate/_JustDecor/_BigBos/Actions/weapon_actions.yml, Resources/Prototypes/_Pirate/_JustDecor/_BigBos/Entities/smart_revolver.yml, Resources/Prototypes/_Pirate/_JustDecor/_BigBos/Entities/ocelot_ammo.yml
Додано прототип дії для циклування цілей, прототип револьвера з компонентом SmartRevolver, та прототипи патрона й кулі з конфігурацією для рикошетних снарядів.
Localization
Resources/Locale/uk-UA/_Pirate/ui/smart-revolver.ftl
Додано рядки локалізації для інтерфейсу вибору/очищення цілі та повідомлень про відсутність цілей.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Клієнт
    participant Overlay as SmartRevolverOverlay
    participant CSystem as SmartRevolverSystem (Client)
    participant Network as Мережа
    participant SSystem as SmartRevolverSystem (Server)
    participant Projectile as Снаряд
    participant RSystem as RicochetProjectileSystem

    Client->>CSystem: Правий клік по цілі
    CSystem->>CSystem: Перевірка гравця / режиму / предмета
    CSystem->>Network: SmartRevolverSetTargetMessage
    Overlay->>Overlay: Відрендерити індикатор цілі

    Network->>SSystem: Отримано SmartRevolverSetTargetMessage
    SSystem->>SSystem: SetTarget / ClearTarget та оновлення стану

    Client->>SSystem: Подія AmmoShot (вистріл)
    SSystem->>Projectile: Додати RicochetProjectileComponent (ціль, параметри)
    Projectile->>RSystem: Ініціалізація шляху / перевірка LoS
    loop Рух та відскоки
      Projectile->>RSystem: Оновлення положення/швидкості
      RSystem->>Projectile: Обчислити відскок / наведення або прямий hit
    end
    RSystem->>Projectile: Потрапляння у ціль -> застосувати пошкодження, маркувати spent
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

size/XL

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
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.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Розумний Револьвер' (Smart Revolver) directly and clearly summarizes the main change: introduction of a new smart revolver weapon item with advanced targeting and ricochet capabilities.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 24

🤖 Fix all issues with AI agents
In
`@Content.Pirate.Client/_JustDecor/Weapons/SmartRevolver/SmartRevolverOverlay.cs`:
- Around line 61-94: The indicator uses hardcoded pixel values (boxSize,
cornerLength, diamondSize and the +/-5 offsets) which don't scale; update
SmartRevolverOverlay to compute these sizes from UI scale or viewport instead of
literals: replace boxSize = new Vector2(60,60), cornerLength = 12f, diamondSize
= 5f and the fixed offsets with scaled values derived from a UIScale factor (or
viewport size / reference resolution) and use those scaled variables wherever
screenPos, handle.DrawRect and UIBox2.FromDimensions are used so the overlay
preserves visual size across resolutions.
- Line 21: The private field `_transform` in class SmartRevolverOverlay is
unused (assigned in the constructor but never referenced in `Draw`), so remove
the dead field and its constructor initialization to clean up the class: delete
the `_transform` field declaration and the `_transform = ...` assignment in the
SmartRevolverOverlay constructor (and remove any now-unneeded constructor
parameter or using/import related to SharedTransformSystem), or alternatively,
if the transform was intended to be used, update the `Draw` method to reference
`_transform` appropriately—identify `_transform`, the SmartRevolverOverlay
constructor, and the `Draw` method to apply the chosen fix.
- Around line 37-38: The code uses _playerManager.LocalPlayer?.ControlledEntity
which is inconsistent with the rest of the codebase; replace this access with
_playerManager.LocalSession?.AttachedEntity (or _player.LocalEntity where
appropriate) to match existing patterns, and update any null checks and
downstream uses that expect a ControlledEntity to instead handle the
AttachedEntity type returned by LocalSession?.AttachedEntity so behavior remains
correct.

In
`@Content.Pirate.Client/_JustDecor/Weapons/SmartRevolver/SmartRevolverSystem.cs`:
- Around line 48-82: TryHandleInstantTargeting currently sends targets without
client-side range checks; fix by reading the SmartRevolverComponent from
activeItem (e.g. GetComp<SmartRevolverComponent>(activeItem.Value)) and
comparing the distance between the player (use player.Value’s
transform/coordinates) and the clicked coords or target entity position against
component.MaxTargetDistance; if beyond range, raise
SmartRevolverSetTargetMessage(NetEntity.Invalid) (and optionally show local "out
of range" feedback) and return true. Also guard the start of
TryHandleInstantTargeting for a null session where client input contexts may
pass null: either return false immediately when session is null or ensure
downstream code (player, _hands) is only accessed when session is non-null.
Ensure you reference TryHandleInstantTargeting, session, coords, activeItem,
SmartRevolverComponent, MaxTargetDistance, and the existing
SmartRevolverSetTargetMessage when applying the changes.
- Around line 78-81: The click-on-empty handling in SmartRevolverSystem
currently sends RaiseNetworkEvent(new
SmartRevolverSetTargetMessage(GetNetEntity(target))) which can set an
invalid/non-target entity instead of clearing; change the payload to explicitly
use NetEntity.Invalid (or the equivalent invalid sentinel) when target is
null/empty so SmartRevolverSetTargetMessage receives NetEntity.Invalid; update
the code paths that call GetNetEntity(target) (in SmartRevolverSystem) to branch
on missing target and pass NetEntity.Invalid to the message.
- Around line 62-63: Add a using for the SmartRevolver namespace
(Content.Pirate.Shared._JustDecor.Weapons.SmartRevolver) at the top of the file
and replace all fully-qualified type usages of
Content.Pirate.Shared._JustDecor.Weapons.SmartRevolver.SmartRevolverComponent
with the simple SmartRevolverComponent identifier (locations where
HasComp<...>(activeItem) and other references occur inside SmartRevolverSystem);
keep the same generic/type usages and functionality, only simplify the type name
after adding the using.
- Around line 38-40: Реєстрація прив'язки через
CommandBinds.Builder.Bind(EngineKeyFunctions.UseSecondary, new
PointerInputCmdHandler(OnRightClick)).Register<SmartRevolverSystem>() не
визначає пріоритет обробки і може спрацювати раніше UI-обробників (наприклад
EntityMenuUIController); виправте це, або обмеживши реєстрацію лише коли в руках
SmartRevolver (перевірка стану/власності перед Bind), або явно вкажіть пріоритет
через .BindBefore(...)/.BindAfter(...) так щоб обробник OnRightClick виконувався
після/перед потрібних UI-контролерів; також переконайтесь, що жодна зброя не
очікує UseSecondary для стрільби (перевірити GunComponent.UseKey) і скоригуйте
порядок реєстрації відповідно.

In
`@Content.Pirate.Shared/_JustDecor/Weapons/Ranged/RicochetProjectileComponent.cs`:
- Around line 15-34: PlannedPath is marked with AutoNetworkedField which will
serialize the entire List<Vector2> to clients; if the client doesn't need full
waypoint data, remove the AutoNetworkedField attribute from the PlannedPath
declaration (leave DataField or replace with ViewVariables for inspection) or
replace networked serialization with a compact/partial representation sent only
when needed; update any code that relies on the client-side PlannedPath to
instead request the path on demand or use a capped/aggregated structure and keep
the symbol PlannedPath and its uses (and the AutoNetworkedField attribute) in
mind when making the change.
- Around line 43-47: Field CurrentWaypointIndex is unused and misleading; remove
the dead field declaration (public int CurrentWaypointIndex = 0; with
[DataField, AutoNetworkedField]) from RicochetProjectileComponent so only the
active index (CurrentBounces) is used, and search for any residual
references/serialization expectations and update/remap networked fields in
RicochetProjectileSystem and related code to avoid breaking
networking/serialization order; alternatively, if you prefer to standardize on
CurrentWaypointIndex, replace usages of component.CurrentBounces in
RicochetProjectileSystem (and any logic that mutates CurrentBounces) to use
CurrentWaypointIndex instead and keep the networking attributes on that single
field.
- Around line 86-96: Поле(я) SpeedAccumulator, HomingDistance та
CurrentWaypointIndex в компоненті RicochetProjectileComponent є
невикористовуваними і повинні бути видалені; у виправленні видаліть ці поля з
класу і з будь-яких DataField/серіалізації, переконайтеся, що система
RicochetProjectileSystem продовжує користуватися існуючими полями
SpeedBonusPerBounce, HomingAccumulator, HomingDelay та CurrentBounces (посилання
на ці імена допоможе знайти використовувані дані) і запустіть збірку/тести щоб
переконатися, що нічого інше на них не посилається.

In `@Content.Pirate.Shared/_JustDecor/Weapons/Ranged/RicochetProjectileSystem.cs`:
- Around line 50-79: In OnPreventCollide, the LoS shortcut currently allows
collision as soon as CurrentBounces >= 1 regardless of a planned path; change
the logic so HasDirectLineOfSight only causes an early return (allow collision)
when either component.FollowPlannedPath is false OR CurrentBounces >= maxBounces
(where maxBounces is computed as shown), i.e. only permit LoS shortcut if not
following a planned path or all planned bounces have been completed; adjust the
condition around HasDirectLineOfSight(...) accordingly so args.Cancelled remains
true when FollowPlannedPath is true and remaining bounces exist.
- Around line 143-157: HandleTargetHit currently skips the normal
ProjectileHitEvent pipeline and calls _damageable.TryChangeDamage directly, and
also lacks a Deleted(...) check for component.Target.Value; update
HandleTargetHit (still inside the TryComp<ProjectileComponent> branch) to first
verify the target isn't deleted (Deleted(component.Target.Value)) before
proceeding, then replace the direct _damageable.TryChangeDamage(...) call with
raising or dispatching the standard ProjectileHitEvent for the projectile/target
so armor/shields and other subscribers run, and after raising the event still
set proj.DeleteOnCollide = true, proj.ProjectileSpent = true, zero the velocity
and call PredictedQueueDel(uid) so the projectile is cleaned up after event
processing. Ensure you reference ProjectileHitEvent and
RicochetProjectileComponent/ProjectileComponent when making the change.
- Around line 81-141: The Update loop uses a framerate-dependent interpolation
(factor = component.SteeringStrength * frameTime * 50f) fed into Vector2.Lerp
which makes steering behave differently at different FPS; change to a
frameTime-independent interpolation by converting the responsiveness into an
exponential/smoothing alpha per-frame (e.g. alpha = 1 - exp(-responsiveness *
frameTime)) and use that alpha in Vector2.Lerp instead of Math.Min(factor,
1.0f); adjust the responsiveness constant (previously baked into 50f) as a new
tunable (still referencing component.SteeringStrength) so the steer math in
Update / the variables currentDir, towardsTarget, factor, and Vector2.Lerp
becomes deterministic across framerates.
- Around line 299-331: TryRecursiveSolve currently does up to 36^depth
IntersectRay calls causing exponential cost; add a global raycast counter (e.g.
int raycastCount) with a configurable MaxRaycasts threshold and decrement/check
it on every IntersectRay call so the recursion early-returns null when the
budget is exceeded; thread this counter through TryRecursiveSolve (add a
parameter or encapsulate in a small state struct) and initialize/reset it in
CalculateRicochetPath before the first call; optionally increase AngleSearchStep
(e.g. to 30f) or make it configurable to further reduce branching.
- Around line 159-235: OnProjectileHit: fix three issues—(1) correct the homing
accumulator assignment: replace the inverted MathF.Min(component.HomingDelay,
0.05f) with the intended behavior (either assign a fixed short delay like 0.05f
if you want a short post-bounce delay, or use MathF.Max(component.HomingDelay,
0.05f) if HomingDelay is meant as an upper bound) and set
component.HomingAccumulator accordingly; (2) remove the magic number when
extending lifetime (timed.Lifetime += 3f) by introducing a named constant or a
field on RicochetProjectileComponent (e.g., BounceLifetimeBonus) and use that
instead; (3) stop approximating the surface normal from entity positions—replace
the normal calculation (currentPos -
_transform.GetWorldPosition(args.Target)).Normalized() with the actual
collision/impact normal provided by the hit event or physics contact (use
args.Normal or query the physics contact manifold for the collision normal) so
CalculateDynamicBounce and the position pushout use the true surface normal.
- Around line 306-311: The hot path is allocating a List for every IntersectRay
call by calling .ToList(); in TryRecursiveSolve (loop over angles) and
HasDirectLineOfSight you should stop materializing the IEnumerable and instead
take only the first hit—replace usages of _physics.IntersectRay(...).ToList()
with a non-allocating operation that retrieves the first result (e.g., use
.FirstOrDefault() or manual foreach to grab the first element) and update
subsequent code that expects a list to handle a single hit or null accordingly.

In
`@Content.Pirate.Shared/_JustDecor/Weapons/SmartRevolver/SmartRevolverSystem.cs`:
- Around line 247-265: IsValidTarget uses OR (HasComp<MobStateComponent> ||
HasComp<DamageableComponent>) while IsValidCycleTarget uses AND
(HasComp<MobStateComponent> && HasComp<DamageableComponent>), which leads to
inconsistent target selection; update the methods so they use the same logical
condition (pick either OR in both or AND in both) or, if the difference is
intentional, add a clear comment above both IsValidTarget and IsValidCycleTarget
explaining the rationale and expected behavior so future readers understand why
cycle selection differs from direct selection; reference the method names
IsValidTarget and IsValidCycleTarget and ensure the chosen logic is applied
consistently across any call sites that rely on these checks.
- Around line 111-145: In OnAmmoShot, avoid using gun.Target as an unsafe
fallback for component.SelectedTarget; instead validate the chosen target
(component.SelectedTarget ?? gun.Target) with an IsValidTarget check (e.g.,
existence, not deleted, and within SmartRevolverComponent range) before
assigning ricochet data, and if it fails call ClearTarget(uid, component, null)
and return; update references to Target, TargetBounces, MaxBounces,
FollowPlannedPath and Dirty only after this validation so ricochet is never set
toward an out-of-range or non-entity target.
- Around line 155-169: OnCycleTarget lacks a server check and uses a hardcoded
English popup; add an early guard that returns if !_net.IsServer in
OnCycleTarget, and replace the string passed to _popup.PopupEntity("No valid
targets in view!", uid, PopupType.Medium) with a Loc.GetString call (e.g.
Loc.GetString("smart-revolver-no-valid-targets")). Also update the hardcoded
Ukrainian strings in SetTarget and ClearTarget to use Loc.GetString with
appropriate keys so all popups use localization consistently; ensure you call
_popup.PopupEntity with the localized string.
- Around line 106-109: The empty event handler OnAfterInteract (subscribed to
AfterInteractEvent) contains no logic and should be removed to avoid dead code;
either delete the OnAfterInteract method and remove its event subscription where
the handler is registered, or if work is planned, add a clear TODO comment
inside OnAfterInteract describing the intended behavior and keep the
subscription until implemented. Ensure you update/clean the registration site
that attaches OnAfterInteract to AfterInteractEvent (so no dangling subscription
remains) and run a build to confirm no references remain.
- Around line 77-104: Add a distance and visibility check in OnSetTargetMessage
before calling SetTarget: after resolving target (GetEntity(msg.Target)) and
after existing validation (msg.Target != Invalid and IsValidTarget), compute
distance from the user entity to the target and ensure it is <= the
SmartRevolverComponent.MaxTargetDistance (or equivalent field) and verify
visibility using the same visibility check used by UpdateAvailableTargets (e.g.,
line-of-sight/CanSee/visibility helper). If either check fails, call
ClearTarget(heldEntity.Value, component, user.Value) and return; otherwise
proceed to SetTarget(heldEntity.Value, component, target, user.Value). Ensure
you reference SmartRevolverComponent.MaxTargetDistance, OnSetTargetMessage,
UpdateAvailableTargets, SetTarget and ClearTarget when making the change.
- Around line 217-244: UpdateAvailableTargets currently iterates every
TransformComponent via EntityQueryEnumerator<TransformComponent>, causing huge
overhead; inject and use EntityLookupSystem.GetEntitiesInRange instead to only
enumerate candidates within component.MaxTargetDistance (add an
EntityLookupSystem dependency field), replace the EntityQueryEnumerator loop
with a loop over entities returned by GetEntitiesInRange centered on
_transform.GetMapCoordinates(revolverUid) and limited by
component.MaxTargetDistance, and keep the existing checks (IsValidCycleTarget,
_examine.InRangeUnOccluded(user, uid, component.MaxTargetDistance), map id
comparison using _transform.GetMapCoordinates, and the distance check) before
adding to component.AvailableTargets.

In `@Resources/Prototypes/_Pirate/_JustDecor/_BigBos/Entities/ocelot_ammo.yml`:
- Around line 23-27: Remove the unknown field rotationSyncSpeed from the
RicochetProjectile YAML entry: locate the RicochetProjectile block in
ocelot_ammo.yml and delete the rotationSyncSpeed: 0.0 line since
RicochetProjectileComponent does not define it; leave other fields (e.g.,
speedBonusPerBounce) intact because speed scaling is already handled by
RicochetProjectileComponent/System via SpeedRetentionOnBounce.

In `@Resources/Prototypes/_Pirate/_JustDecor/_BigBos/Entities/smart_revolver.yml`:
- Line 6: The description says "Шість куль" while the YAML field capacity is set
to 8; open the smart_revolver.yml entity and make them consistent by either
updating the description string under the description key to reference eight
bullets (or remove the numeric mention) or change the capacity key value from 8
to 6 so lore and data match; ensure you edit the description field and/or the
capacity field (look for the description: line and the capacity: line in the
smart_revolver.yml between the BigBos/Entities block) and keep the flavor text
punctuation/escaping intact.

@Vareniko
Copy link

Vareniko commented Feb 8, 2026

А відео є?

@JustDecor
Copy link
Author

JustDecor commented Feb 8, 2026

@CyberLanos
Copy link

тепер треба ПШІ роль сюди

image

@coderabbitai coderabbitai bot added the size/XL label Feb 8, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

🤖 Fix all issues with AI agents
In
`@Content.Pirate.Client/_JustDecor/Weapons/SmartRevolver/SmartRevolverSystem.cs`:
- Around line 57-105: The method TryHandleInstantTargeting contains redundant
null-propagation and a dead ternary branch: replace session?.AttachedEntity with
session.AttachedEntity (since session is already null-checked) and simplify the
targetPos assignment to always use _transform.GetMapCoordinates(target).Position
(remove the _transform.ToMapCoordinates(coords).Position branch and the
unnecessary target.IsValid() check there), ensuring you still use
target.IsValid() earlier to guard this code path; update references in
TryHandleInstantTargeting accordingly.

In `@Content.Pirate.Shared/_JustDecor/Weapons/Ranged/RicochetProjectileSystem.cs`:
- Around line 229-241: The PlannedPath indexing can throw if the list is mutated
between the Count check and access; in the method containing the block that uses
component.PlannedPath[component.CurrentBounces] (inside the ricochet update that
also calls _physics.SetLinearVelocity and ResetProjectileState), protect the
access by re-checking the list length or obtaining a local snapshot before
indexing (e.g. copy component.PlannedPath to a local variable or read count into
a local and re-validate) or wrap the access in a minimal try/catch that falls
back to the CalculateDynamicBounce path; ensure you reference
component.PlannedPath and component.CurrentBounces and keep the else path that
calls CalculateDynamicBounce and ResetProjectileState as the safe fallback.
- Around line 85-141: The code calls HasDirectLineOfSight every frame for
projectiles in Update (see RicochetProjectileSystem.Update and
RicochetProjectileComponent.CurrentBounces), which is expensive; modify the
component to cache LoS checks (e.g., add a LoSCheckAccumulator or
LastLoSCheckTick and CachedHasLoS) and only call HasDirectLineOfSight when the
accumulator/tick indicates N frames have passed or when projectile state
changes, then use the cached value in the homing condition (replace direct
HasDirectLineOfSight call with the cached check); alternatively document the
per-frame raycast cost near HasDirectLineOfSight usage if you intend to accept
the behavior.
- Around line 295-325: The current normal used in TryRecursiveSolve is an
approximation ((current - hit.HitPos).Normalized()) which can be wrong for
angled surfaces; instead retrieve the true contact normal the same way
OnProjectileHit does: after TryGetFirstRayHit returns, call the existing
TryGetContactNormal (or equivalent physics helper) with hit.HitPos /
hit.HitEntity to obtain the surface normal and use that to compute nextPos (fall
back to the old approximation only if TryGetContactNormal fails); update
TryRecursiveSolve to use the obtained normal when computing nextPos and keep the
visited/raycastBudget logic unchanged.
- Around line 143-162: Handle the double-deletion by adding a guard in
OnProjectileHit to skip processing when the hit corresponds to the ricochet
component's explicit target: check if component.Target is set and args.Target ==
component.Target.Value and return early (thus letting HandleTargetHit fully own
TryChangeDamage, setting proj.ProjectileSpent and calling PredictedQueueDel).
Update OnProjectileHit (referencing OnProjectileHit, HandleTargetHit,
component.Target, PredictedQueueDel, and ProjectileHitEvent) to short-circuit
that case rather than performing deletion there.

In
`@Content.Pirate.Shared/_JustDecor/Weapons/SmartRevolver/SmartRevolverSystem.cs`:
- Around line 61-73: The OnAlternativeVerb handler currently adds the targeting
verb for any valid target without checking range; update OnAlternativeVerb
(handling GetVerbsEvent<AlternativeVerb>) to mirror the distance check used in
OnSetTargetMessage by verifying the distance between args.User and args.Target
against component.MaxTargetDistance (and bail out if beyond that range or not
visible), so the verb is only added when within MaxTargetDistance before calling
SetTarget.
- Around line 164-189: The popup is currently sent to the weapon entity (uid) in
OnCycleTarget which makes it visible to everyone; change the recipient to the
performer so only the acting player sees it: in OnCycleTarget
(SmartRevolverComponent) replace the _popup.PopupEntity call that uses uid as
the target with a call that uses args.Performer (ensuring
args.Performer.IsValid()) as the popup recipient so the message is shown only to
the performer.
- Around line 268-286: IsValidTarget and IsValidCycleTarget contain identical
logic; remove the duplicate IsValidCycleTarget method and update all usages
(e.g., calls from UpdateAvailableTargets) to call IsValidTarget instead; ensure
any remaining references to IsValidCycleTarget are replaced and run a build to
confirm no remaining references.
- Around line 253-262: У функції UpdateAvailableTargets видаліть надлишкові
перевірки, бо _lookup.GetEntitiesInRange(revolverPos,
component.MaxTargetDistance) вже повертає лише сутності на тій самій карті і в
межах радіуса: збережіть перевірку TryComp(uid, out TransformComponent? xform) і
використання _transform.GetMapCoordinates(uid, xform) лише якщо потрібні інші
дані, але приберіть перевірку targetPos.MapId != revolverPos.MapId та ручний
розрахунок/порівняння distance > component.MaxTargetDistance; покладіться на
гарантовані фільтри GetEntitiesInRange (методи/символи: UpdateAvailableTargets,
_lookup.GetEntitiesInRange, TryComp, TransformComponent,
_transform.GetMapCoordinates, component.MaxTargetDistance).
- Around line 145-161: In RicochetProjectileSystem.OnProjectileHit, when you set
projectile component property ProjectileComponent.DeleteOnCollide = true because
component.CurrentBounces >= maxBounces, immediately enqueue the entity for
deletion by calling PredictedQueueDel(uid) right after setting DeleteOnCollide;
modify the OnProjectileHit flow that currently only flips DeleteOnCollide so it
also calls PredictedQueueDel(uid) using the same uid in that branch to ensure
immediate removal.

Comment on lines +57 to +105
private bool TryHandleInstantTargeting(ICommonSession? session, EntityCoordinates coords, EntityUid uid)
{
if (session == null)
return false;

var player = session?.AttachedEntity;
if (player == null)
return false;

// Instant targeting only in combat mode
if (!_combat.IsInCombatMode(player.Value))
return false;

// Must be holding Smart Revolver
if (!_hands.TryGetActiveItem(player.Value, out var activeItem))
return false;

if (!TryComp(activeItem.Value, out SmartRevolverComponent? revolver))
return false;

EntityUid target = uid;

if (!target.IsValid() || target == player || target == activeItem.Value)
{
RaiseNetworkEvent(new SmartRevolverSetTargetMessage(NetEntity.Invalid));
return true;
}

var playerPos = _transform.GetMapCoordinates(player.Value).Position;
var targetPos = target.IsValid()
? _transform.GetMapCoordinates(target).Position
: _transform.ToMapCoordinates(coords).Position;

if ((targetPos - playerPos).Length() > revolver.MaxTargetDistance)
{
RaiseNetworkEvent(new SmartRevolverSetTargetMessage(NetEntity.Invalid));
return true;
}

if (HasComp<MobStateComponent>(target) || HasComp<DamageableComponent>(target))
{
RaiseNetworkEvent(new SmartRevolverSetTargetMessage(GetNetEntity(target)));
return true;
}

// Клік в пусте місце -> очищення цілі
RaiseNetworkEvent(new SmartRevolverSetTargetMessage(NetEntity.Invalid));
return true;
}
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Зайвий оператор ?. та мертва гілка тернарного оператора.

  1. Рядок 62: session вже перевірено на null у рядку 59, тому session?.AttachedEntity може бути спрощено до session.AttachedEntity.
  2. Рядок 86-88: target.IsValid() тернарний оператор завжди буде true в цій точці, оскільки невалідні цілі вже оброблені на рядках 79-83. Гілка false (_transform.ToMapCoordinates(coords).Position) — мертвий код.
♻️ Запропоноване спрощення
-        var player = session?.AttachedEntity;
+        var player = session.AttachedEntity;
-        var targetPos = target.IsValid()
-            ? _transform.GetMapCoordinates(target).Position
-            : _transform.ToMapCoordinates(coords).Position;
+        var targetPos = _transform.GetMapCoordinates(target).Position;
🤖 Prompt for AI Agents
In
`@Content.Pirate.Client/_JustDecor/Weapons/SmartRevolver/SmartRevolverSystem.cs`
around lines 57 - 105, The method TryHandleInstantTargeting contains redundant
null-propagation and a dead ternary branch: replace session?.AttachedEntity with
session.AttachedEntity (since session is already null-checked) and simplify the
targetPos assignment to always use _transform.GetMapCoordinates(target).Position
(remove the _transform.ToMapCoordinates(coords).Position branch and the
unnecessary target.IsValid() check there), ensuring you still use
target.IsValid() earlier to guard this code path; update references in
TryHandleInstantTargeting accordingly.

Comment on lines +85 to +141
public override void Update(float frameTime)
{
base.Update(frameTime);

var query = EntityQueryEnumerator<RicochetProjectileComponent, PhysicsComponent, TransformComponent>();
while (query.MoveNext(out var uid, out var component, out var physics, out var xform))
{
// Reduce homing delay
if (component.HomingAccumulator > 0)
component.HomingAccumulator -= frameTime;

// Sync rotation
var currentVelocity = physics.LinearVelocity;
var currentSpeed = currentVelocity.Length();

if (currentSpeed > 0.1f)
{
_transform.SetWorldRotation(uid, currentVelocity.ToWorldAngle());
}

if (component.Target == null || !component.Target.Value.IsValid() || Deleted(component.Target.Value))
continue;

if (!TryComp(component.Target.Value, out TransformComponent? targetXform))
continue;

var currentPos = _transform.GetWorldPosition(xform);
var targetPos = _transform.GetWorldPosition(targetXform);
var dist = (targetPos - currentPos).Length();

// Manual hit fallback to prevent orbiting/flying through hitboxes
if (dist < 0.6f)
{
HandleTargetHit(uid, component);
continue;
}

// Homing logic
var hasLoS = component.CurrentBounces >= 1 && HasDirectLineOfSight(currentPos, targetPos, xform.MapID, component.Target.Value);

if (component.HomingAccumulator <= 0 && (component.CurrentBounces >= component.TargetBounces || hasLoS))
{
var towardsTarget = (targetPos - currentPos).Normalized();
var currentDir = currentVelocity.Normalized();

if (currentSpeed < 1f) continue;

// Very aggressive steering
var responsiveness = component.SteeringStrength * SteeringResponsiveness;
var alpha = 1f - MathF.Exp(-responsiveness * frameTime);
var newDir = Vector2.Normalize(Vector2.Lerp(currentDir, towardsTarget, MathF.Min(alpha, 1.0f)));
_physics.SetLinearVelocity(uid, newDir * currentSpeed, body: physics);

// Keep projectile alive for ricochet handling.
}
}
}
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

HasDirectLineOfSight виконує рейкаст щокадру для кожного активного снаряда.

Рядок 123: для кожного рикошетного снаряда з CurrentBounces >= 1 викликається HasDirectLineOfSight, що робить рейкаст кожен кадр. При невеликій кількості одночасних снарядів це прийнятно, але варто задокументувати це обмеження або додати throttle (наприклад, перевіряти LoS кожні N кадрів), якщо система масштабуватиметься.

🤖 Prompt for AI Agents
In `@Content.Pirate.Shared/_JustDecor/Weapons/Ranged/RicochetProjectileSystem.cs`
around lines 85 - 141, The code calls HasDirectLineOfSight every frame for
projectiles in Update (see RicochetProjectileSystem.Update and
RicochetProjectileComponent.CurrentBounces), which is expensive; modify the
component to cache LoS checks (e.g., add a LoSCheckAccumulator or
LastLoSCheckTick and CachedHasLoS) and only call HasDirectLineOfSight when the
accumulator/tick indicates N frames have passed or when projectile state
changes, then use the cached value in the homing condition (replace direct
HasDirectLineOfSight call with the cached check); alternatively document the
per-frame raycast cost near HasDirectLineOfSight usage if you intend to accept
the behavior.

Comment on lines +143 to +162
private void HandleTargetHit(EntityUid uid, RicochetProjectileComponent component)
{
if (Deleted(uid) || component.Target == null)
return;

if (Deleted(component.Target.Value))
return;

if (TryComp<ProjectileComponent>(uid, out var proj))
{
var hitEvent = new ProjectileHitEvent(proj.Damage, component.Target.Value, proj.Shooter);
RaiseLocalEvent(uid, ref hitEvent);
_damageable.TryChangeDamage(component.Target.Value, hitEvent.Damage, proj.IgnoreResistances, origin: proj.Shooter);

proj.DeleteOnCollide = true;
proj.ProjectileSpent = true;
_physics.SetLinearVelocity(uid, Vector2.Zero);
PredictedQueueDel(uid);
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

HandleTargetHit викликає RaiseLocalEvent<ProjectileHitEvent>, що активує OnProjectileHit цієї ж системи — подвійне видалення.

Коли HandleTargetHit піднімає ProjectileHitEvent на рядку 154, OnProjectileHit (рядок 164) також спрацьовує. На рядку 166-169 він бачить args.Target == component.Target і викликає PredictedQueueDel(uid). Потім виконання повертається до HandleTargetHit, де на рядку 160 знову викликається PredictedQueueDel(uid).

Результат:

  1. Подвійний PredictedQueueDel на одну сутність.
  2. _damageable.TryChangeDamage на рядку 155 виконується після того, як OnProjectileHit вже запланував видалення сутності.
🐛 Запропоноване виправлення — уникнути повторної обробки

Найпростіше рішення — додати guard в OnProjectileHit або в HandleTargetHit:

     private void OnProjectileHit(EntityUid uid, RicochetProjectileComponent component, ref ProjectileHitEvent args)
     {
+        // HandleTargetHit manages its own cleanup when it raises this event
+        if (Deleted(uid) || Terminating(uid))
+            return;
+
         if (args.Target == component.Target)
         {
             PredictedQueueDel(uid);
             return;
         }

Або перенесіть логіку видалення з OnProjectileHit для випадку влучання в ціль — нехай HandleTargetHit повністю керує цим сценарієм.

🤖 Prompt for AI Agents
In `@Content.Pirate.Shared/_JustDecor/Weapons/Ranged/RicochetProjectileSystem.cs`
around lines 143 - 162, Handle the double-deletion by adding a guard in
OnProjectileHit to skip processing when the hit corresponds to the ricochet
component's explicit target: check if component.Target is set and args.Target ==
component.Target.Value and return early (thus letting HandleTargetHit fully own
TryChangeDamage, setting proj.ProjectileSpent and calling PredictedQueueDel).
Update OnProjectileHit (referencing OnProjectileHit, HandleTargetHit,
component.Target, PredictedQueueDel, and ProjectileHitEvent) to short-circuit
that case rather than performing deletion there.

Comment on lines +229 to +241
if (component.FollowPlannedPath && component.PlannedPath.Count > component.CurrentBounces)
{
var nextWaypoint = component.PlannedPath[component.CurrentBounces];
var direction = (nextWaypoint - currentPos).Normalized();
_physics.SetLinearVelocity(uid, direction * speed, body: physics);
ResetProjectileState(uid);
}
else
{
var velocity = CalculateDynamicBounce(uid, physics, component.Target ?? EntityUid.Invalid, speed, normal);
_physics.SetLinearVelocity(uid, velocity, body: physics);
ResetProjectileState(uid);
}
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Коментар до індексації PlannedPath — коректно, але потребує захисту від невалідного стану.

Рядок 231: component.PlannedPath[component.CurrentBounces] — після інкременту CurrentBounces на рядку 202. Перевірка PlannedPath.Count > component.CurrentBounces на рядку 229 захищає від виходу за межі масиву. Логіка індексації коректна.

Однак, якщо PlannedPath буде очищено зовнішнім кодом між перевіркою та доступом (теоретичний race), це спричинить виключення. Для надійності можна додати try-catch або додаткову перевірку.

🤖 Prompt for AI Agents
In `@Content.Pirate.Shared/_JustDecor/Weapons/Ranged/RicochetProjectileSystem.cs`
around lines 229 - 241, The PlannedPath indexing can throw if the list is
mutated between the Count check and access; in the method containing the block
that uses component.PlannedPath[component.CurrentBounces] (inside the ricochet
update that also calls _physics.SetLinearVelocity and ResetProjectileState),
protect the access by re-checking the list length or obtaining a local snapshot
before indexing (e.g. copy component.PlannedPath to a local variable or read
count into a local and re-validate) or wrap the access in a minimal try/catch
that falls back to the CalculateDynamicBounce path; ensure you reference
component.PlannedPath and component.CurrentBounces and keep the else path that
calls CalculateDynamicBounce and ResetProjectileState as the safe fallback.

Comment on lines +295 to +325
private List<Vector2>? TryRecursiveSolve(Vector2 current, Vector2 target, MapId mapId, int depth, EntityUid targetEnt, HashSet<EntityUid> visited, int currentDepth, ref int raycastBudget)
{
if (depth == 0)
return HasDirectLineOfSight(current, target, mapId, targetEnt, ref raycastBudget) ? new List<Vector2>() : null;

if (currentDepth > 8 || raycastBudget <= 0) return null;

for (float angle = 0; angle < 360; angle += AngleSearchStep)
{
var dir = Angle.FromDegrees(angle).ToWorldVec();
var ray = new CollisionRay(current, dir, (int) (CollisionGroup.Impassable | CollisionGroup.MidImpassable | CollisionGroup.BulletImpassable));
if (!TryGetFirstRayHit(mapId, ray, MaxSearchRadius, out var hit, ref raycastBudget))
continue;

if (visited.Contains(hit.HitEntity)) continue;

var normal = (current - hit.HitPos).Normalized();
var nextPos = hit.HitPos + normal * MinWallDistance;

visited.Add(hit.HitEntity);
var subPath = TryRecursiveSolve(nextPos, target, mapId, depth - 1, targetEnt, visited, currentDepth + 1, ref raycastBudget);
visited.Remove(hit.HitEntity);

if (subPath != null)
{
subPath.Insert(0, hit.HitPos);
return subPath;
}
}
return null;
}
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Нормаль відскоку в TryRecursiveSolve — апроксимація, не реальна нормаль поверхні.

Рядок 311: (current - hit.HitPos).Normalized() — це напрямок від точки зіткнення до початкової позиції, а не нормаль стіни. Для планування траєкторії це працює як грубе наближення, але для стін під кутом може давати неточний nextPos, що призведе до невдалого планування шляху.

Для runtime відскоків (OnProjectileHit) ви вже використовуєте TryGetContactNormal (рядок 209) — це правильно. Але тут, при плануванні, контактів ще немає.

🤖 Prompt for AI Agents
In `@Content.Pirate.Shared/_JustDecor/Weapons/Ranged/RicochetProjectileSystem.cs`
around lines 295 - 325, The current normal used in TryRecursiveSolve is an
approximation ((current - hit.HitPos).Normalized()) which can be wrong for
angled surfaces; instead retrieve the true contact normal the same way
OnProjectileHit does: after TryGetFirstRayHit returns, call the existing
TryGetContactNormal (or equivalent physics helper) with hit.HitPos /
hit.HitEntity to obtain the surface normal and use that to compute nextPos (fall
back to the old approximation only if TryGetContactNormal fails); update
TryRecursiveSolve to use the obtained normal when computing nextPos and keep the
visited/raycastBudget logic unchanged.

Comment on lines +61 to +73
private void OnAlternativeVerb(EntityUid uid, SmartRevolverComponent component, GetVerbsEvent<AlternativeVerb> args)
{
if (!args.CanInteract || !IsValidTarget(args.Target))
return;

args.Verbs.Add(new AlternativeVerb
{
Act = () => SetTarget(uid, component, args.Target, args.User),
Text = Loc.GetString("smart-revolver-target-selection"),
Icon = new SpriteSpecifier.Texture(new ResPath("/Textures/Interface/VerbIcons/scope.svg.192dpi.png")),
Priority = 100
});
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

OnAlternativeVerb не перевіряє дальність до цілі.

Верб додається для будь-якої валідної цілі без перевірки відстані чи видимості. Гравець може відкрити контекстне меню на сутності далеко за межами MaxTargetDistance (наприклад, через Examine або інші механізми) і встановити ціль, яку не можна досягти пострілом.

Додайте перевірку дальності аналогічно до OnSetTargetMessage.

🛡️ Запропоноване виправлення
     private void OnAlternativeVerb(EntityUid uid, SmartRevolverComponent component, GetVerbsEvent<AlternativeVerb> args)
     {
         if (!args.CanInteract || !IsValidTarget(args.Target))
             return;
 
+        if (!_examine.InRangeUnOccluded(args.User, args.Target, component.MaxTargetDistance))
+            return;
+
         args.Verbs.Add(new AlternativeVerb
         {
🤖 Prompt for AI Agents
In
`@Content.Pirate.Shared/_JustDecor/Weapons/SmartRevolver/SmartRevolverSystem.cs`
around lines 61 - 73, The OnAlternativeVerb handler currently adds the targeting
verb for any valid target without checking range; update OnAlternativeVerb
(handling GetVerbsEvent<AlternativeVerb>) to mirror the distance check used in
OnSetTargetMessage by verifying the distance between args.User and args.Target
against component.MaxTargetDistance (and bail out if beyond that range or not
visible), so the verb is only added when within MaxTargetDistance before calling
SetTarget.

Comment on lines +145 to +161
foreach (var projectile in args.FiredProjectiles)
{
if (TryComp<ProjectileComponent>(projectile, out var proj))
{
proj.DeleteOnCollide = false;
Dirty(projectile, proj);
}

var ricochet = EnsureComp<RicochetProjectileComponent>(projectile);
ricochet.Target = target;

ricochet.TargetBounces = _random.Next(component.MinRicochets, component.MaxRicochets + 1);
ricochet.MaxBounces = ricochet.TargetBounces;

ricochet.FollowPlannedPath = true;
Dirty(projectile, ricochet);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "*.cs" | xargs grep -l "RicochetProjectileSystem" | head -20

Repository: SpaceStationUA/Goob-Station

Length of output: 148


🏁 Script executed:

find . -type f -name "*Ricochet*" -o -name "*ricochet*"

Repository: SpaceStationUA/Goob-Station

Length of output: 229


🏁 Script executed:

fd -e cs "Ricochet" --exec grep -l "class RicochetProjectileSystem" {} \;

Repository: SpaceStationUA/Goob-Station

Length of output: 148


🏁 Script executed:

cat -n ./Content.Pirate.Shared/_JustDecor/Weapons/Ranged/RicochetProjectileSystem.cs

Repository: SpaceStationUA/Goob-Station

Length of output: 19875


🏁 Script executed:

cat -n ./Content.Pirate.Shared/_JustDecor/Weapons/Ranged/RicochetProjectileComponent.cs

Repository: SpaceStationUA/Goob-Station

Length of output: 3374


🏁 Script executed:

# Check SmartRevolverSystem to see where DeleteOnCollide is set to false
cat -n ./Content.Pirate.Shared/_JustDecor/Weapons/SmartRevolver/SmartRevolverSystem.cs | sed -n '140,165p'

Repository: SpaceStationUA/Goob-Station

Length of output: 1104


proj.DeleteOnCollide = false дозволяє снаряду залишатися живим, але видалення після рикошетів не гарантовано.

При CurrentBounces >= MaxBounces у RicochetProjectileSystem.OnProjectileHit (рядки 173-180) код встановлює лише DeleteOnCollide = true без негайного виклику PredictedQueueDel(). Це означає, що снаряд залишиться активним до наступного зіткнення або до закінчення часу життя TimedDespawnComponent. Якщо снаряд з якоїсь причини не зіткнеться знову (наприклад, вилетить за межі карти), він може зберігатися невизначено довгий час.

Додайте явний виклик PredictedQueueDel(uid) одразу після встановлення DeleteOnCollide = true при виконанні умови component.CurrentBounces >= maxBounces.

🤖 Prompt for AI Agents
In
`@Content.Pirate.Shared/_JustDecor/Weapons/SmartRevolver/SmartRevolverSystem.cs`
around lines 145 - 161, In RicochetProjectileSystem.OnProjectileHit, when you
set projectile component property ProjectileComponent.DeleteOnCollide = true
because component.CurrentBounces >= maxBounces, immediately enqueue the entity
for deletion by calling PredictedQueueDel(uid) right after setting
DeleteOnCollide; modify the OnProjectileHit flow that currently only flips
DeleteOnCollide so it also calls PredictedQueueDel(uid) using the same uid in
that branch to ensure immediate removal.

Comment on lines +164 to +189
private void OnCycleTarget(EntityUid uid, SmartRevolverComponent component, CycleSmartRevolverTargetEvent args)
{
if (args.Handled)
return;

if (!_net.IsServer)
return;

if (!args.Performer.IsValid())
return;

UpdateAvailableTargets(uid, component, args.Performer);

if (component.AvailableTargets.Count == 0)
{
_popup.PopupEntity(Loc.GetString("smart-revolver-no-valid-targets"), uid, PopupType.Medium);
args.Handled = true;
return;
}

component.CurrentTargetIndex = (component.CurrentTargetIndex + 1) % component.AvailableTargets.Count;
var newTarget = component.AvailableTargets[component.CurrentTargetIndex];

SetTarget(uid, component, newTarget, null);
args.Handled = true;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Попап "немає цілей" транслюється всім гравцям замість лише виконавця.

Рядок 179: _popup.PopupEntity(..., uid, PopupType.Medium) показує попап прив'язаний до зброї (uid) без вказівки конкретного гравця. Це може показати повідомлення всім, хто бачить зброю. Використовуйте args.Performer як отримувача.

🛡️ Запропоноване виправлення
-            _popup.PopupEntity(Loc.GetString("smart-revolver-no-valid-targets"), uid, PopupType.Medium);
+            _popup.PopupEntity(Loc.GetString("smart-revolver-no-valid-targets"), uid, args.Performer, PopupType.Medium);
🤖 Prompt for AI Agents
In
`@Content.Pirate.Shared/_JustDecor/Weapons/SmartRevolver/SmartRevolverSystem.cs`
around lines 164 - 189, The popup is currently sent to the weapon entity (uid)
in OnCycleTarget which makes it visible to everyone; change the recipient to the
performer so only the acting player sees it: in OnCycleTarget
(SmartRevolverComponent) replace the _popup.PopupEntity call that uses uid as
the target with a call that uses args.Performer (ensuring
args.Performer.IsValid()) as the popup recipient so the message is shown only to
the performer.

Comment on lines +253 to +262
if (!TryComp(uid, out TransformComponent? xform))
continue;

var targetPos = _transform.GetMapCoordinates(uid, xform);
if (targetPos.MapId != revolverPos.MapId)
continue;

var distance = (targetPos.Position - revolverPos.Position).Length();
if (distance > component.MaxTargetDistance)
continue;
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Надлишкові перевірки у UpdateAvailableTargets після GetEntitiesInRange.

_lookup.GetEntitiesInRange(revolverPos, component.MaxTargetDistance) вже повертає сутності в межах радіусу. Перевірка MapId (рядок 257) та ручний розрахунок відстані (рядки 260-262) після цього — надлишкові, оскільки GetEntitiesInRange обмежує пошук однією картою та заданим радіусом.

♻️ Запропоноване спрощення
             if (!_examine.InRangeUnOccluded(user, uid, component.MaxTargetDistance))
                 continue;
 
-            if (!TryComp(uid, out TransformComponent? xform))
-                continue;
-
-            var targetPos = _transform.GetMapCoordinates(uid, xform);
-            if (targetPos.MapId != revolverPos.MapId)
-                continue;
-
-            var distance = (targetPos.Position - revolverPos.Position).Length();
-            if (distance > component.MaxTargetDistance)
-                continue;
-
             component.AvailableTargets.Add(uid);
🤖 Prompt for AI Agents
In
`@Content.Pirate.Shared/_JustDecor/Weapons/SmartRevolver/SmartRevolverSystem.cs`
around lines 253 - 262, У функції UpdateAvailableTargets видаліть надлишкові
перевірки, бо _lookup.GetEntitiesInRange(revolverPos,
component.MaxTargetDistance) вже повертає лише сутності на тій самій карті і в
межах радіуса: збережіть перевірку TryComp(uid, out TransformComponent? xform) і
використання _transform.GetMapCoordinates(uid, xform) лише якщо потрібні інші
дані, але приберіть перевірку targetPos.MapId != revolverPos.MapId та ручний
розрахунок/порівняння distance > component.MaxTargetDistance; покладіться на
гарантовані фільтри GetEntitiesInRange (методи/символи: UpdateAvailableTargets,
_lookup.GetEntitiesInRange, TryComp, TransformComponent,
_transform.GetMapCoordinates, component.MaxTargetDistance).

Comment on lines +268 to +286
private bool IsValidTarget(EntityUid target)
{
// Перевіряємо чи ціль існує
if (!Exists(target) || Deleted(target))
return false;

// Перевіряємо наявність MobStateComponent або DamageableComponent
return HasComp<MobStateComponent>(target) ||
HasComp<DamageableComponent>(target);
}

private bool IsValidCycleTarget(EntityUid target)
{
if (!Exists(target) || Deleted(target))
return false;

return HasComp<MobStateComponent>(target) || HasComp<DamageableComponent>(target);
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

IsValidTarget та IsValidCycleTarget тепер ідентичні — дублювання коду.

Обидва методи мають однакову логіку (OR для MobStateComponent / DamageableComponent). Один із них можна видалити і використовувати єдиний метод скрізь.

♻️ Запропоноване виправлення
-    private bool IsValidCycleTarget(EntityUid target)
-    {
-        if (!Exists(target) || Deleted(target))
-            return false;
-
-        return HasComp<MobStateComponent>(target) || HasComp<DamageableComponent>(target);
-    }

Замініть виклики IsValidCycleTarget на IsValidTarget у UpdateAvailableTargets.

🤖 Prompt for AI Agents
In
`@Content.Pirate.Shared/_JustDecor/Weapons/SmartRevolver/SmartRevolverSystem.cs`
around lines 268 - 286, IsValidTarget and IsValidCycleTarget contain identical
logic; remove the duplicate IsValidCycleTarget method and update all usages
(e.g., calls from UpdateAvailableTargets) to call IsValidTarget instead; ensure
any remaining references to IsValidCycleTarget are replaced and run a build to
confirm no remaining references.

@v0idRift
Copy link

v0idRift commented Feb 9, 2026

Тест з рефлекшеном валить цей пістолет

@JustDecor
Copy link
Author

Тест з рефлекшеном валить цей пістолет

Тест з рефлекшеном?

@JustDecor JustDecor marked this pull request as draft February 9, 2026 18:40
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.

5 participants