Conversation
WalkthroughДобавлены HTN-предусловие LaydownPrecondition и оператор StandOperator; StandingStateSystem интегрирован в зомбификацию и восстановление тел; в LavalandPlanetSystem добавлен/перенес сложный механизм загрузки и слияния руин; внедрена масштабная GPT-логика в NPCConversationSystem; обновлены прототипы NPC; удалены/упрощены некоторые зависимости. Changes
Sequence Diagram(s)sequenceDiagram
participant Planner as HTN Planner
participant BB as NPCBlackboard
participant Owner as NPC Entity
participant StandSys as StandingStateSystem
Planner->>BB: Evaluate LaydownPrecondition(IsDown)
BB-->>Planner: return owner Entity
alt Owner needs to change state
Planner->>StandSys: Startup(owner) => Stand(owner)
StandSys-->>Owner: perform Stand action
StandSys-->>Planner: acknowledge
Planner->>Planner: Update -> return Finished
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Content.Server/Backmen/Shipwrecked/NPCZombieSystem.cs (1)
67-78: Дублирующееся присваиваниеHealingOnBite.
z.HealingOnBite = new()устанавливается дважды: на строках 67 и 77. Первое присваивание избыточно.💚 Удаление дубликата
if (!ev.IsBoss) { - z.HealingOnBite = new(); - DamageSpecifier dspec = new() { DamageDict = new()Resources/Prototypes/_Backmen/Entities/Mobs/NPCs/glimmer_monsters.yml (1)
221-230: Риск слишком широкого выбора цели для UtilityOperator.После удаления ограничений по LOS/дистанции
NearbyGunWispTargetsтеперь может выбирать цели за стенами или на другом конце карты. Это ведёт к лишнему pathfinding и странному поведению (бесконечное преследование). Если это не намеренно, стоит вернуть хотя быTargetDistanceConиTargetInLOSOrCurrentConлибо ограничить радиус запроса.
🤖 Fix all issues with AI agents
In `@Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.Grid.cs`:
- Around line 89-94: When the intersection check (usedSpace.Any(used =>
used.Intersects(ruinBox))) fails you must remove the temporary grid that was
loaded onto the preloader before returning false; locate the variable that holds
the loaded grid (the temporary grid/entity placed on the preloader map when
computing bounds) and ensure you call the appropriate removal method (e.g.
_entityManager.DeleteEntity(loadedGrid) or preloader.RemoveEntity(loadedGrid))
right before the early return in the failing branch so the unused grid entity is
not left on the preloader.
- Around line 56-62: The code currently builds a list `bounds` from
`manager.Fixtures.Values` (filtering `fixture.Hard`) and then calls
`_random.Pick(bounds)` which will throw if `bounds` is empty; add a check after
computing `bounds` to handle empty lists: log an error mentioning the ruin
preloader/spawn (`spawned` and `preloader.Owner` or other contextual
identifiers), delete the temporary entity (`spawned.Value` or the same temp
entity used) and return false from the enclosing method; ensure you reference
the same variables used here (`_physics.GetRelativePhysicsTransform`,
`manager.Fixtures`, `fixture.Hard`, `_random.Pick`) so the fix is applied in the
same block.
In `@Content.Shared/_Lavaland/Aggression/SharedAggressorsSystem.cs`:
- Around line 34-38: В обработчике OnBeforeDamageChanged не отменяй урон для
сущностей без ActorComponent на событиях с null Origin — вместо присваивания
args.Cancelled = true просто прекрати обработку и выйди из метода: проверку
оставь (args.Origin == null && !_actorQuery.HasComp(ent)) но не выставляй
Cancelled; нужен лишь ранний return в методе OnBeforeDamageChanged, чтобы не
блокировать средовой урон, при этом сохранил логику работы с
BeforeDamageChangedEvent и метод/поле _actorQuery.HasComp(ent).
In `@Content.Shared/_White/BackStab/BackStabSystem.cs`:
- Around line 100-103: В вызове _audio.PlayPredicted(BackstabSound, user,
target) поменяйте местами аргументы user и target, т.е. передавайте target как
источник звука, чтобы вызов стал _audio.PlayPredicted(BackstabSound, target,
user); это исправит некорректную семантику источника звука для эффекта в
BackStabSystem, сохраняя вызов PopupPredicted без изменений.
In `@Resources/Prototypes/NPCs/Combat/melee.yml`:
- Around line 39-45: В файле YAML есть несогласованность отступов: блоки списка
под preconditions и tasks используют 2 пробела, тогда как остальные элементы
(строки 16–37) — 4 пробела; приведите отступы к единому стилю (4 пробела) для
записей - !type:LaydownPrecondition, - !type:HTNPrimitiveTask и вложенных полей
operator, shutdownState: TaskFinished и isDown: true, сохранив структуру и
вложенность.
🧹 Nitpick comments (3)
Content.Server/Backmen/NPC/HTN/PrimitiveTasks/Operators/Combat/StandOperator.cs (1)
15-20: РезультатStandигнорируется.Метод
_stand.Stand(owner)возвращаетbool, указывающий на успешность операции. Если вставание не удалось (например, отменено событиемStandAttemptEvent), оператор всё равно вернётFinished. Это может быть намеренным, но стоит убедиться, что это ожидаемое поведение.♻️ Опциональное улучшение: логирование или обработка неудачи
public override void Startup(NPCBlackboard blackboard) { base.Startup(blackboard); var owner = blackboard.GetValue<EntityUid>(NPCBlackboard.Owner); - _stand.Stand(owner); + if (!_stand.Stand(owner)) + { + // Логирование или установка флага неудачи при необходимости + } }Content.Server/Backmen/NPC/HTN/LaydownPrecondition.cs (1)
24-30: ЛогикуIsMetможно упростить.Текущее выражение эквивалентно простому сравнению равенства.
♻️ Упрощение логики
public override bool IsMet(NPCBlackboard blackboard) { var owner = blackboard.GetValue<EntityUid>(NPCBlackboard.Owner); - return IsDown && _stand.IsDown(owner) || - !IsDown && !_stand.IsDown(owner); + return IsDown == _stand.IsDown(owner); }Content.Shared/_Lavaland/Aggression/SharedAggressorsSystem.cs (1)
77-83: Guard для Aggro/DeAggro, чтобы избежать повторных побочных эффектов.Сейчас
AggroEntity/DeAggroEntityвызываются даже если агрессор уже есть/его нет вHashSet. Это может дублировать работу и потенциально портить состояние, если методы не идемпотентны. Лучше вызывать их только при реальном изменении множества.♻️ Вариант правки
public void RemoveAggressor(Entity<AggressiveComponent> ent, EntityUid aggressor) { - _npcFaction.DeAggroEntity(ent.Owner, aggressor); - ent.Comp.Aggressors.Remove(aggressor); + if (!ent.Comp.Aggressors.Remove(aggressor)) + return; + _npcFaction.DeAggroEntity(ent.Owner, aggressor); RaiseLocalEvent(ent, new AggressorRemovedEvent(GetNetEntity(aggressor))); } public void AddAggressor(Entity<AggressiveComponent> ent, EntityUid aggressor) { - _npcFaction.AggroEntity(ent.Owner, aggressor); - ent.Comp.Aggressors.Add(aggressor); + var isNew = ent.Comp.Aggressors.Add(aggressor); + if (isNew) + _npcFaction.AggroEntity(ent.Owner, aggressor); var aggcomp = EnsureComp<AggressorComponent>(aggressor); - RaiseLocalEvent(ent, new AggressorAddedEvent(GetNetEntity(aggressor))); + if (isNew) + RaiseLocalEvent(ent, new AggressorAddedEvent(GetNetEntity(aggressor))); aggcomp.Aggressives.Add(ent);Also applies to: 97-107
| // Actually calculate ruin bound | ||
| var transform = _physics.GetRelativePhysicsTransform(spawned.Value, preloader.Owner); | ||
| // holy shit | ||
| var bounds = (from fixture in manager.Fixtures.Values where fixture.Hard select fixture.Shape.ComputeAABB(transform, 0).Rounded(0)).ToList(); | ||
| // Round this list of boxes up to | ||
| var calculatedBox = _random.Pick(bounds); | ||
| foreach (var bound in bounds) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "LavalandPlanetSystem.Grid.cs" 2>/dev/null | head -20Repository: Rxup/space-station-14
Length of output: 186
🏁 Script executed:
cat -n "Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.Grid.cs" | sed -n '50,70p'Repository: Rxup/space-station-14
Length of output: 1108
🏁 Script executed:
# Search for _random.Pick implementation
rg "_random\.Pick" -A 5 -B 2 --type csRepository: Rxup/space-station-14
Length of output: 50378
🏁 Script executed:
# Search for the Random class implementation to understand Pick behavior
rg "Pick\(" --type cs | grep -i "random\|\.pick" | head -20Repository: Rxup/space-station-14
Length of output: 2719
🏁 Script executed:
# Search for existing empty collection guards in similar contexts
rg "\.Count\s*==\s*0" --type cs | head -20Repository: Rxup/space-station-14
Length of output: 2349
🏁 Script executed:
# Search for Random.Pick implementation in the codebase
rg "public.*Pick\(" --type cs -A 10 | grep -A 10 "IRobustRandom\|IEnumerable"Repository: Rxup/space-station-14
Length of output: 3951
🏁 Script executed:
# Check the Random extension methods that implement Pick
find . -name "*.cs" -type f -exec grep -l "SharedRandomExtensions\|RandomExtensions" {} \; 2>/dev/null | head -5Repository: Rxup/space-station-14
Length of output: 327
🏁 Script executed:
# Look at the actual Random.Pick implementation
fd -t f "RandomExtensions.cs" | head -3Repository: Rxup/space-station-14
Length of output: 120
🏁 Script executed:
cat -n "Content.Shared/Random/Helpers/SharedRandomExtensions.cs" | head -100Repository: Rxup/space-station-14
Length of output: 3899
🏁 Script executed:
# Search for the generic Pick<T> overload that works with IList/IEnumerable
rg "public static.*Pick.*IList|public static.*Pick.*IEnumerable" --type cs -A 8 -B 2Repository: Rxup/space-station-14
Length of output: 47
🏁 Script executed:
cat -n "Content.Shared/Random/Helpers/SharedRandomExtensions.cs" | sed -n '100,200p'Repository: Rxup/space-station-14
Length of output: 4047
🏁 Script executed:
# Search for Pick<T> with IList or similar collections
rg "Pick.*IList|Pick.*List\<" --type cs -B 2 -A 8Repository: Rxup/space-station-14
Length of output: 177
🏁 Script executed:
cat -n "Content.Shared/Random/Helpers/SharedRandomExtensions.cs" | tail -100Repository: Rxup/space-station-14
Length of output: 4055
🏁 Script executed:
# Search for Pick method overloads that work with lists/collections
rg "public static.*Pick" --type cs Content.Shared/Random/Helpers/SharedRandomExtensions.csRepository: Rxup/space-station-14
Length of output: 1143
🏁 Script executed:
# Look in Robust.Shared for the base Pick implementation
fd -t f "RandomExtensions.cs" 2>/dev/null | xargs grep -l "Pick" 2>/dev/null | head -5Repository: Rxup/space-station-14
Length of output: 120
🏁 Script executed:
# Search for Pick implementations in the entire codebase that work with lists
rg "Pick.*IList\|Pick.*IEnumerable\|Pick.*List" --type cs -B 2 -A 5 | head -100Repository: Rxup/space-station-14
Length of output: 47
🏁 Script executed:
# Look for the actual generic Pick method used on bounds (which is List<Box2>)
# Search files that might have the base implementation
find . -type f -name "*.cs" | xargs grep -l "public static.*Pick.*where.*:" | head -10Repository: Rxup/space-station-14
Length of output: 47
🏁 Script executed:
# Check robustness library or look directly in the used interfaces
rg "IRobustRandom.*Pick" --type cs -A 10 | head -150Repository: Rxup/space-station-14
Length of output: 47
🏁 Script executed:
# Search in the code for similar patterns where Pick is called on a list
rg "_random\.Pick.*bounds\|_random\.Pick.*list\|_random\.Pick.*\(\)" --type cs -B 3 -A 3Repository: Rxup/space-station-14
Length of output: 47
🏁 Script executed:
# Look for ANY Pick usage with angle brackets (generic)
rg "\.Pick\<" --type cs | head -20Repository: Rxup/space-station-14
Length of output: 145
🏁 Script executed:
# Try to understand the actual usage at the problem location
cat -n "Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.Grid.cs" | sed -n '50,75p'Repository: Rxup/space-station-14
Length of output: 1368
🏁 Script executed:
# Search for IEnumerable/List Pick implementations across the repo
rg "Pick.*this.*IEnumerable|Pick.*this.*IList" --type cs -B 2 -A 5Repository: Rxup/space-station-14
Length of output: 47
🏁 Script executed:
# Search for any other Pick method that might handle List
rg "Pick" "Content.Shared/Random/Helpers/SharedRandomExtensions.cs" --type csRepository: Rxup/space-station-14
Length of output: 1351
🏁 Script executed:
# Check if there's a generic Pick override that handles any enumerable
python3 << 'EOF'
import subprocess
import re
# Run rg to get all function definitions in the file
result = subprocess.run(
["rg", "public static.*Pick", "Content.Shared/Random/Helpers/SharedRandomExtensions.cs", "-A", "1"],
capture_output=True,
text=True
)
print("All Pick method signatures:")
print(result.stdout)
EOFRepository: Rxup/space-station-14
Length of output: 151
🏁 Script executed:
# Look at what _random is - check the type definition
rg "_random" "Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.Grid.cs" -B 5 | head -30Repository: Rxup/space-station-14
Length of output: 486
🏁 Script executed:
# Search for IRobustRandom usage and see what methods are available
rg "IRobustRandom.*Pick" --type cs -B 5 -A 10 | head -80Repository: Rxup/space-station-14
Length of output: 47
🏁 Script executed:
# Let's check if there's a generic Pick method that works with List in the Robust.Shared library or elsewhere
# by looking at actual usages to understand the signature
rg "Pick\(.*List\|Pick\(.*bounds" --type cs -B 3 -A 3 | head -100Repository: Rxup/space-station-14
Length of output: 47
🏁 Script executed:
# Search for how Pick is typically used with lists across the codebase
rg "_random\.Pick\(" --type cs -A 1 | grep -E "\.ToList|\.Values|bounds|entities" | head -30Repository: Rxup/space-station-14
Length of output: 3362
🏁 Script executed:
# Try to understand by looking at test files
fd "RandomExtensionsTests" --type f | head -3Repository: Rxup/space-station-14
Length of output: 118
🏁 Script executed:
cat -n "Content.Tests/Shared/Utility/RandomExtensionsTests.cs"Repository: Rxup/space-station-14
Length of output: 1568
Добавьте проверку пустого списка перед вызовом _random.Pick.
Если у руины нет фиксчур с флагом Hard, список bounds будет пустым и вызов _random.Pick(bounds) выбросит исключение. Добавьте проверку, залогируйте ошибку, удалите временную сущность и верните false.
🐛 Предложенное исправление
var bounds = (from fixture in manager.Fixtures.Values where fixture.Hard select fixture.Shape.ComputeAABB(transform, 0).Rounded(0)).ToList();
+ if (bounds.Count == 0)
+ {
+ Log.Error($"Ruin {ruin.ID} has no hard fixtures to compute bounds");
+ Del(spawned.Value);
+ return false;
+ }
// Round this list of boxes up to
var calculatedBox = _random.Pick(bounds);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Actually calculate ruin bound | |
| var transform = _physics.GetRelativePhysicsTransform(spawned.Value, preloader.Owner); | |
| // holy shit | |
| var bounds = (from fixture in manager.Fixtures.Values where fixture.Hard select fixture.Shape.ComputeAABB(transform, 0).Rounded(0)).ToList(); | |
| // Round this list of boxes up to | |
| var calculatedBox = _random.Pick(bounds); | |
| foreach (var bound in bounds) | |
| // Actually calculate ruin bound | |
| var transform = _physics.GetRelativePhysicsTransform(spawned.Value, preloader.Owner); | |
| // holy shit | |
| var bounds = (from fixture in manager.Fixtures.Values where fixture.Hard select fixture.Shape.ComputeAABB(transform, 0).Rounded(0)).ToList(); | |
| if (bounds.Count == 0) | |
| { | |
| Log.Error($"Ruin {ruin.ID} has no hard fixtures to compute bounds"); | |
| Del(spawned.Value); | |
| return false; | |
| } | |
| // Round this list of boxes up to | |
| var calculatedBox = _random.Pick(bounds); | |
| foreach (var bound in bounds) |
🤖 Prompt for AI Agents
In `@Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.Grid.cs`
around lines 56 - 62, The code currently builds a list `bounds` from
`manager.Fixtures.Values` (filtering `fixture.Hard`) and then calls
`_random.Pick(bounds)` which will throw if `bounds` is empty; add a check after
computing `bounds` to handle empty lists: log an error mentioning the ruin
preloader/spawn (`spawned` and `preloader.Owner` or other contextual
identifiers), delete the temporary entity (`spawned.Value` or the same temp
entity used) and return false from the enclosing method; ensure you reference
the same variables used here (`_physics.GetRelativePhysicsTransform`,
`manager.Fixtures`, `fixture.Hard`, `_random.Pick`) so the fix is applied in the
same block.
| // If any used boundary intersects with current boundary, return | ||
| if (usedSpace.Any(used => used.Intersects(ruinBox))) | ||
| { | ||
| Log.Debug("Ruin can't be placed on it's coordinates, skipping spawn"); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.Grid.cs | head -120 | tail -40Repository: Rxup/space-station-14
Length of output: 186
🏁 Script executed:
cat -n Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.Grid.cs | head -150 | tail -80Repository: Rxup/space-station-14
Length of output: 4440
🏁 Script executed:
wc -l Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.Grid.csRepository: Rxup/space-station-14
Length of output: 141
🏁 Script executed:
sed -n '1,150p' Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.Grid.cs | cat -nRepository: Rxup/space-station-14
Length of output: 7653
Удалите временный грид при отказе размещения.
При первом вычислении границ грид загружается и помещается на карту preloader (строка 79). Если проверка пересечения (строка 90) обнаружит конфликт, метод возвращает false, но загруженный грид остаётся на preloader-карте, так как не удаляется. Это приводит к накоплению неиспользованных сущностей.
🐛 Исправление
if (usedSpace.Any(used => used.Intersects(ruinBox)))
{
Log.Debug("Ruin can't be placed on it's coordinates, skipping spawn");
+ if (spawned != null)
+ Del(spawned.Value);
return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // If any used boundary intersects with current boundary, return | |
| if (usedSpace.Any(used => used.Intersects(ruinBox))) | |
| { | |
| Log.Debug("Ruin can't be placed on it's coordinates, skipping spawn"); | |
| return false; | |
| } | |
| // If any used boundary intersects with current boundary, return | |
| if (usedSpace.Any(used => used.Intersects(ruinBox))) | |
| { | |
| Log.Debug("Ruin can't be placed on it's coordinates, skipping spawn"); | |
| if (spawned != null) | |
| Del(spawned.Value); | |
| return false; | |
| } |
🤖 Prompt for AI Agents
In `@Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.Grid.cs`
around lines 89 - 94, When the intersection check (usedSpace.Any(used =>
used.Intersects(ruinBox))) fails you must remove the temporary grid that was
loaded onto the preloader before returning false; locate the variable that holds
the loaded grid (the temporary grid/entity placed on the preloader map when
computing bounds) and ensure you call the appropriate removal method (e.g.
_entityManager.DeleteEntity(loadedGrid) or preloader.RemoveEntity(loadedGrid))
right before the early return in the failing branch so the unused grid entity is
not left on the preloader.
| private void OnBeforeDamageChanged(Entity<AggressiveComponent> ent, ref BeforeDamageChangedEvent args) | ||
| { | ||
| if (args.Origin == null) | ||
| if (args.Origin == null && !_actorQuery.HasComp(ent)) | ||
| args.Cancelled = true; | ||
| } |
There was a problem hiding this comment.
Линия 36 отменяет урон по NPC от без-Origins источников.
Сейчас args.Cancelled = true срабатывает для урона без Origin по сущностям без ActorComponent, что делает NPC фактически неуязвимыми к средовым источникам (огонь, космос, ловушки). Если цель — лишь не учитывать агрессора, лучше не отменять урон и просто выйти из обработчика.
🛠️ Вариант правки
- if (args.Origin == null && !_actorQuery.HasComp(ent))
- args.Cancelled = true;
+ if (args.Origin == null || !_actorQuery.HasComp(args.Origin.Value))
+ return;🤖 Prompt for AI Agents
In `@Content.Shared/_Lavaland/Aggression/SharedAggressorsSystem.cs` around lines
34 - 38, В обработчике OnBeforeDamageChanged не отменяй урон для сущностей без
ActorComponent на событиях с null Origin — вместо присваивания args.Cancelled =
true просто прекрати обработку и выйди из метода: проверку оставь (args.Origin
== null && !_actorQuery.HasComp(ent)) но не выставляй Cancelled; нужен лишь
ранний return в методе OnBeforeDamageChanged, чтобы не блокировать средовой
урон, при этом сохранил логику работы с BeforeDamageChangedEvent и метод/поле
_actorQuery.HasComp(ent).
| - preconditions: | ||
| - !type:LaydownPrecondition | ||
| isDown: true | ||
| tasks: | ||
| - !type:HTNPrimitiveTask | ||
| operator: !type:StandOperator | ||
| shutdownState: TaskFinished |
There was a problem hiding this comment.
Несогласованность отступов в YAML.
Новая ветка использует 2-пробельные отступы для элементов списка (- !type:LaydownPrecondition), тогда как предыдущие ветки (строки 16-37) используют 4-пробельные отступы. Рекомендуется выровнять стиль для согласованности.
💚 Предлагаемое исправление отступов
- preconditions:
- - !type:LaydownPrecondition
- isDown: true
- tasks:
- - !type:HTNPrimitiveTask
- operator: !type:StandOperator
- shutdownState: TaskFinished
+ - !type:LaydownPrecondition
+ isDown: true
+ tasks:
+ - !type:HTNPrimitiveTask
+ operator: !type:StandOperator
+ shutdownState: TaskFinished🤖 Prompt for AI Agents
In `@Resources/Prototypes/NPCs/Combat/melee.yml` around lines 39 - 45, В файле
YAML есть несогласованность отступов: блоки списка под preconditions и tasks
используют 2 пробела, тогда как остальные элементы (строки 16–37) — 4 пробела;
приведите отступы к единому стилю (4 пробела) для записей -
!type:LaydownPrecondition, - !type:HTNPrimitiveTask и вложенных полей operator,
shutdownState: TaskFinished и isDown: true, сохранив структуру и вложенность.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Content.Shared/_White/BackStab/BackStabSystem.cs (1)
56-83: Исправьте вызывающие места из-за смены defaultalwaysBackstabLayingсtrueнаfalse.Параметр был изменён с дефолтного значения
trueнаfalse. Это влияет на следующие вызовы:
BackStabSystem.cs:46—TryBackstab(target, args.User, ent.Comp.Tolerance)— полагается на дефолтDamageMarkerSystem.cs:40—TryBackstab(uid, args.User, Angle.FromDegrees(45d), playSound: false)— полагается на дефолтЯвно передайте
alwaysBackstabLaying: trueв эти вызовы, если требуется прежнее поведение, или обновите логику в зависимости от желаемого результата.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Content.Server/Zombies/ZombieSystem.Transform.cs`:
- Line 276: Вызов _bodySystem.ForceRestoreBody(target, true) использует явный
аргумент, совпадающий со значением параметра по умолчанию; упростите вызов,
убрав передачу true и вызвав _bodySystem.ForceRestoreBody(target) вместо
_bodySystem.ForceRestoreBody(target, true). Найдите использование
ForceRestoreBody в файле (в контексте переменной _bodySystem и параметра
removeWounds) и удалите явный true, оставив только target.
| PainType.TraumaticPain, | ||
| nerveSys.Value); | ||
| } | ||
| _bodySystem.ForceRestoreBody(target, true); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Step 1: Find the ForceRestoreBody method definition
echo "=== Searching for ForceRestoreBody method signature ==="
rg -n --type cs 'ForceRestoreBody' --max-count=20
echo ""
echo "=== Examining the ZombieSystem.Transform.cs file around line 276 ==="
# First, locate the file
find . -name "ZombieSystem.Transform.cs" -type fRepository: Rxup/space-station-14
Length of output: 702
🏁 Script executed:
#!/bin/bash
# Step 2: Read the context around line 276 in ZombieSystem.Transform.cs
echo "=== Code context around line 276 ==="
sed -n '250,290p' ./Content.Server/Zombies/ZombieSystem.Transform.cs | cat -n
echo ""
echo "=== Full method signature of ForceRestoreBody ==="
sed -n '249,260p' ./Content.Shared/Body/Systems/SharedBodySystem.Body.cs | cat -n
echo ""
echo "=== Check broader context (lines 259-280) ==="
sed -n '259,280p' ./Content.Server/Zombies/ZombieSystem.Transform.cs | cat -nRepository: Rxup/space-station-14
Length of output: 3522
Упростите вызов, используя значение параметра по умолчанию.
Параметр removeWounds в методе ForceRestoreBody имеет значение по умолчанию true. Вызов _bodySystem.ForceRestoreBody(target, true) на строке 276 можно упростить до _bodySystem.ForceRestoreBody(target) без явной передачи параметра, совпадающего с умолчанием.
Размещение вызова вне блока проверки нервной системы является намеренным — метод применяется ко всем зомбифицированным сущностям. Параметр removeWounds достаточно ясно описывает назначение.
🤖 Prompt for AI Agents
In `@Content.Server/Zombies/ZombieSystem.Transform.cs` at line 276, Вызов
_bodySystem.ForceRestoreBody(target, true) использует явный аргумент,
совпадающий со значением параметра по умолчанию; упростите вызов, убрав передачу
true и вызвав _bodySystem.ForceRestoreBody(target) вместо
_bodySystem.ForceRestoreBody(target, true). Найдите использование
ForceRestoreBody в файле (в контексте переменной _bodySystem и параметра
removeWounds) и удалите явный true, оставив только target.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@Content.Server/Backmen/NPC/Systems/NPCConversationSystem.cs`:
- Around line 856-889: In ProcessGptResponse, when handling choices with
finish_reason "stop"/"length"/"blacklist" you currently only clear
uid.Comp.GptProcessing when gptMsg.message.content != null, leaving the flag
true for null content; change the flow so GptProcessing is set to false
regardless of whether gptMsg.message.content is null (e.g., add an else branch
that sets uid.Comp.GptProcessing = false or move that assignment after the
content-handling block), and keep the history.Lock.EnterWriteLock()/Add() logic
strictly inside the non-null-content branch so you don't acquire the lock for
null content; target the ProcessGptResponse method and the
gptMsg.message.content check to implement this fix.
- Around line 970-1006: In TryGptResponse the speaker name is pulled via
MetaData(speaker).EntityName which can bypass masking; change to resolve the
displayed identity using Identity.Name(speaker) before adding to the GPT history
(the GetOrCreateGptHistory/history.Add(new GptMessageChat(...)) call) so
masked/hidden identities are used when creating the GptMessageChat; ensure you
reference the same speaker EntityUid variable and remove direct use of
MetaData(speaker).EntityName.
- Around line 376-394: The idle-line selection in TryGetIdleChatLine incorrectly
pre-increments uid.Comp.IdleChatIndex causing the first entry in
uid.Comp.IdleChatOrder to be skipped; change the flow to read the current index
first and then advance it (or use post-increment) and only shuffle/reset when
the next index would exceed Count — e.g., compute var index =
uid.Comp.IdleChatOrder[uid.Comp.IdleChatIndex], then increment
uid.Comp.IdleChatIndex (or uid.Comp.IdleChatIndex++) and if it equals Count
reset to 0 and call _random.Shuffle(uid.Comp.IdleChatOrder); ensure you
reference TryGetIdleChatLine, IdleChatIndex, IdleChatOrder,
ConversationTree.Idle and _random.Shuffle when making the change.
🧹 Nitpick comments (1)
Content.Server/Backmen/NPC/Systems/NPCConversationSystem.cs (1)
1009-1028: Верните в GPT только сериализуемый/минимальный список тем.
Передача целыхNPCTopicможет раздуть payload и/или не сериализоваться. Лучше отдавать промпты и флаги.♻️ Вариант упрощения payload
- ev.History.Add(new GptMessageFunction("GetAvailableTopics", new { result = GetAvailableTopics(ev.NpcUid).ToArray() })); + var topics = GetAvailableTopics(ev.NpcUid) + .Select(t => new { prompts = t.Prompts, locked = t.Locked, hidden = t.Hidden }) + .ToArray(); + ev.History.Add(new GptMessageFunction("GetAvailableTopics", new { result = topics }));
| private bool TryGetIdleChatLine(Entity<NPCConversationComponent> uid, [NotNullWhen(true)] out NPCResponse? line) | ||
| { | ||
| line = null; | ||
|
|
||
| if (component.IdleChatOrder.Count() == 0) | ||
| if (!uid.Comp.IdleChatOrder.Any()) | ||
| return false; | ||
|
|
||
| if (++component.IdleChatIndex == component.IdleChatOrder.Count()) | ||
| if (++uid.Comp.IdleChatIndex == uid.Comp.IdleChatOrder.Count()) | ||
| { | ||
| // Exhausted all lines in the pre-shuffled order. | ||
| // Reset the index and shuffle again. | ||
| component.IdleChatIndex = 0; | ||
| _random.Shuffle(component.IdleChatOrder); | ||
| uid.Comp.IdleChatIndex = 0; | ||
| _random.Shuffle(uid.Comp.IdleChatOrder); | ||
| } | ||
|
|
||
| var index = component.IdleChatOrder[component.IdleChatIndex]; | ||
| var index = uid.Comp.IdleChatOrder[uid.Comp.IdleChatIndex]; | ||
|
|
||
| line = component.ConversationTree.Idle[index]; | ||
| line = uid.Comp.ConversationTree.Idle[index]; | ||
|
|
There was a problem hiding this comment.
Исправьте off‑by‑one в выборе idle‑фраз.
Сейчас первый элемент порядка никогда не используется из‑за ++IdleChatIndex до чтения.
🐛 Предлагаемый фикс
- if (++uid.Comp.IdleChatIndex == uid.Comp.IdleChatOrder.Count())
+ if (uid.Comp.IdleChatIndex >= uid.Comp.IdleChatOrder.Count())
{
// Exhausted all lines in the pre-shuffled order.
// Reset the index and shuffle again.
uid.Comp.IdleChatIndex = 0;
_random.Shuffle(uid.Comp.IdleChatOrder);
}
- var index = uid.Comp.IdleChatOrder[uid.Comp.IdleChatIndex];
+ var index = uid.Comp.IdleChatOrder[uid.Comp.IdleChatIndex++];🤖 Prompt for AI Agents
In `@Content.Server/Backmen/NPC/Systems/NPCConversationSystem.cs` around lines 376
- 394, The idle-line selection in TryGetIdleChatLine incorrectly pre-increments
uid.Comp.IdleChatIndex causing the first entry in uid.Comp.IdleChatOrder to be
skipped; change the flow to read the current index first and then advance it (or
use post-increment) and only shuffle/reset when the next index would exceed
Count — e.g., compute var index =
uid.Comp.IdleChatOrder[uid.Comp.IdleChatIndex], then increment
uid.Comp.IdleChatIndex (or uid.Comp.IdleChatIndex++) and if it equals Count
reset to 0 and call _random.Shuffle(uid.Comp.IdleChatOrder); ensure you
reference TryGetIdleChatLine, IdleChatIndex, IdleChatOrder,
ConversationTree.Idle and _random.Shuffle when making the change.
| private async Task ProcessGptResponse(Entity<NPCConversationComponent> uid, NPCGptHistory history, GptResponseApi info) | ||
| { | ||
| foreach (var gptMsg in info.choices) | ||
| { | ||
| switch (gptMsg.finish_reason) | ||
| { | ||
| case "function_call": | ||
| await ProcessFunctionCall(uid, history, gptMsg); | ||
| break; | ||
| case "stop": | ||
| case "length": | ||
| case "blacklist": | ||
| if (gptMsg.message.content != null) | ||
| { | ||
| history.Lock.EnterWriteLock(); | ||
| try | ||
| { | ||
| history.Add(new GptMessageChat(GptUserDirection.assistant, gptMsg.message.content)); | ||
| } | ||
| finally | ||
| { | ||
| history.Lock.ExitWriteLock(); | ||
| } | ||
|
|
||
| var response = new NPCResponse(gptMsg.message.content, null); | ||
| QueueResponse(uid.AsNullable(), response); | ||
| uid.Comp.GptProcessing = false; | ||
| } | ||
| break; | ||
| default: | ||
| _sawmill.Warning($"GPT вернул неподдерживаемый finish_reason: {gptMsg.finish_reason} для {ToPrettyString(uid)}"); | ||
| uid.Comp.GptProcessing = false; | ||
| break; | ||
| } |
There was a problem hiding this comment.
Сбрасывайте GptProcessing даже при пустом content.
При finish_reason и content == null флаг остаётся true, и NPC больше не отвечает через GPT.
🐛 Предлагаемый фикс
case "stop":
case "length":
case "blacklist":
if (gptMsg.message.content != null)
{
history.Lock.EnterWriteLock();
try
{
history.Add(new GptMessageChat(GptUserDirection.assistant, gptMsg.message.content));
}
finally
{
history.Lock.ExitWriteLock();
}
var response = new NPCResponse(gptMsg.message.content, null);
QueueResponse(uid.AsNullable(), response);
- uid.Comp.GptProcessing = false;
}
+ uid.Comp.GptProcessing = false;
break;🤖 Prompt for AI Agents
In `@Content.Server/Backmen/NPC/Systems/NPCConversationSystem.cs` around lines 856
- 889, In ProcessGptResponse, when handling choices with finish_reason
"stop"/"length"/"blacklist" you currently only clear uid.Comp.GptProcessing when
gptMsg.message.content != null, leaving the flag true for null content; change
the flow so GptProcessing is set to false regardless of whether
gptMsg.message.content is null (e.g., add an else branch that sets
uid.Comp.GptProcessing = false or move that assignment after the
content-handling block), and keep the history.Lock.EnterWriteLock()/Add() logic
strictly inside the non-null-content branch so you don't acquire the lock for
null content; target the ProcessGptResponse method and the
gptMsg.message.content check to implement this fix.
| private async Task TryGptResponse(Entity<NPCConversationComponent> uid, string message, EntityUid speaker) | ||
| { | ||
| if (!uid.Comp.UseGpt || uid.Comp.GptProcessing || !_gptEnabled) | ||
| return; | ||
|
|
||
| uid.Comp.GptProcessing = true; | ||
| try | ||
| { | ||
| var history = GetOrCreateGptHistory(uid); | ||
|
|
||
| history.Lock.EnterWriteLock(); | ||
| try | ||
| { | ||
| var speakerName = MetaData(speaker).EntityName; | ||
| history.Add(new GptMessageChat(GptUserDirection.user, $"{speakerName}: {message}")); | ||
| } | ||
| finally | ||
| { | ||
| history.Lock.ExitWriteLock(); | ||
| } | ||
|
|
||
| var (responseApi, err) = await SendGptApiRequest(history, uid); | ||
|
|
||
| if (err != null || responseApi == null) | ||
| { | ||
| _sawmill.Warning($"Не удалось получить ответ от GPT для {ToPrettyString(uid)}: {err}"); | ||
| uid.Comp.GptProcessing = false; | ||
| return; | ||
| } | ||
|
|
||
| QueueResponse(uid, response, component); | ||
| await ProcessGptResponse(uid, history, responseApi); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _sawmill.Error($"Исключение при обработке GPT запроса для {ToPrettyString(uid)}: {ex}"); | ||
| uid.Comp.GptProcessing = false; | ||
| } |
There was a problem hiding this comment.
Не раскрывайте истинное имя при скрытии личности.
MetaData(speaker).EntityName может обойти механики маскировки и “протечь” в GPT‑ответ. Лучше использовать Identity.Name(...).
🔧 Предлагаемый фикс
+using Content.Shared.IdentityManagement;
@@
- var speakerName = MetaData(speaker).EntityName;
+ var speakerName = Identity.Name(speaker, EntityManager, uid);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private async Task TryGptResponse(Entity<NPCConversationComponent> uid, string message, EntityUid speaker) | |
| { | |
| if (!uid.Comp.UseGpt || uid.Comp.GptProcessing || !_gptEnabled) | |
| return; | |
| uid.Comp.GptProcessing = true; | |
| try | |
| { | |
| var history = GetOrCreateGptHistory(uid); | |
| history.Lock.EnterWriteLock(); | |
| try | |
| { | |
| var speakerName = MetaData(speaker).EntityName; | |
| history.Add(new GptMessageChat(GptUserDirection.user, $"{speakerName}: {message}")); | |
| } | |
| finally | |
| { | |
| history.Lock.ExitWriteLock(); | |
| } | |
| var (responseApi, err) = await SendGptApiRequest(history, uid); | |
| if (err != null || responseApi == null) | |
| { | |
| _sawmill.Warning($"Не удалось получить ответ от GPT для {ToPrettyString(uid)}: {err}"); | |
| uid.Comp.GptProcessing = false; | |
| return; | |
| } | |
| QueueResponse(uid, response, component); | |
| await ProcessGptResponse(uid, history, responseApi); | |
| } | |
| catch (Exception ex) | |
| { | |
| _sawmill.Error($"Исключение при обработке GPT запроса для {ToPrettyString(uid)}: {ex}"); | |
| uid.Comp.GptProcessing = false; | |
| } | |
| private async Task TryGptResponse(Entity<NPCConversationComponent> uid, string message, EntityUid speaker) | |
| { | |
| if (!uid.Comp.UseGpt || uid.Comp.GptProcessing || !_gptEnabled) | |
| return; | |
| uid.Comp.GptProcessing = true; | |
| try | |
| { | |
| var history = GetOrCreateGptHistory(uid); | |
| history.Lock.EnterWriteLock(); | |
| try | |
| { | |
| var speakerName = Identity.Name(speaker, EntityManager, uid); | |
| history.Add(new GptMessageChat(GptUserDirection.user, $"{speakerName}: {message}")); | |
| } | |
| finally | |
| { | |
| history.Lock.ExitWriteLock(); | |
| } | |
| var (responseApi, err) = await SendGptApiRequest(history, uid); | |
| if (err != null || responseApi == null) | |
| { | |
| _sawmill.Warning($"Не удалось получить ответ от GPT для {ToPrettyString(uid)}: {err}"); | |
| uid.Comp.GptProcessing = false; | |
| return; | |
| } | |
| await ProcessGptResponse(uid, history, responseApi); | |
| } | |
| catch (Exception ex) | |
| { | |
| _sawmill.Error($"Исключение при обработке GPT запроса для {ToPrettyString(uid)}: {ex}"); | |
| uid.Comp.GptProcessing = false; | |
| } |
🤖 Prompt for AI Agents
In `@Content.Server/Backmen/NPC/Systems/NPCConversationSystem.cs` around lines 970
- 1006, In TryGptResponse the speaker name is pulled via
MetaData(speaker).EntityName which can bypass masking; change to resolve the
displayed identity using Identity.Name(speaker) before adding to the GPT history
(the GetOrCreateGptHistory/history.Add(new GptMessageChat(...)) call) so
masked/hidden identities are used when creating the GptMessageChat; ensure you
reference the same speaker EntityUid variable and remove direct use of
MetaData(speaker).EntityName.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@Content.Server/Backmen/NPC/Systems/NPCConversationSystem.cs`:
- Around line 713-753: NPCGptHistory currently holds a ReaderWriterLockSlim that
is never disposed and its methods are inconsistent about locking; make
NPCGptHistory implement IDisposable to dispose Lock, change Add to acquire and
release Lock.EnterWriteLock/ExitWriteLock internally, change read-only accessors
(IsCanAnswer, GetMessagesForApi) to use Lock.EnterReadLock/ExitReadLock, keep
the constructor behavior but avoid holding the lock longer than necessary, and
ensure caller code (e.g., OnComponentRemove/_npcGptHistory removal) calls
Dispose on the removed NPCGptHistory (or remove and dispose the instance) so the
ReaderWriterLockSlim is released properly.
- Around line 946-956: Add a depth limit to prevent unbounded recursive
function_call handling: introduce a max depth constant (e.g.,
MAX_FUNCTION_CALL_DEPTH) and a counter stored on the NPC conversation context or
component (e.g., a new property like GptFunctionCallDepth on uid.Comp) or passed
as an explicit parameter through SendGptApiRequest → ProcessGptResponse →
ProcessFunctionCall; increment the counter each time ProcessFunctionCall is
entered and check it before making a recursive SendGptApiRequest call, log and
stop processing (set uid.Comp.GptProcessing = false) if the counter exceeds the
limit to avoid infinite recursion or stack overflows.
♻️ Duplicate comments (3)
Content.Server/Backmen/NPC/Systems/NPCConversationSystem.cs (3)
371-381: Off-by-one ошибка в выборе idle-фраз всё ещё присутствует.Pre-increment
++uid.Comp.IdleChatIndexперед чтением приводит к пропуску первого элементаIdleChatOrder.
853-872: GptProcessing не сбрасывается при пустом content — проблема всё ещё присутствует.При
finish_reasonравном "stop"/"length"/"blacklist" иcontent == null, флагGptProcessingостаётсяtrue, блокируя дальнейшие GPT-ответы для этого NPC.
968-977: MetaData(speaker).EntityName обходит механики маскировки — проблема всё ещё присутствует.Использование
MetaData(speaker).EntityNameраскрывает истинное имя игрока в GPT-истории, игнорируя маски и переодевания. Согласно relevant code snippet, следует использоватьIdentity.Name(speaker, EntityManager, uid).
🧹 Nitpick comments (3)
Content.Server/Backmen/NPC/Systems/NPCConversationSystem.cs (3)
52-81: Потенциальные проблемы с SSL и потокобезопасностью HttpClient.
SSL callback принимает сертификаты с
UntrustedRoot— это намеренное решение для self-signed сертификатов, но следует задокументировать причину.
_gptHttpClient— instance field, ноDefaultRequestHeaders.Authorizationмодифицируется из разных мест (GptTokenCVarChangedна линии 173 иSendGptApiRequestна линии 823). При параллельных запросах возможна гонка данных.Рекомендуется передавать токен через
HttpRequestMessage.HeadersвместоDefaultRequestHeaders:♻️ Предлагаемый рефакторинг
private async Task<(GptResponseApi? responseApi, string? err)> SendGptApiRequest(NPCGptHistory history, EntityUid uid) { // ... var postData = new StringContent(JsonSerializer.Serialize(payload), Encoding.UTF8, "application/json"); postData.Headers.Add("X-Session-ID", uid.ToString()); - if (!string.IsNullOrEmpty(_gptApiToken)) - { - _gptHttpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", _gptApiToken); - } - - var request = await _gptHttpClient.PostAsync(...); + using var request = new HttpRequestMessage(HttpMethod.Post, $"{_gptApiUrl + (_gptApiUrl.EndsWith("/") ? "" : "/")}chat/completions"); + request.Content = postData; + if (!string.IsNullOrEmpty(_gptApiToken)) + { + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", _gptApiToken); + } + var response = await _gptHttpClient.SendAsync(request);
636-651: Fire-and-forget async вызовы могут терять исключения.Дискардинг
_ = TryGptResponse(...)отбрасывает Task. Хотя внутриTryGptResponseесть try-catch, любые исключения в async-продолжениях (например, вProcessGptResponse) могут быть потеряны.Рекомендуется добавить ContinueWith для логирования необработанных исключений:
♻️ Предлагаемый рефакторинг
-_ = TryGptResponse(uid, args.Message, args.Source); +TryGptResponse(uid, args.Message, args.Source).ContinueWith(t => +{ + if (t.Exception != null) + Log.Error($"Unhandled GPT exception: {t.Exception}"); +}, TaskContinuationOptions.OnlyOnFaulted);
770-800: Отсутствует буфер времени при проверке истечения токена GigaChat.Линия 774:
if(_gigaTocExpire > DateTimeOffset.Now)— токен может истечь во время выполнения запроса. Рекомендуется добавить буфер:♻️ Предлагаемый рефакторинг
-if(_gigaTocExpire > DateTimeOffset.Now) +if(_gigaTocExpire > DateTimeOffset.Now.AddSeconds(60)) return;
| public sealed class NPCGptHistory | ||
| { | ||
| public List<GptMessage> Messages { get; } = new(); | ||
| public readonly ReaderWriterLockSlim Lock = new(); | ||
|
|
||
| public NPCGptHistory(string systemPrompt) | ||
| { | ||
| Lock.EnterWriteLock(); | ||
| try | ||
| { | ||
| if (!string.IsNullOrEmpty(systemPrompt)) | ||
| { | ||
| Messages.Add(new GptMessageChat(GptUserDirection.system, systemPrompt)); | ||
| } | ||
| } | ||
| finally | ||
| { | ||
| Lock.ExitWriteLock(); | ||
| } | ||
| } | ||
|
|
||
| public void Add(GptMessage msg) | ||
| { | ||
| Messages.Add(msg); | ||
|
|
||
| if (Messages.Count > 100) | ||
| { | ||
| Messages.RemoveRange(0, Messages.Count - 100); | ||
| } | ||
| } | ||
|
|
||
| public bool IsCanAnswer() | ||
| { | ||
| return Messages.Count > 0 && Messages.Last().Role == GptUserDirection.user; | ||
| } | ||
|
|
||
| public object[] GetMessagesForApi() | ||
| { | ||
| return Messages.Select(x => x.ToApi()).ToArray(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Несогласованность блокировок и утечка ReaderWriterLockSlim.
-
ReaderWriterLockSlimреализуетIDisposable, но никогда не освобождается при удалении истории из_npcGptHistory(линии 356, 361). -
Метод
Add(линия 734) не захватывает блокировку, хотя конструктор (линия 720) делает это сам. Вызывающий код должен сам захватыватьLock.EnterWriteLock()перед вызовомAdd, что несогласованно.
🐛 Предлагаемый фикс для Add
public void Add(GptMessage msg)
{
+ // Caller must hold write lock
+ DebugTools.Assert(Lock.IsWriteLockHeld, "Write lock must be held when calling Add");
Messages.Add(msg);
if (Messages.Count > 100)
{
Messages.RemoveRange(0, Messages.Count - 100);
}
}Также добавьте Dispose при удалении из словаря:
private void OnComponentRemove(EntityUid uid, NPCConversationComponent component, ComponentRemove args)
{
if (_npcGptHistory.Remove(uid, out var history))
history.Lock.Dispose();
}🤖 Prompt for AI Agents
In `@Content.Server/Backmen/NPC/Systems/NPCConversationSystem.cs` around lines 713
- 753, NPCGptHistory currently holds a ReaderWriterLockSlim that is never
disposed and its methods are inconsistent about locking; make NPCGptHistory
implement IDisposable to dispose Lock, change Add to acquire and release
Lock.EnterWriteLock/ExitWriteLock internally, change read-only accessors
(IsCanAnswer, GetMessagesForApi) to use Lock.EnterReadLock/ExitReadLock, keep
the constructor behavior but avoid holding the lock longer than necessary, and
ensure caller code (e.g., OnComponentRemove/_npcGptHistory removal) calls
Dispose on the removed NPCGptHistory (or remove and dispose the instance) so the
ReaderWriterLockSlim is released properly.
| // Продолжаем обработку после вызова функции | ||
| var (responseApi, err) = await SendGptApiRequest(history, uid); | ||
| if (err != null || responseApi == null) | ||
| { | ||
| Log.Warning($"Не удалось получить ответ от GPT после функции для {ToPrettyString(uid)}: {err}"); | ||
| uid.Comp.GptProcessing = false; | ||
| return; | ||
| } | ||
|
|
||
| await ProcessGptResponse(uid, history, responseApi); | ||
| } |
There was a problem hiding this comment.
Отсутствует ограничение глубины рекурсии для function calls.
ProcessFunctionCall рекурсивно вызывает SendGptApiRequest → ProcessGptResponse → ProcessFunctionCall. При некорректном ответе GPT (постоянные function_call) возможна бесконечная рекурсия или глубокий стек.
Рекомендуется добавить счётчик глубины или лимит вызовов функций:
♻️ Предлагаемый рефакторинг
-private async Task ProcessGptResponse(Entity<NPCConversationComponent> uid, NPCGptHistory history, GptResponseApi info)
+private async Task ProcessGptResponse(Entity<NPCConversationComponent> uid, NPCGptHistory history, GptResponseApi info, int depth = 0)
{
+ const int MaxFunctionCallDepth = 5;
+ if (depth >= MaxFunctionCallDepth)
+ {
+ Log.Warning($"GPT function call depth exceeded for {ToPrettyString(uid)}");
+ uid.Comp.GptProcessing = false;
+ return;
+ }
foreach (var gptMsg in info.choices)
{
switch (gptMsg.finish_reason)
{
case "function_call":
- await ProcessFunctionCall(uid, history, gptMsg);
+ await ProcessFunctionCall(uid, history, gptMsg, depth);
break;🤖 Prompt for AI Agents
In `@Content.Server/Backmen/NPC/Systems/NPCConversationSystem.cs` around lines 946
- 956, Add a depth limit to prevent unbounded recursive function_call handling:
introduce a max depth constant (e.g., MAX_FUNCTION_CALL_DEPTH) and a counter
stored on the NPC conversation context or component (e.g., a new property like
GptFunctionCallDepth on uid.Comp) or passed as an explicit parameter through
SendGptApiRequest → ProcessGptResponse → ProcessFunctionCall; increment the
counter each time ProcessFunctionCall is entered and check it before making a
recursive SendGptApiRequest call, log and stop processing (set
uid.Comp.GptProcessing = false) if the counter exceeds the limit to avoid
infinite recursion or stack overflows.
1 participant
@Roudenn
Summary by CodeRabbit
Новые функции
Исправления ошибок
Рефакторинг
✏️ Tip: You can customize this high-level summary in your review settings.