Conversation
WalkthroughДобавлена серверная обработка пользовательского ввода для разделения стопки через RequestCustomSplit с использованием IPlayerManager и диалога ввода; ввод валидируется и при успешном результате выполняется Split и попытка подобрать объект через Hands.PickupOrDrop с показом попапа. Введён SpawnRandomEntityEntityEffectSystem и дефиниция SpawnRandomEntity для случайного спавна прототипов. В ComboEffects добавлены записи в админлог и заменены вызовы TryChangeDamage на ChangeDamage с учётом результата для логирования. Добавлены/обновлены локали и несколько prototype/recipe/metadata изменений (Pinpointer, Extractable для органов, теги Toolbox, loadouts, реакции/токсины и т.д.). Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
Resources/Prototypes/Catalog/Fills/Items/toolboxes.yml (1)
25-27: Рекомендуется использовать# ADT-Tweak-Start/# ADT-Tweak-Endдля многострочных изменений.Согласно coding guidelines, многострочные изменения в файлах вне папки
/ADT/должны использовать маркеры# ADT-Tweak-Startи# ADT-Tweak-End. Текущий формат# ADT-Tweakна первой строке работает, но не соответствует примеру из базового кода (### Start ADT Tweak/### End ADT Tweakвtoolbox.yml).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Prototypes/Catalog/Fills/Items/toolboxes.yml` around lines 25 - 27, Replace the single-line marker "# ADT-Tweak" used before the multi-line block containing the "type: Tag" entry and its "tags: - Toolbox" with the two-marker style "# ADT-Tweak-Start" and "# ADT-Tweak-End": wrap the entire multi-line change (the "type: Tag" mapping and its "tags" sequence) between these start and end markers so it follows the required multi-line ADT tweak convention used elsewhere (e.g., replace the lone "# ADT-Tweak" comment near the "type: Tag" block with "# ADT-Tweak-Start" immediately before the block and add "# ADT-Tweak-End" immediately after).Content.Shared/Stacks/SharedStackSystem.cs (1)
245-252: Необходимо добавить маркерыADT-Tweak start/end.Метод
RequestCustomSplitдобавлен в файл вне папки/ADT/, но не обёрнут в маркеры// ADT-Tweak startи// ADT-Tweak end. Упоминание в XML-комментарии недостаточно.🔧 Предлагаемое исправление
- /// <summary> - /// ADT-Tweak: Request custom split amount from user - /// </summary> - protected virtual void RequestCustomSplit(Entity<StackComponent> stack, Entity<TransformComponent?> user) - { - // Implemented on server - } + // ADT-Tweak start + /// <summary> + /// Request custom split amount from user + /// </summary> + protected virtual void RequestCustomSplit(Entity<StackComponent> stack, Entity<TransformComponent?> user) + { + // Implemented on server + } + // ADT-Tweak end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/Stacks/SharedStackSystem.cs` around lines 245 - 252, Метод RequestCustomSplit в классе SharedStackSystem был добавлен без обрамления маркерами адаптации; оберните тело (или определение) этого метода комментариями-триггерами "// ADT-Tweak start" перед и "// ADT-Tweak end" после, сохранив XML-комментарий и сигнатуру метода, чтобы инструмент ADT правильно обнаруживал этот твик (ищите метод RequestCustomSplit(Entity<StackComponent> stack, Entity<TransformComponent?> user) и добавьте точные маркеры рядом с ним).Content.Server/Stack/StackSystem.cs (1)
311-346: Рекомендуется добавить проверку на удаление сущности в callback.Согласно паттернам, используемым в других системах проекта (например,
CritMobActionsSystemиPrayerSystem), в callback-ахQuickDialogследует проверять, не была ли сущность удалена, прежде чем с ней взаимодействовать. Если игрок или стак будут удалены пока диалог открыт, текущий код может вызвать ошибки.🔧 Предлагаемое исправление
_quickDialog.OpenDialog<int>( session, Loc.GetString("comp-stack-split-custom-title"), Loc.GetString("comp-stack-split-custom-description", ("count", maxCount)), (int amount) => { + // Guard against entity deletion while dialog was open + if (Deleted(stack.Owner) || Deleted(user.Owner)) + return; + + if (!Resolve(user.Owner, ref user.Comp, false)) + return; + if (amount <= 0) { Popup.PopupCursor(Loc.GetString("comp-stack-split-too-small"), user.Owner, PopupType.Medium); return; } - if (amount > maxCount) + // Re-check current stack count as it may have changed + if (amount > stack.Comp.Count) { Popup.PopupCursor(Loc.GetString("comp-stack-split-too-large"), user.Owner, PopupType.Medium); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Server/Stack/StackSystem.cs` around lines 311 - 346, The QuickDialog callback must re-check that the involved entities still exist and their components are resolvable before using them; update the lambda passed to _quickDialog.OpenDialog in RequestCustomSplit to verify the user entity (user.Owner) and the stack entity (stack.Owner) are still valid (e.g., call Resolve(user.Owner, ref user.Comp, false) and Resolve(stack.Owner, ref stack.Comp, false) or check entity deletion/existence) before calling Split or Hands.PickupOrDrop, and bail out early if either check fails.
🤖 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/ADT/Combat/ComboEffects.cs`:
- Around line 118-120: The admin log entries currently live outside the success
branches and thus report "performed" even when effects failed; for each logged
effect (e.g., in ComboEffectToDowned and other combo effect handlers that call
TryKnockdown, ComboEvents, SetCoordinates, SwapPositions) move the
adminLogger.Add call into the branch that confirms the effect succeeded (e.g.,
after TryKnockdown returns true, after ComboEvents indicates success, or after a
successful SetCoordinates/SwapPositions), or if you must keep it outside, change
the message to explicitly say "attempted" and include the success
boolean/result; update all similar occurrences noted (lines ~118, 427-429,
513-515, 533-535) so logs only claim success when the operation actually
succeeded or are clearly marked as attempts.
- Around line 53-55: Строки, передаваемые в adminLogger.Add (вызовы через
ISharedAdminLogManager) не должны быть литералами — вынесите текст в FTL и
замените литерал на вызов Loc.GetString("adm-log-combo-damage", ("actor",
entMan.ToPrettyString(user)), ("subject", entMan.ToPrettyString(target)),
("damage", damageResult.GetTotal())); добавьте соответствующую запись в .ftl с
ключом adm-log-combo-damage и placeholders для actor/subject/damage; проделайте
аналогичную замену для всех остальных новых adminLogger.Add(...) в файле,
используя те же символы (adminLogger.Add, Loc.GetString,
damageResult.GetTotal(), entMan.ToPrettyString(...)) чтобы локализация работала
корректно.
- Around line 24-27: Reorder the using directives at the top of ComboEffects.cs
so they are alphabetized: place Content.Shared.Administration.Logs,
Content.Shared.Database, Robust.Shared.GameObjects, then Robust.Shared.IoC;
update the existing block (the lines mentioning
Content.Shared.Administration.Logs, Content.Shared.Database, Robust.Shared.IoC,
Robust.Shared.GameObjects) to that exact alphabetical order to satisfy the style
rule (no code logic changes required).
In
`@Content.Shared/ADT/EntityEffects/Effects/EntitySpawning/SpawnRandomEntityEntityEffectSystem.cs`:
- Line 17: Remove the unused dependency field _prototype from the
SpawnRandomEntityEntityEffectSystem: delete the line "[Dependency] private
readonly IPrototypeManager _prototype = default!;" (and drop any resulting
unused using/imports if they become orphaned). Ensure no other members reference
_prototype; if they do, either reintroduce a used dependency or refactor those
references to avoid the field. Adjust constructor/DI registration only if it was
specifically added for this field.
- Around line 67-71: The guidebook text currently hardcodes "random crystal"
inside EntitySpawning.EntityEffectSystem.EntityEffectGuidebookText; replace that
hardcoded string with a localized string by calling Loc.GetString with a new key
(e.g. "entity-effect-random-crystal") and pass the result as the ("entname",
...) parameter instead of the literal "random crystal"; ensure the new key is
added to the FTL localization files and keep other parameters (Probability,
Number) unchanged.
- Around line 27-30: Проверьте перед выбором случайного прототипа, что коллекция
args.Effect.Prototypes не null и не пустая; в методе в
SpawnRandomEntityEntityEffectSystem, перед вызовом
_random.Pick(args.Effect.Prototypes) внутри цикла for (var i = 0; i < quantity;
i++), добавьте защиту (например пропуск итерации или лог и продолжение) когда
Prototypes пустой или null, чтобы избежать аварийного выбора; сохраните
существующую ветку с args.Effect.Predicted без изменений, только добавьте эту
предварительную проверку и соответствующий ранний continue/return.
In `@Resources/Prototypes/Recipes/Reactions/fun.yml`:
- Around line 156-166: The YAML block defining the SpawnRandomEntity reaction
(the !type: SpawnRandomEntity node with the prototypes list including
ShardCrystalGreen, ShardCrystalPink, etc., and predicted: false) is missing the
required ADT-Tweak comment markers; wrap this entire block with the mandatory
comment lines (e.g., add a "# ADT-Tweak-Start" comment immediately before the
!type: SpawnRandomEntity line and a "# ADT-Tweak-End" comment immediately after
the predicted: false line) so the change outside /ADT/ complies with the policy.
---
Nitpick comments:
In `@Content.Server/Stack/StackSystem.cs`:
- Around line 311-346: The QuickDialog callback must re-check that the involved
entities still exist and their components are resolvable before using them;
update the lambda passed to _quickDialog.OpenDialog in RequestCustomSplit to
verify the user entity (user.Owner) and the stack entity (stack.Owner) are still
valid (e.g., call Resolve(user.Owner, ref user.Comp, false) and
Resolve(stack.Owner, ref stack.Comp, false) or check entity deletion/existence)
before calling Split or Hands.PickupOrDrop, and bail out early if either check
fails.
In `@Content.Shared/Stacks/SharedStackSystem.cs`:
- Around line 245-252: Метод RequestCustomSplit в классе SharedStackSystem был
добавлен без обрамления маркерами адаптации; оберните тело (или определение)
этого метода комментариями-триггерами "// ADT-Tweak start" перед и "// ADT-Tweak
end" после, сохранив XML-комментарий и сигнатуру метода, чтобы инструмент ADT
правильно обнаруживал этот твик (ищите метод
RequestCustomSplit(Entity<StackComponent> stack, Entity<TransformComponent?>
user) и добавьте точные маркеры рядом с ним).
In `@Resources/Prototypes/Catalog/Fills/Items/toolboxes.yml`:
- Around line 25-27: Replace the single-line marker "# ADT-Tweak" used before
the multi-line block containing the "type: Tag" entry and its "tags: - Toolbox"
with the two-marker style "# ADT-Tweak-Start" and "# ADT-Tweak-End": wrap the
entire multi-line change (the "type: Tag" mapping and its "tags" sequence)
between these start and end markers so it follows the required multi-line ADT
tweak convention used elsewhere (e.g., replace the lone "# ADT-Tweak" comment
near the "type: Tag" block with "# ADT-Tweak-Start" immediately before the block
and add "# ADT-Tweak-End" immediately after).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8723526d-af7f-4adc-9559-ecae23ab4e1f
📒 Files selected for processing (13)
Content.Server/Stack/StackSystem.csContent.Shared/ADT/Combat/ComboEffects.csContent.Shared/ADT/EntityEffects/Effects/EntitySpawning/SpawnRandomEntityEntityEffectSystem.csContent.Shared/Stacks/SharedStackSystem.csResources/Locale/en-US/ADT/stack.ftlResources/Locale/ru-RU/ADT/stack.ftlResources/Prototypes/ADT/Entities/Objects/Devices/syndicate_pinpointer.ymlResources/Prototypes/Body/Organs/Animal/animal.ymlResources/Prototypes/Catalog/Fills/Items/toolboxes.ymlResources/Prototypes/Entities/Objects/Tools/cowtools.ymlResources/Prototypes/Loadouts/loadout_groups.ymlResources/Prototypes/Reagents/toxins.ymlResources/Prototypes/Recipes/Reactions/fun.yml
Content.Shared/ADT/EntityEffects/Effects/EntitySpawning/SpawnRandomEntityEntityEffectSystem.cs
Show resolved
Hide resolved
Content.Shared/ADT/EntityEffects/Effects/EntitySpawning/SpawnRandomEntityEntityEffectSystem.cs
Show resolved
Hide resolved
Content.Shared/ADT/EntityEffects/Effects/EntitySpawning/SpawnRandomEntityEntityEffectSystem.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Content.Shared/ADT/Combat/ComboEffects.cs (1)
344-355:⚠️ Potential issue | 🟠 MajorНе логируйте успешную остановку захвата без проверки текущей пары
puller/pulled.Сейчас достаточно иметь
PullerComponent/PullableComponent, чтобы запись ушла какperformed, но код не проверяет, чтоpuller.Pullingдействительно указывает на эту цель. Если сущность тянет кого-то другого, audit trail зафиксирует ложное действие на неверной паре.🐛 Минимальная правка
- if (!entMan.TryGetComponent<PullerComponent>(user, out var puller) || !entMan.TryGetComponent<PullableComponent>(target, out var pulled)) + if (!entMan.TryGetComponent<PullerComponent>(user, out var puller) || + puller.Pulling != target || + !entMan.TryGetComponent<PullableComponent>(target, out var pulled)) return; @@ - if (!entMan.TryGetComponent<PullerComponent>(target, out var puller) || !entMan.TryGetComponent<PullableComponent>(user, out var pulled)) + if (!entMan.TryGetComponent<PullerComponent>(target, out var puller) || + puller.Pulling != user || + !entMan.TryGetComponent<PullableComponent>(user, out var pulled)) return;Also applies to: 364-375
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/ADT/Combat/ComboEffects.cs` around lines 344 - 355, The admin log is recorded whenever both PullerComponent and PullableComponent exist, even if they are not paired; only log (and/or perform the stop-pull sequence) when the components actually reference each other. Update the code around PullingSystem, PullerComponent, PullableComponent and the calls to TryLowerGrabStageOrStopPulling/TryStopPull so you first check that puller.Pulling == target (and/or pulled.Puller == user) or that TryStopPull returned true; only then run the loop and call TryStopPull and only then call adminLogger.Add with the performed message. Ensure the pairing check uses the PullerComponent.Pulling and PullableComponent.Puller fields to validate the current pair before logging.
♻️ Duplicate comments (2)
Content.Shared/ADT/Combat/ComboEffects.cs (2)
24-27:⚠️ Potential issue | 🟡 MinorПереставьте новые
usingпо алфавиту.В ADT-файлах это правило репозитория; здесь
Robust.Shared.GameObjectsдолжен идти передRobust.Shared.IoC.♻️ Минимальная правка
using Content.Shared.Administration.Logs; using Content.Shared.Database; -using Robust.Shared.IoC; using Robust.Shared.GameObjects; +using Robust.Shared.IoC;As per coding guidelines:
И ещё смотри за тем чтобы using был в алфавитном порядке в наших /ADT/ системах и компонентах.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/ADT/Combat/ComboEffects.cs` around lines 24 - 27, Reorder the using directives in ComboEffects.cs so they are alphabetized for ADT files: place Robust.Shared.GameObjects before Robust.Shared.IoC, resulting in the sequence Content.Shared.Administration.Logs, Content.Shared.Database, Robust.Shared.GameObjects, Robust.Shared.IoC; update the using block that currently lists Content.Shared.Administration.Logs, Content.Shared.Database, Robust.Shared.IoC, Robust.Shared.GameObjects to the corrected alphabetical order.
53-56:⚠️ Potential issue | 🟡 MinorВынесите новые тексты admin-логов в FTL.
Все добавленные здесь
adminLogger.Add(...)всё ещё содержат захардкоженный текст в.cs. По правилам репозитория эти строки нужно вынести в.ftlи получать черезLoc.GetString(...).As per coding guidelines:
Указывай на твёрдо вписанный текст в "Вот в таких строках" в переменных, он должен быть в ftl файлах, а в .cs использовать Loc.GetString("text-help-loc"); вот этот менеджер для локализации переводов.Also applies to: 74-76, 118-120, 143-145, 176-178, 211-213, 237-239, 270-272, 287-289, 310-312, 333-335, 353-355, 373-375, 426-428, 493-495, 512-514, 532-534, 554-556, 594-596
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/ADT/Combat/ComboEffects.cs` around lines 53 - 56, The adminLogger.Add calls in ComboEffects.cs (e.g., the call using entMan.ToPrettyString(user), entMan.ToPrettyString(target), damageResult.GetTotal() and the variable adminLogger) contain hardcoded messages and must be moved to an FTL entry and retrieved with Loc.GetString; replace each adminLogger.Add(...) message string with Loc.GetString("combo-effects-admin-log", ("actor", entMan.ToPrettyString(user)), ("subject", entMan.ToPrettyString(target)), ("damage", damageResult.GetTotal())) (or similar named key) and add matching entries in the .ftl file for all listed occurrences (lines around the shown call and the other referenced ranges) so the log uses localized templates with placeholder replacements.
🤖 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/ADT/Combat/ComboEffects.cs`:
- Around line 267-272: Check the boolean result of status.TryAddStatusEffect
when applying MutedComponent in ComboMuteEffect (and similarly for slowdown in
ComboSlowdownEffect); only call adminLogger.Add with the successful-action
message when TryAddStatusEffect returns true, or alternatively change the log
text to indicate an attempted application if the call returns false—locate the
TryAddStatusEffect<MutedComponent>(target, "Muted", ...) and corresponding
slowdown call and gate the adminLogger.Add / entMan.ToPrettyString(...) logging
behind the returned bool.
In `@Resources/Prototypes/Recipes/Reactions/fun.yml`:
- Around line 156-168: The ADT-Tweak block markers use the wrong format/casing;
replace the existing "ADT-Tweak start" and "ADT-Tweak end" markers around the
SpawnRandomEntity block with the canonical markers "ADT-Tweak-Start" (capital S)
and "ADT-Tweak-end" (lowercase e) so the existing SpawnRandomEntity entry and
its prototypes (e.g., ShardCrystalGreen, ShardCrystalPink, etc.) remain
unchanged but are correctly wrapped by the standardized ADT-Tweak markers.
---
Outside diff comments:
In `@Content.Shared/ADT/Combat/ComboEffects.cs`:
- Around line 344-355: The admin log is recorded whenever both PullerComponent
and PullableComponent exist, even if they are not paired; only log (and/or
perform the stop-pull sequence) when the components actually reference each
other. Update the code around PullingSystem, PullerComponent, PullableComponent
and the calls to TryLowerGrabStageOrStopPulling/TryStopPull so you first check
that puller.Pulling == target (and/or pulled.Puller == user) or that TryStopPull
returned true; only then run the loop and call TryStopPull and only then call
adminLogger.Add with the performed message. Ensure the pairing check uses the
PullerComponent.Pulling and PullableComponent.Puller fields to validate the
current pair before logging.
---
Duplicate comments:
In `@Content.Shared/ADT/Combat/ComboEffects.cs`:
- Around line 24-27: Reorder the using directives in ComboEffects.cs so they are
alphabetized for ADT files: place Robust.Shared.GameObjects before
Robust.Shared.IoC, resulting in the sequence Content.Shared.Administration.Logs,
Content.Shared.Database, Robust.Shared.GameObjects, Robust.Shared.IoC; update
the using block that currently lists Content.Shared.Administration.Logs,
Content.Shared.Database, Robust.Shared.IoC, Robust.Shared.GameObjects to the
corrected alphabetical order.
- Around line 53-56: The adminLogger.Add calls in ComboEffects.cs (e.g., the
call using entMan.ToPrettyString(user), entMan.ToPrettyString(target),
damageResult.GetTotal() and the variable adminLogger) contain hardcoded messages
and must be moved to an FTL entry and retrieved with Loc.GetString; replace each
adminLogger.Add(...) message string with
Loc.GetString("combo-effects-admin-log", ("actor", entMan.ToPrettyString(user)),
("subject", entMan.ToPrettyString(target)), ("damage", damageResult.GetTotal()))
(or similar named key) and add matching entries in the .ftl file for all listed
occurrences (lines around the shown call and the other referenced ranges) so the
log uses localized templates with placeholder replacements.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 07f28e31-dfc5-4159-b15a-28219ad1d09a
📒 Files selected for processing (5)
Content.Shared/ADT/Combat/ComboEffects.csContent.Shared/ADT/EntityEffects/Effects/EntitySpawning/SpawnRandomEntityEntityEffectSystem.csResources/Locale/en-US/ADT/EntityEffects/entity-effects.ftlResources/Locale/ru-RU/ADT/EntityEffects/entity-effects.ftlResources/Prototypes/Recipes/Reactions/fun.yml
✅ Files skipped from review due to trivial changes (2)
- Resources/Locale/ru-RU/ADT/EntityEffects/entity-effects.ftl
- Resources/Locale/en-US/ADT/EntityEffects/entity-effects.ftl
🚧 Files skipped from review as they are similar to previous changes (1)
- Content.Shared/ADT/EntityEffects/Effects/EntitySpawning/SpawnRandomEntityEntityEffectSystem.cs
Техническая информация
Чейнджлог
🆑 CrimeMoot