feat: MetaGarbage round-to-round and server-restart persistence#960
Open
rokudara-sen wants to merge 1 commit intospace-sunrise:masterfrom
Open
feat: MetaGarbage round-to-round and server-restart persistence#960rokudara-sen wants to merge 1 commit intospace-sunrise:masterfrom
rokudara-sen wants to merge 1 commit intospace-sunrise:masterfrom
Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 29 minutes and 16 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughДобавлена система сохранения и восстановления данных метамусора между раундами с использованием базы данных и локальных JSON-файлов. Включает новую сущность MetaGarbageEntry, API управления БД, события сохранения/восстановления, сериализацию данных и интеграцию с системой спецкартриджей. Changes
Sequence DiagramsequenceDiagram
participant Map as Инициализация<br/>карты
participant MGSys as MetaGarbage<br/>System
participant Files as Файловая<br/>система
participant DB as База<br/>данных
participant Spawner as Entity<br/>Spawner
Map->>MGSys: OnMapInit()
activate MGSys
MGSys->>MGSys: LoadFromDbAndSpawn()
MGSys->>Files: TryLoadFromFile()
alt Файл существует и версия совпадает
Files-->>MGSys: StationMetaGarbageData[]
else Файл не существует или старая версия
MGSys->>DB: TryLoadFromDb()
DB-->>MGSys: StationMetaGarbageData[]
end
MGSys->>Spawner: TrySpawnGarbage()
Spawner->>Spawner: Для каждого предмета
Spawner->>Spawner: Создать сущность
alt ExtraData присутствуют
Spawner->>Spawner: Raise MetaGarbageRestoreEvent
end
deactivate MGSys
Note over MGSys,Spawner: === При завершении раунда ===
participant Round as Round<br/>End
Round->>MGSys: RealRoundEndedMessage
activate MGSys
MGSys->>MGSys: Собрать кэшированный метамусор
MGSys->>MGSys: Для каждого предмета SaveEntity()
MGSys->>MGSys: Raise MetaGarbageSaveEvent
MGSys->>Files: Сохранить JSON
MGSys->>DB: SaveMetaGarbageAsync()
DB-->>MGSys: Сохранено
deactivate MGSys
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (6 passed)
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c15078589
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Content.Server.Database/Migrations/Postgres/20260417095055_AddMetaGarbage.Designer.cs`:
- Around line 18-20: Create a matching SQLite migration with the same migration
id/class (20260417095055_AddMetaGarbage / AddMetaGarbage) under the Sqlite
migrations set so SQLite servers also get the meta_garbage table: add both the
migration .cs and Designer.cs in Content.Server.Database/Migrations/Sqlite/,
annotate with the appropriate SQLite DbContext (e.g.,
[DbContext(typeof(SqliteServerDbContext))]), and implement Up/Down to
create/drop the meta_garbage table with the same columns, types, indexes and
constraints as in the Postgres AddMetaGarbage migration to ensure parity.
In `@Content.Server.Database/Migrations/Sqlite/20260417095043_AddMetaGarbage.cs`:
- Around line 6-10: The SQLite and PostgreSQL migration timestamps/names are out
of sync: class AddMetaGarbage exists as 20260417095043_AddMetaGarbage in the
SQLite directory but as 20260417095055_AddMetaGarbage in Postgres; rename or
regenerate one of the migration files so both engines have identical
timestamp/name pairs (e.g., change 20260417095043 to 20260417095055 or vice
versa) and ensure the class name AddMetaGarbage inside the file matches the
filename.
In
`@Content.Server.Database/Migrations/Sqlite/20260417095043_AddMetaGarbage.Designer.cs`:
- Line 14: SQLite migration timestamp in the Migration attribute
("20260417095043_AddMetaGarbage") doesn't match the PostgreSQL migration
("20260417095055"); update the timestamps so both migrations share the exact
same timestamp and name. Locate the migration class and Designer files
referencing the Migration attribute (e.g., the attribute on the class like
[Migration("20260417095043_AddMetaGarbage")]) and rename the timestamp to match
the counterpart (or make the counterpart match this one), and also rename the
migration filenames and any class/partial-class names that include the timestamp
to keep them consistent across both Sqlite and Postgres migrations.
In
`@Content.Server.Database/Migrations/Sqlite/SqliteServerDbContextModelSnapshot.cs`:
- Around line 861-879: The model currently allows duplicate MapPrototype values
for the meta_garbage table; update the entity configuration that defines the
MetaGarbage (the property MapPrototype / column "map_prototype") to add a unique
constraint (e.g. configure an index on MapPrototype and call IsUnique() or
define an alternate key) and then regenerate the EF Core migration and snapshot
so the snapshot (SqliteServerDbContextModelSnapshot) contains a unique
index/constraint on MapPrototype for table meta_garbage; ensure the generated
snapshot shows HasIndex("MapPrototype").IsUnique() (or equivalent) so
upserts/read-by-prototype are unambiguous.
In `@Content.Server.Database/Model.cs`:
- Around line 1511-1515: The MetaGarbageEntry.Data property is stored as plain
text but should be mapped to PostgreSQL jsonb (and remain TEXT/BLOB for SQLite);
update OnModelCreating to configure modelBuilder.Entity<MetaGarbageEntry> to set
b.Property(e => e.Data).HasColumnType("jsonb") for Npgsql, or add a
ValueConverter/ValueComparer and conditionally call
.HasColumnType(dbContext.Database.IsNpgsql() ? "jsonb" : null) so the Data
column uses native jsonb in Postgres while remaining compatible with SQLite.
In `@Content.Server/_Scp/MetaGarbage/MetaGarbageCatridgeSystem.cs`:
- Around line 11-28: The save/restore only handle CartridgeAmmoComponent but
must also handle HitScanCartridgeAmmoComponent; add
SubscribeLocalEvent<HitScanCartridgeAmmoComponent, MetaGarbageSaveEvent>(OnSave)
and SubscribeLocalEvent<HitScanCartridgeAmmoComponent,
MetaGarbageRestoreEvent>(OnRestore) in the system initialization and ensure the
existing OnSave and OnRestore handlers (which read/write
args.ExtraData["cartridge.spent"], set ent.Comp.Spent, and call Dirty(ent)) are
used for the hitscan component as well so HitScanCartridgeAmmoComponent.Spent is
persisted and restored correctly.
In `@Content.Server/_Scp/MetaGarbage/MetaGarbageSerializer.cs`:
- Around line 10-16: The MetaGarbageSerializer class and its namespace
(Content.Server._Scp.MetaGarbage) were added under the legacy _Scp folder but
must live in the sunrise-specific project; move the file into the _Sunrise
project folder, change the namespace to a Sunrise-prefixed one (e.g.
Content.Server._Sunrise.MetaGarbage), update any affected using directives and
references to the MetaGarbageSerializer symbol, and surround any unavoidable
upstream-copy edits with the "Sunrise edit start"/"Sunrise edit end" markers per
guidelines.
- Around line 118-122: MetaGarbageFileSave.SavedAt currently uses plain DateTime
which can produce mismatched DateTimeKind against the DB's "timestamp with time
zone"; change SavedAt's type to DateTimeOffset (i.e., public DateTimeOffset
SavedAt { get; set; }) and update all producers/consumers of MetaGarbageFileSave
(constructors, deserialization, places that assign SavedAt) to use
DateTimeOffset.UtcNow or preserved offsets, and adjust any DB
mapping/serialization code that references SavedAt to use offset-aware handling
so the stored value remains UTC/offset-consistent.
In `@Content.Server/_Scp/MetaGarbage/MetaGarbageSystem.cs`:
- Line 78: The static field SaveDirectory is declared in the wrong spot; move
the declaration of SaveDirectory into the static/constants block at the top of
the MetaGarbageSystem class alongside other static/constant fields so it appears
immediately after dependencies and before runtime caches and initialization
code; update any code that references SaveDirectory unchanged (e.g., save/load
logic and event handlers remain the same) and ensure ordering follows the
EntitySystem convention in MetaGarbageSystem.
- Around line 119-168: Currently the code treats the presence of filePath as
proof of success and skips DB lookup; change the logic in the file-load branch
so that reading/parsing the file (File.ReadAllTextAsync +
JsonSerializer.Deserialize<MetaGarbageFileSave>), validating payload != null,
successful MetaGarbageSerializer.Deserialize(payload.Data) and
payload.MapVersion == ent.Comp.MapVersion are all required to mark the load as
successful and populate CachedGarbage[proto]; if any step fails (null payload,
deserialization error, version mismatch, or exception) log the problem and then
call _db.GetMetaGarbageAsync(proto.ID) and apply the same validation (entry !=
null and entry.MapVersion == ent.Comp.MapVersion) before assigning
CachedGarbage[proto]; ensure exceptions from file operations fall through to the
DB fallback and that Log.Warning/Log.Error messages still include proto.ID and
version info.
- Around line 80-106: OnRoundEnded currently iterates CachedGarbage across
rounds and performs filesystem writes and a fire-and-forget DB save which can
throw and overwrite valid saves for absent stations; change OnRoundEnded to
first determine the set of station protos present in the current round (instead
of blindly iterating CachedGarbage), create SaveDirectory once, then for each
current station only: serialize via MetaGarbageSerializer, compute version with
GetMapVersion(stationProto) and write to disk inside a try/catch that logs any
exceptions (wrap Directory.CreateDirectory and File.WriteAllText), and replace
the fire-and-forget _db.SaveMetaGarbageAsync call with an awaited helper method
SaveMetaGarbageBackupAsync(stationProto, json, version, savedAt) that catches
and logs DB exceptions (use the provided pattern to Log.Error on failure).
- Line 109: Change the method signature from async void to async Task for
LoadFromDbAndSpawn(Entity<MetaGarbageTargetComponent> ent, StationDataComponent
stationComp) and ensure any ECS mutations (e.g., calls to TrySpawnGarbage and
other entity/component changes that occur after awaiting
File.ReadAllTextAsync(...) or _db.GetMetaGarbageAsync(...)) are executed on the
main game thread; after each await, marshal continuations back to the main
thread (for example by wrapping ECS calls in EntitySystem.RunOnMainThread(...)
or the Robust.Shared main-thread helper) so TrySpawnGarbage and similar ECS
operations run in the proper context.
In `@Content.Server/Database/ServerDbBase.cs`:
- Around line 2181-2204: The SaveMetaGarbageAsync path can create duplicate
MetaGarbageEntry rows when two callers race on MapPrototype; add a unique
constraint on MapPrototype in the EF model and migrations and handle races: in
OnModelCreating add an index for MetaGarbageEntry via .HasIndex(m =>
m.MapPrototype).IsUnique(), update both Postgres and SQLite migrations to create
a unique index on map_prototype, and make SaveMetaGarbageAsync robust by using
EF Core upsert/update APIs (e.g., ExecuteUpdateAsync/ExecuteUpdate or a single
UPSERT/ON CONFLICT statement) or catching DbUpdateException on insert and
retrying an update so concurrent inserts don’t create duplicates; reference
SaveMetaGarbageAsync, MetaGarbageEntry, MapPrototype, OnModelCreating,
GetMetaGarbageAsync, ExecuteUpdateAsync, and DbUpdateException when applying the
change.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 372a74e4-19a8-452a-a91f-4b6eb1181f0e
📒 Files selected for processing (14)
Content.Server.Database/Migrations/Postgres/20260417095055_AddMetaGarbage.Designer.csContent.Server.Database/Migrations/Postgres/20260417095055_AddMetaGarbage.csContent.Server.Database/Migrations/Postgres/PostgresServerDbContextModelSnapshot.csContent.Server.Database/Migrations/Sqlite/20260417095043_AddMetaGarbage.Designer.csContent.Server.Database/Migrations/Sqlite/20260417095043_AddMetaGarbage.csContent.Server.Database/Migrations/Sqlite/SqliteServerDbContextModelSnapshot.csContent.Server.Database/Model.csContent.Server/Database/ServerDbBase.csContent.Server/Database/ServerDbManager.csContent.Server/_Scp/MetaGarbage/MetaGarbageCatridgeSystem.csContent.Server/_Scp/MetaGarbage/MetaGarbageEvents.csContent.Server/_Scp/MetaGarbage/MetaGarbageSerializer.csContent.Server/_Scp/MetaGarbage/MetaGarbageSystem.csContent.Server/_Scp/MetaGarbage/MetaGarbageTargetComponent.cs
rokudara-sen
force-pushed
the
bugfix/metagarbagesystem
branch
from
6c15078 to
c2c509f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
Content.Server/Database/ServerDbBase.cs (1)
2183-2204:⚠️ Potential issue | 🟡 MinorВозможная гонка при параллельных вызовах
SaveMetaGarbageAsync— теперь приведёт кDbUpdateException.С учётом добавленного уникального индекса на
MapPrototype(см.Model.cs/ миграции), классический паттерн «load-or-insert» может выкинутьDbUpdateExceptionпри одновременных сохранениях для одной и той же карты (например, race round-end + DB backup). Сейчас исключение пробросится и порушит вызывающий код (включая fire-and-forget записи вMetaGarbageSystem).Рассмотрите обёртку в
try/catch (DbUpdateException)с одним повтором (как уже сделано вUpsertIPIntelCacheрядом), либо переписать наExecuteUpdateAsync/INSERT ... ON CONFLICT DO UPDATEдля атомарного upsert.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Server/Database/ServerDbBase.cs` around lines 2183 - 2204, The current load-or-insert in SaveMetaGarbageAsync risks DbUpdateException under concurrent calls due to the unique index on MapPrototype; wrap the add/update + SaveChangesAsync block in a try/catch(DbUpdateException) and retry the operation once (same pattern as UpsertIPIntelCache) so a conflicting concurrent insert is handled, or replace the logic with an atomic upsert (ExecuteUpdateAsync or raw INSERT ... ON CONFLICT DO UPDATE) on the MetaGarbage DbSet; locate SaveMetaGarbageAsync, GetDb, MetaGarbage and MetaGarbageEntry to implement the retry or atomic upsert.Content.Server/_Scp/MetaGarbage/MetaGarbageSystem.cs (2)
135-151:⚠️ Potential issue | 🟠 MajorНе выполняйте ECS-мутации из post-
awaitfire-and-forget continuation.После
await TryLoadFromFile(...)/await TryLoadFromDb(...)код меняетCachedGarbageи вызываетTrySpawnGarbage(), не гарантируя возврат на главный ECS-поток и не перепроверяя, что station entity/components всё ещё валидны. Это может заспавнить мусор после unload/round transition или тронуть ECS с thread pool continuation.Проверьте доступный в кодовой базе main-thread dispatcher / scheduler и используйте его перед
TrySpawnGarbage, плюс повторноResolve/проверьте entity lifetime после асинхронной загрузки:#!/bin/bash set -eu echo 'Async load/spawn call sites:' rg -n --type=cs -C4 '_ = LoadFromDbAndSpawn|private async Task LoadFromDbAndSpawn|TrySpawnGarbage\(' echo echo 'Available main-thread / scheduler helpers:' rg -n --type=cs -C3 'RunOnMainThread|MainThread|IThreadManager|TaskManager|SynchronizationContext|TaskScheduler'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Server/_Scp/MetaGarbage/MetaGarbageSystem.cs` around lines 135 - 151, LoadFromDbAndSpawn performs async I/O (await TryLoadFromFile/TryLoadFromDb) then mutates CachedGarbage and calls TrySpawnGarbage without guaranteeing execution on the main ECS thread or re-checking entity/component validity; change it to capture the result of the load, then schedule the mutation/spawn back onto the main-thread dispatcher/scheduler (use your project's RunOnMainThread/MainThread/TaskManager helper), and once running on the main thread re-resolve the station/entity/components (re-call Prototype(ent) / Resolve the Entity/MetaGarbageTargetComponent and verify the stationComp is still valid) before updating CachedGarbage and calling TrySpawnGarbage to avoid post-await race/unload issues.
85-131:⚠️ Potential issue | 🟠 MajorНе отключайте DB backup при ошибке локального каталога.
Сейчас
Directory.CreateDirectory(SaveDirectory)при ошибке делаетreturn, поэтому рабочая БД не получит backup вообще. Файловый путь должен деградировать независимо от DB-сохранения.Направление исправления
- try + var canWriteFiles = true; + try { Directory.CreateDirectory(SaveDirectory); } catch (Exception e) { Log.Error($"[MetaGarbage] Failed to create save directory: {e}"); - return; + canWriteFiles = false; } @@ - try + if (canWriteFiles) { - File.WriteAllText(path, payload); - Log.Info($"[MetaGarbage] Saved {dataList.Count} items to file for {stationProto.ID}"); - } - catch (Exception e) - { - Log.Error($"[MetaGarbage] Failed to write save file for {stationProto.ID}: {e}"); + try + { + File.WriteAllText(path, payload); + Log.Info($"[MetaGarbage] Saved {dataList.Count} items to file for {stationProto.ID}"); + } + catch (Exception e) + { + Log.Error($"[MetaGarbage] Failed to write save file for {stationProto.ID}: {e}"); + } } // Fire-and-forget DB backup with explicit fault logging🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Server/_Scp/MetaGarbage/MetaGarbageSystem.cs` around lines 85 - 131, The create-directory error currently returns early (Directory.CreateDirectory(SaveDirectory) catch { Log.Error(...); return; }) which prevents any DB backups; change the control flow so a failed local directory creation is logged but does not return—track a boolean (e.g., localSaveAvailable) set to true only if the directory exists or CreateDirectory succeeded, then later only attempt File.WriteAllText when localSaveAvailable (or Directory.Exists(SaveDirectory)), but always invoke _db.SaveMetaGarbageAsync(...).ContinueWith(...) for each station so DB backups run even if local file saves are disabled; update the try/catch and surrounding logic in MetaGarbageSystem (the Directory.CreateDirectory block and the per-station save loop using File.WriteAllText and _db.SaveMetaGarbageAsync) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Content.Server/_Scp/MetaGarbage/MetaGarbageCartridgeSystem.cs`:
- Line 20: The code currently uses duplicated string literals "cartridge.spent"
and "hitscan_cartridge.spent" when writing/reading args.ExtraData (e.g., the
line args.ExtraData["cartridge.spent"] =
JsonSerializer.SerializeToElement(ent.Comp.Spent); in
MetaGarbageCartridgeSystem), so declare private const string fields (e.g.,
private const string CartridgeSpentKey and private const string
HitscanCartridgeSpentKey) inside the MetaGarbageCartridgeSystem class and
replace all occurrences of those literals in both save and restore code paths
with the new constants to eliminate duplication and reduce typo risk.
- Around line 23-31: OnCartridgeRestore currently only handles
JsonValueKind.True, causing asymmetric restore; update OnCartridgeRestore (and
the similar block at lines 38-46) to explicitly read a boolean (use
args.ExtraData.TryGetBoolean or GetBoolean/TryGetValue + check
JsonValueKind.False) and assign ent.Comp.Spent = <that boolean>, then call
Dirty(ent) when you change the component so the serialized value is symmetric
with how you write it.
In `@Content.Server/_Scp/MetaGarbage/MetaGarbageSerializer.cs`:
- Around line 49-57: To fix loss of ReagentId.Data in MetaGarbage serialization,
update the ToDto() (around the MetaGarbageSerializer block building
MetaGarbageSolutionDto/MetaGarbageReagentDto) to include a stable DTO
representation of ReagentId.Data (e.g., add a Data field on
MetaGarbageReagentDto and serialize the relevant aspects of ReagentData) and
update FromDto() (the code that recreates new ReagentId(..., null) around lines
70–74) to reconstruct ReagentId with that deserialized Data; alternatively, if
you choose not to support serializing custom reagent data, modify ToDto() to
explicitly filter out/skip solutions whose reagents have non-null ReagentId.Data
and log or mark them so they aren’t silently zeroed—ensure changes touch
MetaGarbageSolutionDto/MetaGarbageReagentDto and the ToDto()/FromDto() paths to
preserve or explicitly exclude ReagentId.Data.
In `@Content.Server/Database/ServerDbBase.cs`:
- Around line 2181-2205: Add a CancellationToken parameter to
SaveMetaGarbageAsync (e.g., CancellationToken cancel = default) to match
GetMetaGarbageAsync and other ServerDbBase methods, then pass that token into
GetDb(cancel), the EF call FirstOrDefaultAsync(e => e.MapPrototype ==
mapPrototype, cancel) and SaveChangesAsync(cancel) so the operation can be
cancelled; update the method signature and all internal async calls in
SaveMetaGarbageAsync accordingly.
---
Duplicate comments:
In `@Content.Server/_Scp/MetaGarbage/MetaGarbageSystem.cs`:
- Around line 135-151: LoadFromDbAndSpawn performs async I/O (await
TryLoadFromFile/TryLoadFromDb) then mutates CachedGarbage and calls
TrySpawnGarbage without guaranteeing execution on the main ECS thread or
re-checking entity/component validity; change it to capture the result of the
load, then schedule the mutation/spawn back onto the main-thread
dispatcher/scheduler (use your project's RunOnMainThread/MainThread/TaskManager
helper), and once running on the main thread re-resolve the
station/entity/components (re-call Prototype(ent) / Resolve the
Entity/MetaGarbageTargetComponent and verify the stationComp is still valid)
before updating CachedGarbage and calling TrySpawnGarbage to avoid post-await
race/unload issues.
- Around line 85-131: The create-directory error currently returns early
(Directory.CreateDirectory(SaveDirectory) catch { Log.Error(...); return; })
which prevents any DB backups; change the control flow so a failed local
directory creation is logged but does not return—track a boolean (e.g.,
localSaveAvailable) set to true only if the directory exists or CreateDirectory
succeeded, then later only attempt File.WriteAllText when localSaveAvailable (or
Directory.Exists(SaveDirectory)), but always invoke
_db.SaveMetaGarbageAsync(...).ContinueWith(...) for each station so DB backups
run even if local file saves are disabled; update the try/catch and surrounding
logic in MetaGarbageSystem (the Directory.CreateDirectory block and the
per-station save loop using File.WriteAllText and _db.SaveMetaGarbageAsync)
accordingly.
In `@Content.Server/Database/ServerDbBase.cs`:
- Around line 2183-2204: The current load-or-insert in SaveMetaGarbageAsync
risks DbUpdateException under concurrent calls due to the unique index on
MapPrototype; wrap the add/update + SaveChangesAsync block in a
try/catch(DbUpdateException) and retry the operation once (same pattern as
UpsertIPIntelCache) so a conflicting concurrent insert is handled, or replace
the logic with an atomic upsert (ExecuteUpdateAsync or raw INSERT ... ON
CONFLICT DO UPDATE) on the MetaGarbage DbSet; locate SaveMetaGarbageAsync,
GetDb, MetaGarbage and MetaGarbageEntry to implement the retry or atomic upsert.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a97e29c6-3886-4eb5-b2dc-d72d0bf5732d
📒 Files selected for processing (14)
Content.Server.Database/Migrations/Postgres/20260418115645_AddMetaGarbage.Designer.csContent.Server.Database/Migrations/Postgres/20260418115645_AddMetaGarbage.csContent.Server.Database/Migrations/Postgres/PostgresServerDbContextModelSnapshot.csContent.Server.Database/Migrations/Sqlite/20260418115645_AddMetaGarbage.Designer.csContent.Server.Database/Migrations/Sqlite/20260418115645_AddMetaGarbage.csContent.Server.Database/Migrations/Sqlite/SqliteServerDbContextModelSnapshot.csContent.Server.Database/Model.csContent.Server/Database/ServerDbBase.csContent.Server/Database/ServerDbManager.csContent.Server/_Scp/MetaGarbage/MetaGarbageCartridgeSystem.csContent.Server/_Scp/MetaGarbage/MetaGarbageEvents.csContent.Server/_Scp/MetaGarbage/MetaGarbageSerializer.csContent.Server/_Scp/MetaGarbage/MetaGarbageSystem.csContent.Server/_Scp/MetaGarbage/MetaGarbageTargetComponent.cs
rokudara-sen
force-pushed
the
bugfix/metagarbagesystem
branch
from
c2c509f to
27f5650
Compare
ThereDrD0
reviewed
Apr 19, 2026
Comment on lines
+53
to
+55
| // Sunrise-Start | ||
| public DbSet<MetaGarbageEntry> MetaGarbage { get; set; } = null!; | ||
| // Sunrise-End |
Collaborator
There was a problem hiding this comment.
We use // Fire edit markers for edits inside vanilla content because we are "SCP: Project Fire".
Sunrise is a network of servers with a shared base repository sunrise-station for vanilla content.
Coderabbit tells you to use a sunrise varianty because it sees repository as space-sunrise/project-fire probably. This is a bug inside config.
TLDR: Use // Fire edit/added markerks instead of "sunrise" ones
Comment on lines
+29
to
+36
| public static List<StationMetaGarbageData> Deserialize(string json) | ||
| { | ||
| var dtos = JsonSerializer.Deserialize<List<MetaGarbageDataDto>>(json, Options); | ||
| if (dtos == null) | ||
| return []; | ||
|
|
||
| return dtos.Select(FromDto).ToList(); | ||
| } |
Collaborator
There was a problem hiding this comment.
It isn't related to this method, but you should create a mechanism to deal with non-existing prototypes and container names.
Some prototypes can be removed from codebase, containers can be renamed. Stale data should not be stored inside the database for years.
Basically they should be marked as stale and not be saved after that.
Also we need a command to clear all stored metagarbage for a specific station linked to Server admin flag.
Comment on lines
74
to
+78
| private void OnMapInit(Entity<MetaGarbageTargetComponent> ent, ref StationPostInitEvent args) | ||
| { | ||
| if (!_enableSpawningWithoutRule) | ||
| var stationComp = args.Station.Comp; | ||
| _ = LoadFromDbAndSpawn(ent, stationComp); | ||
| } |
Collaborator
There was a problem hiding this comment.
Return the _enableSpawningWithoutRule check
| } | ||
| catch (Exception e) | ||
| { | ||
| Log.Error($"[MetaGarbage] Failed to create save directory: {e}"); |
Collaborator
There was a problem hiding this comment.
Suggested change
| Log.Error($"[MetaGarbage] Failed to create save directory: {e}"); | |
| Log.Error($"Failed to create save directory: {e}"); |
Log already includes the system name in the log message so there is no need to add it manually
Comment on lines
+95
to
+98
| // Iterate over stations that actually exist this round. | ||
| // Iterating CachedGarbage could include stale entries from previous rounds, | ||
| // and skipping CachedGarbage for empty stations would leave stale files on disk. | ||
| var query = EntityQueryEnumerator<MetaGarbageTargetComponent, StationDataComponent>(); |
Collaborator
There was a problem hiding this comment.
Wouldn't it be better to remove cached garbage dictionary? It stores a large amount of data in memory only for spawning objects on station init.
Of course, even if you haven't removed its functionality already.
|
|
||
| var dataList = CachedGarbage.GetValueOrDefault(stationProto) ?? []; | ||
| var json = MetaGarbageSerializer.Serialize(dataList); | ||
| var version = comp.MapVersion; |
Collaborator
There was a problem hiding this comment.
Storing map version inside a component and expecting that mappers will increment it manually is a bad idea.
It would be better to parse the map file and extract the map metadata
Comment on lines
+168
to
+173
| if (payload.MapVersion != currentMapVersion) | ||
| { | ||
| Log.Warning($"[MetaGarbage] Map version mismatch for {proto.ID} (file): " + | ||
| $"saved={payload.MapVersion} current={currentMapVersion}. Trying DB fallback."); | ||
| return false; | ||
| } |
Collaborator
There was a problem hiding this comment.
Shouldn't it clear stale data from the DB on mismatch?
|
|
||
| if (entry.MapVersion != currentMapVersion) | ||
| { | ||
| Log.Warning($"[MetaGarbage] Map version mismatch for {proto.ID} (DB): " + |
Collaborator
There was a problem hiding this comment.
Suggested change
| Log.Warning($"[MetaGarbage] Map version mismatch for {proto.ID} (DB): " + | |
| Log.Info($"[MetaGarbage] Map version mismatch for {proto.ID} (DB): " + |
It don't think it deserves to be a warning
Comment on lines
+307
to
+309
| // Use EntityPrototype from metadata instead of Prototype() to get the exact | ||
| // spawned prototype ID rather than a resolved parent prototype | ||
| var proto = MetaData(uid).EntityPrototype; |
Comment on lines
+2177
to
+2178
| return await db.DbContext.MetaGarbage | ||
| .FirstOrDefaultAsync(e => e.MapPrototype == mapPrototype, cancel); |
Collaborator
There was a problem hiding this comment.
Wouldn't it be better to add AsNoTracking() to prevent storing all metagarbage data in the db context when it's only used for reading/spawning?
Or the
return await db.DbContext.MetaGarbage
.AsNoTracking()
.Where(e => e.MapPrototype == mapPrototype)
.Select(e => SOME_DTO)
.FirstOrDefaultAsync(cancel);
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
2 participants
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.
Short description
Fixes shell casings spawning as bullets and adds round-to-round and server-restart persistence for the MetaGarbage system. Addresses all three points from the original proposal: serialization, DB persistence, and map-layout gating
CollectGarbage,IsItemAlreadySpawned, andIsSameItemnow useMetaData(uid).EntityPrototypeinstead ofPrototype(uid)to resolve the concrete prototype ID rather than the inherited parent, which fixes the shell casing regression. AddedMetaGarbagePreventSavingtag toBaseCartridgeas a safety netReplaced the hardcoded in-memory approach with proper JSON serialization via
MetaGarbageSerializerand DTOs (MetaGarbageDataDto,MetaGarbageSolutionDto,MetaGarbageReagentDto)On round end, garbage is written synchronously with a fire-and-forget DB write as backup. On round start, the file is preferred over the DB. This survives both round transitions and full server restarts. Added
MetaGarbageEntrytable with corresponding migrations for SQLite and PostgresMap-layout gating uses a manual
MapVersionint onMetaGarbageTargetComponent: if the saved version doesn't match, the save is discarded with a warning. Mappers bump this number after significant layout edits to invalidate old saves and prevent trash spawning in walls. A fully automatic grid hash was considered but deferredMoved
MetaGarbageSaveEvent/MetaGarbageRestoreEventtoContent.Serverto avoidJsonElementsandbox violations inContent.Shared. Spawn after async load is now gated onCachedGarbage.ContainsKey(proto)instead of the rule-enabled flag, so loaded data always spawns regardless of game rule state.Related Issue/Bug Report
N/A
Media
Testing
Changelog
🆑 rokudara
Summary by CodeRabbit
Примечания к выпуску