Conversation
github-actions
Bot
added
S: Untriaged
size/L
Changes: UI
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
Knowledge base: Disabled due to WalkthroughPR переводит голод в starving pain, добавляет хирургическое снятие этой боли, перерабатывает лечение fallback-сценариев, расширяет анализатор здоровья предупреждениями о голоде и жажде, обновляет research UI и прототипы, а также стабилизирует один тест NukeOps. ChangesМедицинские механики и UI
NukeOps breathing stabilization
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
Content.Shared/Nutrition/Components/HungerComponent.cs (1)
155-170: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winНовые поля попадают под
[Access(typeof(HungerSystem))].Класс объявлен с
[Access(typeof(HungerSystem))], поэтому прямое чтение этих полей изHungerPainSystemможет нарушить access-анализатор. См. подробный комментарий и проверку вHungerPainSystem.cs.🤖 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/Nutrition/Components/HungerComponent.cs` around lines 155 - 170, The new starvation pain fields in HungerComponent are being read from HungerPainSystem even though HungerComponent is marked with [Access(typeof(HungerSystem))], so update the access pattern to satisfy the analyzer. Use HungerSystem as the sole direct accessor for StarvingPainEnabled, StarvingPainGrowthRate, StarvingPainMax, StarvingPainDecayRate, and StarvingOrganTraumaThreshold, and have HungerPainSystem obtain the values through HungerSystem or another approved boundary instead of reading the component fields directly.
🧹 Nitpick comments (3)
Content.Shared/Medical/Healing/HealingSystem.cs (1)
383-414: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffДублирование логики разрешения цели/попапов.
Блок повторно вычисляет
totalBleeds,stuffToHealи три ветки fallback-попапов, которые по сути дублируют логикуTryResolveHealTarget/GetTotalBleeds. Это расходящаяся копия (здесьstuffToHealключуется поdamage.Key, а вTryResolveHealTarget— поdamage.Key.Id). Рассмотрите вынос построения попап-сообщения в общий хелпер, чтобы пороги и условия не разъезжались между двумя местами.🤖 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/Medical/Healing/HealingSystem.cs` around lines 383 - 414, This block duplicates the healing target resolution and fallback popup logic, and the duplicated `stuffToHeal` filter is inconsistent with `TryResolveHealTarget` because it keys by `damage.Key` instead of `damage.Key.Id`. Refactor the popup decision into a shared helper used by `HealingSystem` alongside `TryResolveHealTarget`/`GetTotalBleeds`, and make both paths derive `totalBleeds`, `stuffToHeal`, and the three fallback messages from the same logic so thresholds and conditions cannot drift.Content.Server/Backmen/Nutrition/HungerPainSystem.cs (1)
27-104: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffОбновление модификатора боли каждый кадр может быть дорогим.
ApplyStarvingPainвызываетTryChangePainModifierна каждом тике для каждой голодающей сущности, что каждый раз триггеритUpdateNerveSystemPain(пересчёт +Dirty). При большом числе мобов это заметная нагрузка. Рассмотрите аккумулятор/интервал обновления (например, раз в N секунд или только при измененииCurrentStarvingPainсверх порога).🤖 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/Backmen/Nutrition/HungerPainSystem.cs` around lines 27 - 104, The `HungerPainSystem.Update` path is recalculating and dirtying nerve-system pain every frame for each starving entity via `ApplyStarvingPain`, which is too expensive. Update the logic so pain modifier changes are applied only when `CurrentStarvingPain` changes meaningfully or on a fixed interval, and avoid calling `TryChangePainModifier`/`UpdateNerveSystemPain` every tick unless the value actually needs to change. Use the existing `HungerPainSystem`, `ApplyStarvingPain`, and `ClearStarvingPain` flow to add a small accumulator or threshold-based guard.Content.Shared/Nutrition/EntitySystems/HungerSystem.cs (1)
252-258: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winЗафиксируйте авторитетное значение голода до смены
ActualDecayRate.
GetHungerвычисляет значение какLastAuthoritativeHungerValue - dt * ActualDecayRate. ЗдесьActualDecayRateуже обновлён (строка 254) к моменту вызоваSetAuthoritativeHungerValue(..., GetHunger(component))(строка 258), поэтому накопленный заdtdecay пересчитывается по новой ставке задним числом — при заметномdtэто даёт скачок голода. Стоит сначала пересчитать авторитетное значение по старой ставке, затем менятьBaseDecayRate/ActualDecayRate.♻️ Предлагаемый порядок операций
component.BaseDecayRate = baseDecayRate; if (clearStarvationDamage) component.StarvationDamage = null; + SetAuthoritativeHungerValue((uid, component), GetHunger(component)); + if (component.HungerThresholdDecayModifiers.TryGetValue(component.CurrentThreshold, out var modifier)) { component.ActualDecayRate = component.BaseDecayRate * modifier; DirtyField(uid, component, nameof(HungerComponent.ActualDecayRate)); } - - SetAuthoritativeHungerValue((uid, component), GetHunger(component));🤖 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/Nutrition/EntitySystems/HungerSystem.cs` around lines 252 - 258, `HungerSystem` is recalculating the authoritative hunger value after `ActualDecayRate` has already been updated, which retroactively applies the new decay rate to the whole elapsed `dt`. In the `SetAuthoritativeHungerValue` flow around `GetHunger(component)`, first compute and store the authoritative hunger using the current rate, then update `BaseDecayRate`/`ActualDecayRate` and dirty the field. Keep the fix localized to the `HungerSystem` logic so `GetHunger` and `SetAuthoritativeHungerValue` continue to use the correct pre-change rate for the elapsed interval.
🤖 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.IntegrationTests/Tests/Backmen/Medical/HealingFallbackTest.cs`:
- Around line 83-105: Add an explicit assertion in HealingFallbackTest around
the chest setup to verify the chest is actually in heavy bleeding state before
healing. Use the existing bodySys/woundSys setup after damage is applied, and
assert via the chest target obtained from TryGetWoundableTargetByType that the
piercing damage is classified as heavy bleeding so the test exercises the
intended branch instead of only validating Cold removal.
In `@Content.IntegrationTests/Tests/Backmen/Surgery/StarvingPainSurgeryTest.cs`:
- Around line 71-72: The StarvingPainSurgeryTest check is too weak because it
only asserts HungerComponent.CurrentThreshold, so it can miss changes to the
actual hunger value. In the test around GetComponent<HungerComponent>(patient),
capture the patient’s hunger value before the surgery step and compare it after
the operation to ensure the hunger itself remains unchanged while the pain is
removed.
In `@Content.Server/Tools/Innate/InnateToolSystem.cs`:
- Around line 71-94: In EnsureHandsForTools, the hand count is based on
InnateToolComponent.Tools.Count, which does not match the actual number of
spawned tools after EntitySpawnCollection.GetSpawns expands amount/prob entries.
Update the logic to count the real expanded spawn list used for spawning, and
use that value when adding hands so TrySpawnOneInHand never runs out of
available hands; keep the fix localized to EnsureHandsForTools and the spawn
flow that feeds it.
In `@Content.Shared/Medical/Healing/HealingSystem.cs`:
- Around line 420-477: TryResolveHealTarget in HealingSystem uses _random.Pick
for the fallback candidate selection, which is unsafe in the shared predicted
path and can desync later random calls; replace it with a deterministic
selection strategy or move the fallback to a server-only path. While updating
this logic, avoid calling TryCandidate twice for the final chosen woundable and
either remove usedFallback if it is truly unused or thread it into the caller if
it is meant to affect behavior.
---
Duplicate comments:
In `@Content.Shared/Nutrition/Components/HungerComponent.cs`:
- Around line 155-170: The new starvation pain fields in HungerComponent are
being read from HungerPainSystem even though HungerComponent is marked with
[Access(typeof(HungerSystem))], so update the access pattern to satisfy the
analyzer. Use HungerSystem as the sole direct accessor for StarvingPainEnabled,
StarvingPainGrowthRate, StarvingPainMax, StarvingPainDecayRate, and
StarvingOrganTraumaThreshold, and have HungerPainSystem obtain the values
through HungerSystem or another approved boundary instead of reading the
component fields directly.
---
Nitpick comments:
In `@Content.Server/Backmen/Nutrition/HungerPainSystem.cs`:
- Around line 27-104: The `HungerPainSystem.Update` path is recalculating and
dirtying nerve-system pain every frame for each starving entity via
`ApplyStarvingPain`, which is too expensive. Update the logic so pain modifier
changes are applied only when `CurrentStarvingPain` changes meaningfully or on a
fixed interval, and avoid calling
`TryChangePainModifier`/`UpdateNerveSystemPain` every tick unless the value
actually needs to change. Use the existing `HungerPainSystem`,
`ApplyStarvingPain`, and `ClearStarvingPain` flow to add a small accumulator or
threshold-based guard.
In `@Content.Shared/Medical/Healing/HealingSystem.cs`:
- Around line 383-414: This block duplicates the healing target resolution and
fallback popup logic, and the duplicated `stuffToHeal` filter is inconsistent
with `TryResolveHealTarget` because it keys by `damage.Key` instead of
`damage.Key.Id`. Refactor the popup decision into a shared helper used by
`HealingSystem` alongside `TryResolveHealTarget`/`GetTotalBleeds`, and make both
paths derive `totalBleeds`, `stuffToHeal`, and the three fallback messages from
the same logic so thresholds and conditions cannot drift.
In `@Content.Shared/Nutrition/EntitySystems/HungerSystem.cs`:
- Around line 252-258: `HungerSystem` is recalculating the authoritative hunger
value after `ActualDecayRate` has already been updated, which retroactively
applies the new decay rate to the whole elapsed `dt`. In the
`SetAuthoritativeHungerValue` flow around `GetHunger(component)`, first compute
and store the authoritative hunger using the current rate, then update
`BaseDecayRate`/`ActualDecayRate` and dirty the field. Keep the fix localized to
the `HungerSystem` logic so `GetHunger` and `SetAuthoritativeHungerValue`
continue to use the correct pre-change rate for the elapsed interval.
🪄 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
Run ID: df18a891-7540-4be3-80eb-7404b82ed62a
📒 Files selected for processing (40)
Content.Client/Backmen/Medical/UI/PainCausesDisplay.xaml.csContent.Client/HealthAnalyzer/UI/HealthAnalyzerWindow.xaml.csContent.IntegrationTests/Tests/Backmen/Medical/HealingFallbackTest.csContent.IntegrationTests/Tests/Backmen/Medical/HealthAnalyzerSatiationTest.csContent.IntegrationTests/Tests/Backmen/Nutrition/HungerPainTest.csContent.IntegrationTests/Tests/Backmen/Surgery/SiliconSurgeryToolsTest.csContent.IntegrationTests/Tests/Backmen/Surgery/StarvingPainSurgeryTest.csContent.Server/Backmen/Nutrition/HungerPainSystem.csContent.Server/Backmen/Shipwrecked/ShipwreckedRuleSystem.csContent.Server/Backmen/Surgery/Pain/Systems/ServerPainSystem.csContent.Server/Backmen/Surgery/SurgerySystem.csContent.Server/Medical/HealthAnalyzerSystem.csContent.Server/Tools/Innate/InnateToolSystem.csContent.Shared/Backmen/Nutrition/HungerPainTrackerComponent.csContent.Shared/Backmen/Surgery/Conditions/SurgeryStarvingPainConditionComponent.csContent.Shared/Backmen/Surgery/Effects/Step/SurgeryRelieveStarvingPainEffectComponent.csContent.Shared/Backmen/Surgery/Pain/PainSerializable.csContent.Shared/Backmen/Surgery/SharedSurgerySystem.Steps.csContent.Shared/Backmen/Surgery/SharedSurgerySystem.csContent.Shared/Medical/Healing/HealingSystem.csContent.Shared/MedicalScanner/HealthAnalyzerScannedUserMessage.csContent.Shared/Nutrition/Components/HungerComponent.csContent.Shared/Nutrition/EntitySystems/HungerSystem.csContent.Shared/Nutrition/EntitySystems/ThirstSystem.csResources/Locale/en-US/medical/components/healing-component.ftlResources/Locale/en-US/medical/components/health-analyzer-component.ftlResources/Locale/en-US/ss14-ru/prototypes/entities/objects/specific/medical/healing.ftlResources/Locale/ru-RU/medical/components/healing-component.ftlResources/Locale/ru-RU/medical/components/health-analyzer-component.ftlResources/Locale/ru-RU/ss14-ru/prototypes/_backmen/entities/surgery/surgeries.ftlResources/Locale/ru-RU/ss14-ru/prototypes/_backmen/entities/surgery/surgery_steps.ftlResources/Locale/ru-RU/ss14-ru/prototypes/entities/objects/specific/medical/healing.ftlResources/Prototypes/Body/species_base.ymlResources/Prototypes/Entities/Objects/Specific/Robotics/borg_modules.ymlResources/Prototypes/_Backmen/Body/species_surgery_base.ymlResources/Prototypes/_Backmen/Entities/Mobs/Player/Silicons/BPLA.ymlResources/Prototypes/_Backmen/Entities/Mobs/Player/Silicons/BPLA_MED.ymlResources/Prototypes/_Backmen/Entities/Surgery/surgeries.ymlResources/Prototypes/_Backmen/Entities/Surgery/surgery_steps.ymlResources/Prototypes/borg_types.yml
Comment on lines
+83
to
+105
| var chestDamage = new DamageSpecifier | ||
| { | ||
| DamageDict = | ||
| { | ||
| ["Cold"] = FixedPoint2.New(4), | ||
| ["Piercing"] = FixedPoint2.New(30), | ||
| }, | ||
| }; | ||
| damageSys.ChangeDamage(patient, chestDamage, targetPart: TargetBodyPart.Chest); | ||
|
|
||
| Assert.That(bodySys.TryGetWoundableTargetByType(patient, BodyPartType.Chest, null, out var chest), Is.True); | ||
| Assert.That(woundSys.HasDamageOfType(chest, "Cold"), Is.True); | ||
|
|
||
| var targeting = Server.EntMan.EnsureComponent<TargetingComponent>(healer); | ||
| targeting.Target = TargetBodyPart.Chest; | ||
| }); | ||
|
|
||
| await RaiseHealingDoAfter(patient, healer, ointment); | ||
|
|
||
| await Server.WaitAssertion(() => | ||
| { | ||
| Assert.That(bodySys.TryGetWoundableTargetByType(patient, BodyPartType.Chest, null, out var chest), Is.True); | ||
| Assert.That(woundSys.HasDamageOfType(chest, "Cold"), Is.False); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Зафиксируйте в тесте, что на груди действительно есть сильное кровотечение.
Иначе кейс про heavy bleeding может зелёным пройти мимо нужной ветки: если 30 Piercing перестанет считаться тяжёлым кровотечением, тест всё равно останется зелёным и будет проверять только обычное снятие Cold.
🤖 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.IntegrationTests/Tests/Backmen/Medical/HealingFallbackTest.cs` around
lines 83 - 105, Add an explicit assertion in HealingFallbackTest around the
chest setup to verify the chest is actually in heavy bleeding state before
healing. Use the existing bodySys/woundSys setup after damage is applied, and
assert via the chest target obtained from TryGetWoundableTargetByType that the
piercing damage is classified as heavy bleeding so the test exercises the
intended branch instead of only validating Cold removal.
Comment on lines
+71
to
+72
| var hunger = Server.EntMan.GetComponent<HungerComponent>(patient); | ||
| Assert.That(hunger.CurrentThreshold, Is.LessThanOrEqualTo(HungerThreshold.Starving)); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Проверяйте неизменность самого значения голода, а не только порога.
Сейчас тест пройдёт даже если шаг операции изменит hunger внутри диапазона Starving. Для контракта “снимает боль, не сбрасывает голод” лучше сохранить значение до операции и сравнить его после.
🤖 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.IntegrationTests/Tests/Backmen/Surgery/StarvingPainSurgeryTest.cs`
around lines 71 - 72, The StarvingPainSurgeryTest check is too weak because it
only asserts HungerComponent.CurrentThreshold, so it can miss changes to the
actual hunger value. In the test around GetComponent<HungerComponent>(patient),
capture the patient’s hunger value before the surgery step and compare it after
the operation to ensure the hunger itself remains unchanged while the pain is
removed.
Comment on lines
+71
to
+94
| // start-backmen: innate-tool-auto-hands | ||
| private void EnsureHandsForTools(EntityUid uid, InnateToolComponent component) | ||
| { | ||
| if (!TryComp<HandsComponent>(uid, out var hands)) | ||
| return; | ||
|
|
||
| var requiredHands = component.Tools.Count; | ||
| var handIndex = 3; | ||
| var handCount = 0; | ||
| foreach (var _ in _sharedHandsSystem.EnumerateHands((uid, hands))) | ||
| handCount++; | ||
|
|
||
| while (handCount < requiredHands) | ||
| { | ||
| string handId; | ||
| do | ||
| { | ||
| handId = $"innate-{handIndex++}"; | ||
| } while (_sharedHandsSystem.TryGetHand((uid, hands), handId, out _)); | ||
|
|
||
| _sharedHandsSystem.AddHand((uid, hands), handId, HandLocation.Middle); | ||
| handCount++; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Проверяем определение EntitySpawnEntry/GetSpawns и наличие amount/probability,
# а также порядок инициализации ToSpawn.
rg -nP --type=cs -C3 'class\s+EntitySpawnEntry'
rg -nP --type=cs -C3 'GetSpawns\s*\('
rg -nP --type=cs -C3 '\bToSpawn\b' Content.Server/Tools/Innate/InnateToolSystem.csRepository: Rxup/space-station-14
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Files\n'
git ls-files 'Content.Server/Tools/Innate/*' 'Content.Shared/*Spawn*' 'Content.*/*Spawn*' | sed -n '1,200p'
printf '\n## Search for relevant symbols\n'
rg -n --hidden --glob '!**/bin/**' --glob '!**/obj/**' '\b(EntitySpawnCollection|EntitySpawnEntry|GetSpawns|ToSpawn|TryFillHands|TrySpawnOneInHand|EnsureHandsForTools)\b' .
printf '\n## Outline candidate files if found\n'
for f in \
Content.Server/Tools/Innate/InnateToolSystem.cs \
Content.Shared/ContentEntitySpawn.cs \
Content.Shared/EntitySpawnCollection.cs \
Content.Shared/EntitySpawnEntry.cs \
; do
[ -f "$f" ] && { echo "### $f"; wc -l "$f"; ast-grep outline "$f" --view expanded || true; }
doneRepository: Rxup/space-station-14
Length of output: 28095
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Innate tool system\n'
wc -l Content.Server/Tools/Innate/InnateToolSystem.cs
sed -n '1,220p' Content.Server/Tools/Innate/InnateToolSystem.cs
printf '\n## Spawn-related symbols in repo\n'
rg -n --hidden --glob '!**/bin/**' --glob '!**/obj/**' '\bToSpawn\b|\bGetSpawns\b|\bEntitySpawnEntry\b|\bEntitySpawnCollection\b' Content.Shared Content.Server | sed -n '1,220p'Repository: Rxup/space-station-14
Length of output: 23281
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## EntitySpawnEntry / EntitySpawnCollection\n'
sed -n '1,340p' Content.Shared/Storage/EntitySpawnEntry.cs
printf '\n## Innate tool component\n'
sed -n '1,120p' Content.Server/Tools/Innate/InnateToolComponent.cs
printf '\n## Nearby body events / spawn ordering cues\n'
rg -n --hidden --glob '!**/bin/**' --glob '!**/obj/**' '\bInitialBodySpawnedEvent\b|\bMapInitEvent\b|\bInitialBodySystem\b' Content.Server Content.Shared | sed -n '1,220p'Repository: Rxup/space-station-14
Length of output: 42250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## InitialBodySystem and event definitions\n'
rg -n --hidden --glob '!**/bin/**' --glob '!**/obj/**' '\bInitialBodySpawnedEvent\b|\bclass\s+InitialBodySystem\b|\bstruct\s+InitialBodySpawnedEvent\b|\brecord\s+InitialBodySpawnedEvent\b' Content.Server Content.Shared | sed -n '1,220p'
printf '\n## InnateToolSystem subscriptions / shutdown paths\n'
sed -n '1,180p' Content.Server/Tools/Innate/InnateToolSystem.cs
printf '\n## Search for cleanup of added hands in similar systems\n'
rg -n --hidden --glob '!**/bin/**' --glob '!**/obj/**' 'RemoveHand|DeleteHand|TryDrop\(|HandsComponent' Content.Server/Tools Content.Server/Body Content.Server | sed -n '1,220p'Repository: Rxup/space-station-14
Length of output: 16523
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## InitialBodySystem\n'
sed -n '1,120p' Content.Shared/Body/InitialBodySystem.cs
printf '\n## InitialBodySpawnedEvent definition\n'
sed -n '1,80p' Content.Shared/Body/Events/InitialBodyEvents.cs
printf '\n## Any explicit ordering in InnateToolSystem registration\n'
sed -n '1,80p' Content.Server/Tools/Innate/InnateToolSystem.csRepository: Rxup/space-station-14
Length of output: 5237
Считать руки по фактическому числу спавнов, а не по Tools.Count.
EntitySpawnCollection.GetSpawns(...) разворачивает amount/prob, поэтому при Tools.Count часть инструментов может остаться без руки и быть удалена в TrySpawnOneInHand. ToSpawn.Count здесь тоже не универсален: в OnInitialBodySpawned список ещё пуст, так что нужен подсчёт из уже расширенного списка.
🤖 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/Tools/Innate/InnateToolSystem.cs` around lines 71 - 94, In
EnsureHandsForTools, the hand count is based on InnateToolComponent.Tools.Count,
which does not match the actual number of spawned tools after
EntitySpawnCollection.GetSpawns expands amount/prob entries. Update the logic to
count the real expanded spawn list used for spawning, and use that value when
adding hands so TrySpawnOneInHand never runs out of available hands; keep the
fix localized to EnsureHandsForTools and the spawn flow that feeds it.
Comment on lines
+420
to
+477
| private bool TryResolveHealTarget( | ||
| EntityUid body, | ||
| EntityUid healer, | ||
| HealingComponent healing, | ||
| out EntityUid woundable, | ||
| out Dictionary<string, FixedPoint2> stuffToHeal, | ||
| out bool usedFallback) | ||
| { | ||
| woundable = EntityUid.Invalid; | ||
| stuffToHeal = new Dictionary<string, FixedPoint2>(); | ||
| usedFallback = false; | ||
|
|
||
| if (!TryComp<TargetingComponent>(healer, out var targeting)) | ||
| return false; | ||
|
|
||
| bool TryCandidate(EntityUid candidate, out Dictionary<string, FixedPoint2> healDict) | ||
| { | ||
| healDict = new Dictionary<string, FixedPoint2>(); | ||
| if (!TryComp<WoundableComponent>(candidate, out var woundableComp)) | ||
| return false; | ||
|
|
||
| healDict = healing.Damage.DamageDict | ||
| .Where(damage => _wounds.HasDamageOfType(candidate, damage.Key)) | ||
| .ToDictionary(damage => damage.Key.Id, damage => damage.Value); | ||
|
|
||
| var bleeds = GetTotalBleeds(candidate, woundableComp); | ||
| if (bleeds > healing.UnableToHealBleedsThreshold | ||
| && (healing.BloodlossModifier != 0 || healDict.Count == 0)) | ||
| return false; | ||
|
|
||
| return healDict.Count > 0 | ||
| || healing.BloodlossModifier != 0 | ||
| || healing.ModifyBloodLevel > 0; | ||
| } | ||
|
|
||
| var (partType, symmetry) = _bodySystem.ConvertTargetBodyPart(targeting.Target); | ||
| if (_bodySystem.TryGetWoundableTargetByType(body, partType, symmetry, out var selected) | ||
| && TryCandidate(selected, out stuffToHeal)) | ||
| { | ||
| woundable = selected; | ||
| return true; | ||
| } | ||
|
|
||
| var candidates = new List<EntityUid>(); | ||
| foreach (var candidate in _bodySystem.GetWoundableTargets(body)) | ||
| { | ||
| if (TryCandidate(candidate, out _)) | ||
| candidates.Add(candidate); | ||
| } | ||
|
|
||
| if (candidates.Count == 0) | ||
| return false; | ||
|
|
||
| usedFallback = true; | ||
| woundable = _random.Pick(candidates); | ||
| TryCandidate(woundable, out stuffToHeal); | ||
| return true; | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Подтвердить, что HealingSystem — общий/предсказываемый
rg -nP 'class\s+HealingSystem' --type=cs -C2
rg -nP 'HealingDoAfterEvent|UseInHandEvent|AfterInteractEvent' Content.Shared/Medical/Healing/HealingSystem.cs -nRepository: Rxup/space-station-14
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the system and inspect the relevant implementation.
git ls-files 'Content.Shared/Medical/Healing/*' 'Content.Shared/**/HealingSystem.cs' | sed 's#^`#FILE` #'
echo '---'
wc -l Content.Shared/Medical/Healing/HealingSystem.cs
echo '---'
sed -n '1,220p' Content.Shared/Medical/Healing/HealingSystem.cs
echo '---'
sed -n '220,520p' Content.Shared/Medical/Healing/HealingSystem.cs
echo '---'
# Check how random is provided and whether this system participates in prediction/event flows.
rg -n "IRobustRandom|_random|UseInHandEvent|AfterInteractEvent|DoAfterEvent|Predict|predicted|Prediction" Content.Shared/Medical/Healing Content.Shared -g '*.cs'Repository: Rxup/space-station-14
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- TryHeal / end of HealingSystem.cs ---'
sed -n '520,597p' Content.Shared/Medical/Healing/HealingSystem.cs
echo '--- HealingDoAfterEvent definition ---'
rg -n "class\s+HealingDoAfterEvent|record\s+HealingDoAfterEvent|HealingDoAfterEvent" Content.Shared Content.Server -g '*.cs' -C 2
echo '--- prediction/timing guards in HealingSystem.cs ---'
rg -n "_timing|IsFirstTimePredicted|Predicted|Prediction|DoAfterArgs" Content.Shared/Medical/Healing/HealingSystem.cs -C 2Repository: Rxup/space-station-14
Length of output: 6672
Не используйте _random.Pick в TryResolveHealTarget для предсказываемого shared-пути
HealingSystem обрабатывает UseInHandEvent/AfterInteractEvent и HealingDoAfterEvent в Content.Shared, поэтому этот fallback выполняется и на клиенте. _random.Pick(candidates) сдвигает RNG и может разъехать последующие предсказываемые случайные вызовы. Для fallback нужен детерминированный выбор или серверный путь; заодно TryCandidate для выбранного кандидата вызывается дважды, а usedFallback сейчас не используется.
🤖 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/Medical/Healing/HealingSystem.cs` around lines 420 - 477,
TryResolveHealTarget in HealingSystem uses _random.Pick for the fallback
candidate selection, which is unsafe in the shared predicted path and can desync
later random calls; replace it with a deterministic selection strategy or move
the fallback to a server-only path. While updating this logic, avoid calling
TryCandidate twice for the final chosen woundable and either remove usedFallback
if it is truly unused or thread it into the caller if it is meant to affect
behavior.
Contributor
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 (5)
Content.IntegrationTests/Tests/Backmen/Nutrition/HungerPainTest.cs (1)
116-119: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winНе обращайтесь к причине боли через индексатор словаря.
Здесь
?["Starving"]всё ещё броситKeyNotFoundException, если запись ещё не появилась. Тогда тест упадёт исключением вместо нормальной проверки черезAssert. Лучше читать значение черезTryGetValue, как вы уже делаете ниже в этом же тесте.Предлагаемое исправление
await Server.WaitAssertion(() => { - painBeforeEat = consciousnessSys.GetPainCauses(human)?["Starving"] ?? 0; + var causes = consciousnessSys.GetPainCauses(human); + painBeforeEat = causes != null && causes.TryGetValue("Starving", out var starving) ? starving : 0; Assert.That(painBeforeEat, Is.GreaterThan(0)); });🤖 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.IntegrationTests/Tests/Backmen/Nutrition/HungerPainTest.cs` around lines 116 - 119, The HungerPainTest assertion still reads the “Starving” pain cause with a dictionary indexer inside Server.WaitAssertion, so it can throw before the Assert runs; update the pain lookup in this block to use TryGetValue on the result of consciousnessSys.GetPainCauses(human) instead of ?["Starving"], matching the safer pattern already used later in the same test, and keep the assertion on the retrieved value.Content.Server/Backmen/Nutrition/HungerPainSystem.cs (4)
97-100: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winНе сбрасывайте флаг травмы без удаления модификатора.
Если
CurrentStarvingPainуже обнулили извне, Line 99 теряет признак применённой"StarvingOrganTrauma", не вызываяTryRemoveOrganDamageModifier; модификатор может остаться навсегда.Предлагаемое исправление
else { - tracker.StarvingOrganTraumaApplied = false; + ClearStarvingPain(uid, tracker, nerveSys.Value, chest); }🤖 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/Backmen/Nutrition/HungerPainSystem.cs` around lines 97 - 100, In HungerPainSystem, the else branch in the starvation pain update clears tracker.StarvingOrganTraumaApplied without removing the applied organ damage modifier, which can leave StarvingOrganTrauma stuck on the tracker. Update the logic around the CurrentStarvingPain/StarvingOrganTraumaApplied handling to call TryRemoveOrganDamageModifier before resetting the flag, and keep the flag/state transitions in sync wherever StarvingOrganTrauma is applied or cleared.
70-78: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winВыставляйте
StarvingOrganTraumaAppliedтолько после успешного добавления травмы.Сейчас флаг становится
true, даже еслиTryAddOrganDamageModifierвернулfalse; это блокирует повторное применение и рассинхронизирует трекер с реальным trauma-модификатором.Предлагаемое исправление
- tracker.StarvingOrganTraumaApplied = true; - _trauma.TryAddOrganDamageModifier( + if (_trauma.TryAddOrganDamageModifier( chest, FixedPoint2.New(5), uid, - "StarvingOrganTrauma"); + "StarvingOrganTrauma")) + { + tracker.StarvingOrganTraumaApplied = true; + }🤖 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/Backmen/Nutrition/HungerPainSystem.cs` around lines 70 - 78, The starvation trauma flag in HungerPainSystem is being set before confirming that TryAddOrganDamageModifier succeeded, which can leave StarvingOrganTraumaApplied out of sync with the actual modifier state. Update the HungerPainSystem logic around the chest/StarvingOrganTrauma path so tracker.StarvingOrganTraumaApplied is assigned only when _trauma.TryAddOrganDamageModifier returns success, and leave it false on failure so the attempt can be retried.
145-157: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winНе блокируйте локальную очистку отсутствием nerve/chest.
Когда
ClearStarvingPainвызывается из ветки отсутствующей нервной системы, Line 148 возвращает до сбросаCurrentStarvingPain; аналогично Line 156–157 блокирует cleanup при отсутствующей груди. Разделите удаление внешних модификаторов и сброс локального трекера.🤖 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/Backmen/Nutrition/HungerPainSystem.cs` around lines 145 - 157, In ClearStarvingPain, separate the local cleanup of CurrentStarvingPain from the optional nerve/chest lookup paths: if nerveSys or chest are missing, do not return before clearing the hunger pain state. Keep the external modifier removal conditional on resolving nerveUid/chestUid, but always reset the local tracker first, using the existing ClearStarvingPain logic and the nerve/chest lookup branches as the fix points.
107-113: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winСинхронизируйте реальный pain-модификатор при
ResetStarvingPain.Метод меняет только трекер; активный
PainType.StarvingвPainSystemостаётся со старым значением до следующегоUpdate, а при сбросе в0может не удалиться корректно.Предлагаемое направление исправления
public void ResetStarvingPain(EntityUid body, float remainingPain = 0) { if (!TryComp<HungerPainTrackerComponent>(body, out var tracker)) return; + if (remainingPain <= 0) + { + ClearStarvingPain(body, tracker); + return; + } + tracker.CurrentStarvingPain = remainingPain; + + if (_consciousness.TryGetNerveSystem(body, out var nerveSys) && + _body.TryGetWoundableTargetByType(body, BodyPartType.Chest, null, out var chest)) + { + ApplyStarvingPain(nerveSys.Value, chest, tracker); + } + Dirty(body, tracker); }🤖 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/Backmen/Nutrition/HungerPainSystem.cs` around lines 107 - 113, `ResetStarvingPain` only updates `HungerPainTrackerComponent`, so the live `PainSystem` modifier for `PainType.Starving` can stay stale until the next update or fail to clear when resetting to zero. After setting `tracker.CurrentStarvingPain`, also synchronize the corresponding starving pain entry in `PainSystem` for the same `EntityUid` by updating/removing the active modifier through the existing pain-management path used by `PainSystem` so the runtime value matches the tracker immediately.
🧹 Nitpick comments (1)
Content.Server/Backmen/Nutrition/HungerPainSystem.cs (1)
103-103: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winВызывайте
Dirtyтолько при реальном изменении трекера.Сейчас стабильные сущности помечаются dirty каждый тик, а после
ClearStarvingPainвозможен повторныйDirtyв том же кадре. Лучше завести локальныйdirty-флаг и выставлять его только при изменении полей трекера.Also applies to: 169-170
🤖 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/Backmen/Nutrition/HungerPainSystem.cs` at line 103, The HungerPainSystem is marking entities dirty unconditionally, which can trigger redundant Dirty calls every tick and again after ClearStarvingPain in the same frame. Update the logic around the tracker mutation in HungerPainSystem so a local dirty flag is set only when tracker fields actually change, and call Dirty(uid, tracker) only if that flag is true. Use the existing HungerPainSystem methods and tracker updates to keep the dirtying behavior tied to real state changes.
🤖 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/Backmen/Research/BkmResearchRequirements.cs`:
- Around line 73-84: `HasUnmetRequirements` is treating a missing
`TechnologyDatabaseComponent` as if all requirements were satisfied, which
incorrectly makes `FancyTechnologyInfoPanel.InitializePrerequisites()` show the
no-requirements state for unknown tiers/discipline gates. Update
`BkmResearchRequirements.HasUnmetRequirements` so that a null `component` is
handled as “requirements cannot be verified” rather than returning false; keep
the prerequisite check using `SharedResearchSystem.IsTechnologyUnlocked`, and
make the method report unmet requirements whenever the database component needed
to evaluate tier/discipline gating is absent.
In `@Content.Shared/Backmen/Research/BkmResearchTreeLayout.cs`:
- Around line 70-100: The mixed auto-layout/manual Position path in
BuildDisciplineCells does not reserve space for overridden coordinates, so the
computed DisciplineCell Width/Height can be too small when
FancyResearchConsoleMenu later prefers proto.Position. Update
BuildDisciplineCells to include any manual Position overrides in the bounds
calculation, or explicitly reject mixed auto/manual placement, and make sure the
sizing logic in DisciplineCell matches the placement logic used by
FancyResearchConsoleMenu.
---
Outside diff comments:
In `@Content.IntegrationTests/Tests/Backmen/Nutrition/HungerPainTest.cs`:
- Around line 116-119: The HungerPainTest assertion still reads the “Starving”
pain cause with a dictionary indexer inside Server.WaitAssertion, so it can
throw before the Assert runs; update the pain lookup in this block to use
TryGetValue on the result of consciousnessSys.GetPainCauses(human) instead of
?["Starving"], matching the safer pattern already used later in the same test,
and keep the assertion on the retrieved value.
In `@Content.Server/Backmen/Nutrition/HungerPainSystem.cs`:
- Around line 97-100: In HungerPainSystem, the else branch in the starvation
pain update clears tracker.StarvingOrganTraumaApplied without removing the
applied organ damage modifier, which can leave StarvingOrganTrauma stuck on the
tracker. Update the logic around the
CurrentStarvingPain/StarvingOrganTraumaApplied handling to call
TryRemoveOrganDamageModifier before resetting the flag, and keep the flag/state
transitions in sync wherever StarvingOrganTrauma is applied or cleared.
- Around line 70-78: The starvation trauma flag in HungerPainSystem is being set
before confirming that TryAddOrganDamageModifier succeeded, which can leave
StarvingOrganTraumaApplied out of sync with the actual modifier state. Update
the HungerPainSystem logic around the chest/StarvingOrganTrauma path so
tracker.StarvingOrganTraumaApplied is assigned only when
_trauma.TryAddOrganDamageModifier returns success, and leave it false on failure
so the attempt can be retried.
- Around line 145-157: In ClearStarvingPain, separate the local cleanup of
CurrentStarvingPain from the optional nerve/chest lookup paths: if nerveSys or
chest are missing, do not return before clearing the hunger pain state. Keep the
external modifier removal conditional on resolving nerveUid/chestUid, but always
reset the local tracker first, using the existing ClearStarvingPain logic and
the nerve/chest lookup branches as the fix points.
- Around line 107-113: `ResetStarvingPain` only updates
`HungerPainTrackerComponent`, so the live `PainSystem` modifier for
`PainType.Starving` can stay stale until the next update or fail to clear when
resetting to zero. After setting `tracker.CurrentStarvingPain`, also synchronize
the corresponding starving pain entry in `PainSystem` for the same `EntityUid`
by updating/removing the active modifier through the existing pain-management
path used by `PainSystem` so the runtime value matches the tracker immediately.
---
Nitpick comments:
In `@Content.Server/Backmen/Nutrition/HungerPainSystem.cs`:
- Line 103: The HungerPainSystem is marking entities dirty unconditionally,
which can trigger redundant Dirty calls every tick and again after
ClearStarvingPain in the same frame. Update the logic around the tracker
mutation in HungerPainSystem so a local dirty flag is set only when tracker
fields actually change, and call Dirty(uid, tracker) only if that flag is true.
Use the existing HungerPainSystem methods and tracker updates to keep the
dirtying behavior tied to real state changes.
🪄 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
Run ID: 332b5cd9-7b7a-4f82-8eb2-d0bf83c06d73
📒 Files selected for processing (31)
Content.Client/Backmen/Research/UI/FancyResearchConsoleMenu.xaml.csContent.Client/Backmen/Research/UI/FancyTechnologyInfoPanel.xaml.csContent.IntegrationTests/Tests/Backmen/Nutrition/HungerPainTest.csContent.IntegrationTests/Tests/Backmen/TechnologyPrototypePositionTest.csContent.Server/Backmen/Nutrition/HungerPainSystem.csContent.Shared/Backmen/Research/BkmResearchRequirements.csContent.Shared/Backmen/Research/BkmResearchTreeLayout.csContent.Shared/Backmen/Research/SharedResearchSystemExtensions.csContent.Shared/Research/Prototypes/TechnologyPrototype.csResources/Locale/en-US/_Goobstation/research/ui.ftlResources/Locale/ru-RU/_Goobstation/research/ui.ftlResources/Prototypes/Research/arsenal.ymlResources/Prototypes/Research/civilianservices.ymlResources/Prototypes/_Backmen/Research/arsenal.ymlResources/Prototypes/_Backmen/Research/cargo.ymlResources/Prototypes/_Backmen/Research/engineering.ymlResources/Prototypes/_Backmen/Research/machines_arsenal.ymlResources/Prototypes/_Backmen/Research/machines_engineering.ymlResources/Prototypes/_Backmen/Research/machines_medical.ymlResources/Prototypes/_Backmen/Research/machines_robotics.ymlResources/Prototypes/_Backmen/Research/machines_scientific.ymlResources/Prototypes/_Backmen/Research/machines_service.ymlResources/Prototypes/_Backmen/Research/machines_shaft.ymlResources/Prototypes/_Backmen/Research/machines_shuttle.ymlResources/Prototypes/_Backmen/Research/medical.ymlResources/Prototypes/_Backmen/Research/robotics.ymlResources/Prototypes/_Backmen/Research/scientific.ymlResources/Prototypes/_Backmen/Research/security.ymlResources/Prototypes/_Backmen/Research/service.ymlResources/Prototypes/_Backmen/Research/shaft.ymlResources/Prototypes/_Backmen/Research/shuttle.yml
💤 Files with no reviewable changes (21)
- Resources/Prototypes/_Backmen/Research/shuttle.yml
- Content.Shared/Backmen/Research/SharedResearchSystemExtensions.cs
- Resources/Prototypes/_Backmen/Research/machines_robotics.yml
- Resources/Prototypes/_Backmen/Research/machines_arsenal.yml
- Resources/Prototypes/_Backmen/Research/machines_shaft.yml
- Resources/Prototypes/_Backmen/Research/cargo.yml
- Resources/Prototypes/_Backmen/Research/security.yml
- Resources/Prototypes/_Backmen/Research/machines_engineering.yml
- Resources/Prototypes/_Backmen/Research/machines_medical.yml
- Resources/Prototypes/_Backmen/Research/machines_shuttle.yml
- Resources/Prototypes/_Backmen/Research/scientific.yml
- Resources/Prototypes/_Backmen/Research/machines_service.yml
- Resources/Prototypes/_Backmen/Research/service.yml
- Resources/Prototypes/_Backmen/Research/engineering.yml
- Resources/Prototypes/_Backmen/Research/shaft.yml
- Resources/Prototypes/_Backmen/Research/arsenal.yml
- Resources/Prototypes/_Backmen/Research/machines_scientific.yml
- Resources/Prototypes/_Backmen/Research/robotics.yml
- Resources/Prototypes/_Backmen/Research/medical.yml
- Resources/Prototypes/Research/civilianservices.yml
- Resources/Prototypes/Research/arsenal.yml
Comment on lines
+73
to
+84
| public static bool HasUnmetRequirements( | ||
| SharedResearchSystem research, | ||
| TechnologyPrototype technology, | ||
| TechnologyDatabaseComponent? component, | ||
| IPrototypeManager prototypeManager) | ||
| { | ||
| if (technology.TechnologyPrerequisites.Any(prereq => | ||
| component == null || !research.IsTechnologyUnlocked(EntityUid.Invalid, prereq, component))) | ||
| return true; | ||
|
|
||
| if (component == null) | ||
| return false; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Не приравнивайте component == null к отсутствию требований.
false здесь означает «все требования выполнены», но без TechnologyDatabaseComponent метод просто не может проверить tier/discipline-гейты. В FancyTechnologyInfoPanel.InitializePrerequisites() это превращается в зелёное research-console-no-tech-requirements для технологий, у которых требования неизвестны, а не отсутствуют.
🤖 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/Backmen/Research/BkmResearchRequirements.cs` around lines 73 -
84, `HasUnmetRequirements` is treating a missing `TechnologyDatabaseComponent`
as if all requirements were satisfied, which incorrectly makes
`FancyTechnologyInfoPanel.InitializePrerequisites()` show the no-requirements
state for unknown tiers/discipline gates. Update
`BkmResearchRequirements.HasUnmetRequirements` so that a null `component` is
handled as “requirements cannot be verified” rather than returning false; keep
the prerequisite check using `SharedResearchSystem.IsTechnologyUnlocked`, and
make the method report unmet requirements whenever the database component needed
to evaluate tier/discipline gating is absent.
Comment on lines
+70
to
+100
| private static Dictionary<string, DisciplineCell> BuildDisciplineCells( | ||
| List<TechnologyPrototype> visible, | ||
| List<string> disciplines) | ||
| { | ||
| var cells = new Dictionary<string, DisciplineCell>(); | ||
|
|
||
| foreach (var discipline in disciplines) | ||
| { | ||
| var disciplineTechs = visible.Where(t => t.Discipline == discipline).ToList(); | ||
| if (disciplineTechs.Count == 0) | ||
| continue; | ||
|
|
||
| var positions = new Dictionary<string, Vector2i>(); | ||
| var maxWidth = 0; | ||
| var maxHeight = 0; | ||
|
|
||
| foreach (var tierGroup in disciplineTechs.GroupBy(t => t.Tier).OrderBy(g => g.Key)) | ||
| { | ||
| var ordered = OrderWithinTier(tierGroup.ToList(), disciplineTechs); | ||
| var rowY = (tierGroup.Key - 1) * TierRowSpacing; | ||
| maxHeight = Math.Max(maxHeight, rowY + 1); | ||
|
|
||
| for (var i = 0; i < ordered.Count; i++) | ||
| { | ||
| positions[ordered[i].ID] = new Vector2i(i, rowY); | ||
| maxWidth = Math.Max(maxWidth, i + 1); | ||
| } | ||
| } | ||
|
|
||
| cells[discipline] = new DisciplineCell(positions, maxWidth, maxHeight); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Смешанный auto-layout и ручной Position сейчас не резервирует место под override-координаты.
BuildDisciplineCells() считает Width/Height только по авторасстановке, но меню потом всё равно предпочитает proto.Position (FancyResearchConsoleMenu, Lines 123-125 и 267-270). Любой ручной Position за пределами рассчитанной ячейки может наложиться на соседний macro-cell; либо учитывайте manual overrides при расчёте размеров, либо запретите смешанный режим явно.
🤖 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/Backmen/Research/BkmResearchTreeLayout.cs` around lines 70 -
100, The mixed auto-layout/manual Position path in BuildDisciplineCells does not
reserve space for overridden coordinates, so the computed DisciplineCell
Width/Height can be too small when FancyResearchConsoleMenu later prefers
proto.Position. Update BuildDisciplineCells to include any manual Position
overrides in the bounds calculation, or explicitly reject mixed auto/manual
placement, and make sure the sizing logic in DisciplineCell matches the
placement logic used by FancyResearchConsoleMenu.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Content.IntegrationTests/Tests/Backmen/Medical/HealingFallbackTest.cs (1)
92-99: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winТест не проверяет, что сильное кровотечение действительно достигнуто.
Цикл по
GetWoundableWoundsWithComp<BleedInflicterComponent>молча выполнит ноль итераций, если у индуцированнойSlash-раны нетBleedInflicterComponent, — тогда поля кровотечения не выставятся, а тест всё равно останется зелёным, проверяя лишь снятиеCold. Это та же проблема, что отмечалась ранее: нет явного утверждения, что грудь классифицирована какheavy bleeding.Рекомендую добавить счётчик/
Assert, что цикл задел хотя бы одну рану, и проверку фактического состояния сильного кровотечения после правок.Скрипт для подтверждения порога и поля, определяющих
heavy bleeding:#!/bin/bash # Найти определение порога/проверки сильного кровотечения rg -nP 'HeavyBleeding|heavy.?bleed|4\.5' --type=cs -C3 # Проверить, какие поля BleedInflicterComponent участвуют в расчёте кровотечения fd -i 'BleedInflicterComponent.cs' --exec cat -n {} rg -nP '\bBleedingAmountRaw\b|\bIsBleeding\b|\bScaling\b' --type=cs -C2🤖 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.IntegrationTests/Tests/Backmen/Medical/HealingFallbackTest.cs` around lines 92 - 99, The bleeding setup in the healing fallback test can silently do nothing if no wound has BleedInflicterComponent, so add an explicit assertion in HealingFallbackTest after the GetWoundableWoundsWithComp<BleedInflicterComponent> loop that at least one wound was updated. Also verify the chest is actually in the heavy bleeding state after setting BleedingAmountRaw, IsBleeding, and Scaling, so the test checks the intended condition rather than only the Cold removal path.
🤖 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.
Duplicate comments:
In `@Content.IntegrationTests/Tests/Backmen/Medical/HealingFallbackTest.cs`:
- Around line 92-99: The bleeding setup in the healing fallback test can
silently do nothing if no wound has BleedInflicterComponent, so add an explicit
assertion in HealingFallbackTest after the
GetWoundableWoundsWithComp<BleedInflicterComponent> loop that at least one wound
was updated. Also verify the chest is actually in the heavy bleeding state after
setting BleedingAmountRaw, IsBleeding, and Scaling, so the test checks the
intended condition rather than only the Cold removal path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fae1c829-8c29-425b-b2f6-44704dcce8e4
📒 Files selected for processing (1)
Content.IntegrationTests/Tests/Backmen/Medical/HealingFallbackTest.cs
Contributor
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.IntegrationTests/Tests/GameRules/NukeOpsTest.cs`:
- Around line 253-263: В тесте NukeOpsTest в блоке с damageSys.TryChangeDamage и
финальным Assert.That(GetTotalDamage, Is.EqualTo(FixedPoint2.Zero)) не нужно
безусловно снимать весь накопленный урон, потому что это скрывает регрессии.
Вместо этого в этом участке кода сначала проверьте, что у player есть только
ожидаемый небольшой транзиентный урон (или что это именно асфиксический урон), и
лишь при выполнении этого условия выполняйте обнуление через TryChangeDamage;
если урон выше порога или другого типа, тест должен падать. Используйте
ориентиры по символам damageSys.GetTotalDamage, scratchDamage и TryChangeDamage,
чтобы локализовать место правки.
🪄 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
Run ID: a45eca1d-1846-4bf1-b4a6-bb50cb880a34
📒 Files selected for processing (1)
Content.IntegrationTests/Tests/GameRules/NukeOpsTest.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit