Conversation
github-actions
bot
added
S: Untriaged
size/M
Changes: Map
WalkthroughИзменения интегрируют систему крыш в игровую логику, обновляют версии двигателя на картах Лавленда (270.1.0 → 271.1.0), оптимизируют обработку воспламеняемых компонентов через пакетную обработку и ослабляют проверку API GPT, удаляя требование токена. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Content.Server/Backmen/NPC/Systems/NPCConversationSystem.cs (1)
806-824: ОчищайтеAuthorization, когда токен пустой.Теперь допускается работа без токена, но заголовок может остаться от предыдущего значения (или стать
Bearer), из‑за чего неаутентифицированные эндпоинты будут отклонять запросы. Сбрасывайте заголовок при пустом токене.🐛 Предлагаемое исправление
- if (!string.IsNullOrEmpty(_gptApiToken)) - { - _gptHttpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", _gptApiToken); - } + if (!string.IsNullOrEmpty(_gptApiToken)) + { + _gptHttpClient.DefaultRequestHeaders.Authorization = + new AuthenticationHeaderValue("Bearer", _gptApiToken); + } + else + { + _gptHttpClient.DefaultRequestHeaders.Authorization = null; + }
🤖 Fix all issues with AI agents
In `@Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.Grid.cs`:
- Around line 164-168: The call to _map.SetTile(spawned.Value, spawnedGrid,
offsetTile, ...) is using source-grid coordinates (offsetTile) instead of the
spawned target grid coordinates; update the SetTile invocation in the block that
checks _map.TryGetTileRef(...) / lavalandTile.Tile to pass
tileRef.Value.GridIndices (the original spawned-grid indices) instead of
offsetTile so tiles are placed on the correct grid (note SetRoof correctly uses
offsetTile for the source grid and should remain unchanged).
In `@Content.Server/_Lavaland/Weather/LavalandWeatherSystem.cs`:
- Around line 64-71: The code calls Comp<MapGridComponent>(lavaland.Owner) twice
and uses Comp<RoofComponent>(lavaland.Owner) which can throw; cache the
MapGridComponent result in a local variable and replace the second Comp call
with that variable, and change the roof lookup to
TryComp<RoofComponent>(lavaland.Owner, out var roofComp) and only call
_roof.IsRooved when TryComp returns true (pass roofComp instead of
Comp<RoofComponent>(lavaland.Owner)); keep the existing
_mapSystem.TryGetTileRef(lavaland.Owner, ...) and use tile.GridIndices as
before.
In `@Content.Server/Atmos/EntitySystems/FlammableSystem.cs`:
- Around line 429-438: The Update() loop currently allocates a new Queue and
requests an unused TransformComponent; make a class-level Queue<(EntityUid,
FlammableComponent)> (e.g. _flammableQueue) and reuse it by calling
_flammableQueue.Clear() at the start of Update(), and change the query from
EntityQueryEnumerator<FlammableComponent, TransformComponent>() to
EntityQueryEnumerator<FlammableComponent>() (remove the unused
TransformComponent out _), then either enqueue items into the reused
_flammableQueue or—since components aren’t added/removed—process the flammable
entries inline in Update() to eliminate the two-phase allocation entirely;
update references to the local q variable to use the field name (e.g.
_flammableQueue) and keep all logic that modifies
FlammableComponent/IgnitionSourceComponent the same.
🧹 Nitpick comments (4)
Content.Server/_Lavaland/Weather/LavalandWeatherSystem.cs (1)
32-35: Неиспользуемые зависимости.
_mapManagerи_transformобъявлены, но не используются в этом файле. Если они не планируются к использованию, их следует удалить.♻️ Предлагаемое исправление
[Dependency] private readonly RoofSystem _roof = default!; - [Dependency] private readonly IMapManager _mapManager = default!; - [Dependency] private readonly SharedTransformSystem _transform = default!; [Dependency] private readonly SharedMapSystem _mapSystem = default!;Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.cs (2)
161-168: Захардкоженные значения газовой смеси.Значения кислорода (21.824779f) и азота (82.10312f) дублируют константы, которые также используются в других местах (например, в карте на строке 131-132 используется 21.824879 и 82.10312). Небольшое расхождение в значении кислорода (21.824779 vs 21.824879) может быть намеренным или ошибкой.
Рекомендуется вынести эти значения в константы или использовать общий источник.
222-223: Закомментированный код.Закомментированный вызов
_atmos.RebuildGridTilesследует либо удалить, либо добавить TODO-комментарий с пояснением, почему он отключен и когда планируется вернуть.Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.Grid.cs (1)
155-159: Дублирующийся вызов SetRoof.
_roofSystem.SetRoof(roofMap, offsetTile, true)вызывается дважды для одних и тех же координат: внутри цикла (строка 157) и после merge (строка 176). Если это сделано намеренно для сохранения состояния крыш после операции merge, добавьте комментарий. В противном случае, первый вызов можно удалить.♻️ Предлагаемое исправление (если дублирование не требуется)
if (_roofSystem.IsRooved(spawnedRoof, tileRef.Value.GridIndices)) { - _roofSystem.SetRoof(roofMap, offsetTile, true); tilesToRoof.Add(offsetTile); }Also applies to: 174-177
| if (_map.TryGetTileRef(sourceGridUid, sourceGrid, offsetTile, out var lavalandTile) && | ||
| !lavalandTile.Tile.IsEmpty) | ||
| { | ||
| _map.SetTile(spawned.Value, spawnedGrid, tile.GridIndices, lavalandTile.Tile); | ||
| _map.SetTile(spawned.Value, spawnedGrid, offsetTile, lavalandTile.Tile); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the file and examine the full context around the flagged lines
fd "LavalandPlanetSystem" --type f -x head -n 200 {} \; 2>/dev/null | head -50Repository: Rxup/space-station-14
Length of output: 1823
🏁 Script executed:
# Search for SetTile usage patterns with spawned
rg -n -B2 -A3 "\.SetTile\(" --type=cs -g "*LavalandPlanetSystem*"Repository: Rxup/space-station-14
Length of output: 799
🏁 Script executed:
# Check Grid.Merge patterns to understand coordinate conventions
rg -n -B2 -A2 "Grid\.Merge|_gridFixture\.Merge" --type=cs | head -40Repository: Rxup/space-station-14
Length of output: 1150
🏁 Script executed:
# Find the matrix transformation context to understand source→spawned mapping
rg -n -B5 "Matrix3x2.*transform|offsetTile.*=" --type=cs -g "*LavalandPlanetSystem*" | head -60Repository: Rxup/space-station-14
Length of output: 915
🏁 Script executed:
# Get the full context of the loop around lines 145-175
sed -n '130,180p' "Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.Grid.cs"Repository: Rxup/space-station-14
Length of output: 2734
🏁 Script executed:
# Also check what the matrix represents - look for where it's created
rg -n -B10 "var matrix" --type=cs -g "*LavalandPlanetSystem.Grid.cs*"Repository: Rxup/space-station-14
Length of output: 1634
🏁 Script executed:
# Double-check by looking at SetTile signature and how Merge uses the matrix
rg -n "public.*SetTile" --type=cs | head -5Repository: Rxup/space-station-14
Length of output: 47
🏁 Script executed:
# Verify the matrix direction - check if it's meant to be spawned->source or source->spawned
rg -n "CreateTransform" --type=cs | head -10Repository: Rxup/space-station-14
Length of output: 1666
🏁 Script executed:
# Get the actual implementation of SetTile to confirm it takes Vector2i as coordinates
rg -n -A3 "void SetTile.*Vector2i" --type=cs | head -20Repository: Rxup/space-station-14
Length of output: 47
🏁 Script executed:
# Check if there are similar patterns elsewhere in the codebase that might validate the coordinate handling
rg -n -B5 -A5 "GetAllTilesEnumerator.*while.*tileRef" --type=cs | head -60Repository: Rxup/space-station-14
Length of output: 47
🏁 Script executed:
# Search for SetTile method signature
rg -n "SetTile" --type=cs -A2 | grep -E "(public|void|SetTile)" | head -20Repository: Rxup/space-station-14
Length of output: 2864
🏁 Script executed:
# Look at the actual SharedMapSystem to understand SetTile
fd "SharedMapSystem" --type=f -x cat {} \; | grep -A5 "SetTile" | head -30Repository: Rxup/space-station-14
Length of output: 47
🏁 Script executed:
# Let me check the loop logic more carefully - maybe offsetTile should actually be inverted before SetTile
rg -n -B20 "SetTile.*spawned" --type=cs -g "*LavalandPlanetSystem*" | tail -40Repository: Rxup/space-station-14
Length of output: 2911
Использованы неправильные координаты для SetTile на spawned grid.
В строке 167 _map.SetTile вызывается на spawned.Value с координатами offsetTile, которые представляют позицию в системе координат source grid (результат матричного преобразования). Однако SetTile должен получать координаты в системе координат целевого grid — spawned.Value.
Следует использовать tileRef.Value.GridIndices (оригинальные координаты spawned grid) вместо offsetTile:
_map.SetTile(spawned.Value, spawnedGrid, tileRef.Value.GridIndices, lavalandTile.Tile);Текущая реализация приведёт к тому, что плитки будут установлены на неправильные координаты. Примечание: SetRoof на строке 157 корректно использует offsetTile, так как он устанавливает крышу на source grid.
🤖 Prompt for AI Agents
In `@Content.Server/_Lavaland/Procedural/Systems/LavalandPlanetSystem.Grid.cs`
around lines 164 - 168, The call to _map.SetTile(spawned.Value, spawnedGrid,
offsetTile, ...) is using source-grid coordinates (offsetTile) instead of the
spawned target grid coordinates; update the SetTile invocation in the block that
checks _map.TryGetTileRef(...) / lavalandTile.Tile to pass
tileRef.Value.GridIndices (the original spawned-grid indices) instead of
offsetTile so tiles are placed on the correct grid (note SetRoof correctly uses
offsetTile for the source grid and should remain unchanged).
| if ( | ||
| _mapSystem.TryGetTileRef(lavaland.Owner, Comp<MapGridComponent>(lavaland.Owner), xform.Coordinates, out var tile) && | ||
| _roof.IsRooved( | ||
| (lavaland.Owner, Comp<MapGridComponent>(lavaland.Owner), Comp<RoofComponent>(lavaland.Owner)), | ||
| tile.GridIndices)) | ||
| { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Дублирующийся вызов Comp и потенциальный NullReferenceException.
Comp<MapGridComponent>(lavaland.Owner)вызывается дважды — следует закэшировать результат.Comp<RoofComponent>(lavaland.Owner)выбросит исключение, если компонент отсутствует. ИспользуйтеTryCompдля безопасной проверки.
🐛 Предлагаемое исправление
- if (
- _mapSystem.TryGetTileRef(lavaland.Owner, Comp<MapGridComponent>(lavaland.Owner), xform.Coordinates, out var tile) &&
- _roof.IsRooved(
- (lavaland.Owner, Comp<MapGridComponent>(lavaland.Owner), Comp<RoofComponent>(lavaland.Owner)),
- tile.GridIndices))
- {
- return;
- }
+ if (!TryComp<MapGridComponent>(lavaland.Owner, out var gridComp) ||
+ !TryComp<RoofComponent>(lavaland.Owner, out var roofComp))
+ return;
+
+ if (_mapSystem.TryGetTileRef(lavaland.Owner, gridComp, xform.Coordinates, out var tile) &&
+ _roof.IsRooved((lavaland.Owner, gridComp, roofComp), tile.GridIndices))
+ {
+ return;
+ }🤖 Prompt for AI Agents
In `@Content.Server/_Lavaland/Weather/LavalandWeatherSystem.cs` around lines 64 -
71, The code calls Comp<MapGridComponent>(lavaland.Owner) twice and uses
Comp<RoofComponent>(lavaland.Owner) which can throw; cache the MapGridComponent
result in a local variable and replace the second Comp call with that variable,
and change the roof lookup to TryComp<RoofComponent>(lavaland.Owner, out var
roofComp) and only call _roof.IsRooved when TryComp returns true (pass roofComp
instead of Comp<RoofComponent>(lavaland.Owner)); keep the existing
_mapSystem.TryGetTileRef(lavaland.Owner, ...) and use tile.GridIndices as
before.
| var q = new Queue<(EntityUid, FlammableComponent)>(); | ||
| var query = EntityQueryEnumerator<FlammableComponent, TransformComponent>(); | ||
| while (query.MoveNext(out var uid, out var flammable, out _)) | ||
| { | ||
| q.Enqueue((uid, flammable)); | ||
| } | ||
|
|
||
| while (q.TryDequeue(out var d)) | ||
| { | ||
| var (uid, flammable) = d; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Examine the full Update method context around lines 429-438
sed -n '400,470p' Content.Server/Atmos/EntitySystems/FlammableSystem.cs | cat -nRepository: Rxup/space-station-14
Length of output: 3274
🏁 Script executed:
# Search for FlammableComponent additions/removals within the file
rg -n "AddComp.*Flammable|RemComp.*Flammable|EnsureComp.*Flammable" Content.Server/Atmos/EntitySystems/FlammableSystem.csRepository: Rxup/space-station-14
Length of output: 47
🏁 Script executed:
# Check the entire Update method to understand what operations happen in the processing loop
rg -A 150 "public void Update\(" Content.Server/Atmos/EntitySystems/FlammableSystem.cs | head -200Repository: Rxup/space-station-14
Length of output: 47
🏁 Script executed:
# Look for uses of TransformComponent in FlammableSystem.cs
rg -n "TransformComponent" Content.Server/Atmos/EntitySystems/FlammableSystem.csRepository: Rxup/space-station-14
Length of output: 157
Ненужная аллокация Queue и неиспользуемый запрос TransformComponent
Метод Update() выполняется каждый UpdateTime интервал, и на каждой итерации создается новая Queue. Это вызывает давление на сборщик мусора. Очередь следует переиспользовать как поле класса через Clear().
Также TransformComponent запрашивается в query (строка 431), но никогда не используется (out _).
Анализ показывает, что двухфазная обработка, похоже, не требуется, так как операции внутри цикла не добавляют/удаляют FlammableComponent — они лишь изменяют состояние компонента и добавляют другие типы компонентов (IgnitionSourceComponent).
🔧 Предлагаемое исправление
Добавьте поле класса:
private readonly Dictionary<Entity<FlammableComponent>, float> _fireEvents = new();
+private readonly Queue<(EntityUid, FlammableComponent)> _flammableQueue = new();Затем обновите метод Update:
-var q = new Queue<(EntityUid, FlammableComponent)>();
-var query = EntityQueryEnumerator<FlammableComponent, TransformComponent>();
+_flammableQueue.Clear();
+var query = EntityQueryEnumerator<FlammableComponent>();
while (query.MoveNext(out var uid, out var flammable, out _))
{
- q.Enqueue((uid, flammable));
+ _flammableQueue.Enqueue((uid, flammable));
}
-while (q.TryDequeue(out var d))
+while (_flammableQueue.TryDequeue(out var d))
{
var (uid, flammable) = d;🤖 Prompt for AI Agents
In `@Content.Server/Atmos/EntitySystems/FlammableSystem.cs` around lines 429 -
438, The Update() loop currently allocates a new Queue and requests an unused
TransformComponent; make a class-level Queue<(EntityUid, FlammableComponent)>
(e.g. _flammableQueue) and reuse it by calling _flammableQueue.Clear() at the
start of Update(), and change the query from
EntityQueryEnumerator<FlammableComponent, TransformComponent>() to
EntityQueryEnumerator<FlammableComponent>() (remove the unused
TransformComponent out _), then either enqueue items into the reused
_flammableQueue or—since components aren’t added/removed—process the flammable
entries inline in Update() to eliminate the two-phase allocation entirely;
update references to the local q variable to use the field name (e.g.
_flammableQueue) and keep all logic that modifies
FlammableComponent/IgnitionSourceComponent the same.
1 participant
Summary by CodeRabbit
Примечания к выпуску
Новые функции
Улучшения
✏️ Tip: You can customize this high-level summary in your review settings.