Route every log line through one topic-tagged logger - #24
Conversation
![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: f7400d1833
ℹ️ 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".
| _log.Event(LogTopic.Session, "Join requested: transport=" + config.Transport + | ||
| " target=" + | ||
| (config.Transport == TransportMode.SteamRelay ? config.JoinCode : config.HostAddress + ":" + config.Port) + |
There was a problem hiding this comment.
When a player joins through either transport, this ungated Event includes the direct host address or Steam join code, and SyncLog.Emit mirrors it into CS2MP-flight.log. That file is explicitly intended for public support posts, and the nearby status logger deliberately replaces the same target with target=redacted; LogPaths.Redact only handles filesystem paths. Sharing a flight log can therefore disclose a host's network address or a still-valid relay code, so retain the target only in the private game log or redact it before this event is mirrored.
Useful? React with 👍 / 👎.
| if (phase != ClientWorldPhase.LoadingMap) _sawLoading = false; | ||
| _log.Info("[MP] World phase: " + phase); | ||
| Diagnostics.FlightRecorder.Note("phase " + phase); | ||
| _log.Detail(LogTopic.Session, "World phase: " + phase); |
There was a problem hiding this comment.
With the default logging switches off, Detail returns without writing anywhere, whereas this transition previously had an unconditional FlightRecorder.Note("phase " + phase). If joining hangs or the process crashes during map handover, the support flight log now omits the last client phase—the exact breadcrumb needed to distinguish waiting, loading, and resume-barrier failures. Emit this as a Trace so it remains in the flight log while staying gated in the readable game log.
Useful? React with 👍 / 👎.
f7400d1 to
e16e037
Compare
e16e037 to
5d3008f
Compare
The mod had two logging front doors writing two files that disagreed. 243 Mod.log calls and 97 Mod.Verbose calls went to the game log; 174 separate FlightRecorder.Note calls went to the flight log; SyncLog, added for exactly this purpose, had no callers at all. Roughly 65 places wrote the same fact twice, once as prose and once as key=value, so neither file was complete and the pair had to be read together. Everything now goes through SyncLog, and both files are written from that one path. Four tiers, chosen by what the line is rather than by who wrote it: Detail troubleshooting chatter - both logs, only while its topic is on Trace a compact breadcrumb - flight log always, game log when on Event a milestone - both logs, always Warn something went wrong - both logs, always, flushed Error the mod could not cope - both logs, always, flushed, with the stack Severity, not the settings, decides whether a switch is consulted. Connects, disconnects, world transfers, resyncs, dropped commands, quarantined batches and every fault are written with every switch off, because nobody turns a switch on before the crash they did not know was coming. Gating is per feature, not one "verbose" flag. LogTopic names eighteen of them - session, transport, world transfer, resync, pipeline, nets, buildings, land, city, routes, the four zone economies, players, UI, startup and performance - each with its own switch on a new Logging options tab, plus a Log Everything master for "I do not know which one". The enum lives in Core/Diagnostics so the portable networking and session code names the same topics; IModLogger mirrors the same shape and ColossalModLogger is the seam. Call sites no longer write prefixes. The "[MP] ", "[MP][OCC-DEV] ", "[security] ", "[upnp] ", "[relay] " and "[compatibility] " spellings are gone; the logger attaches one [topic] tag so every line is greppable and no two subsystems can drift. Why two files still, and what the difference is: the game log is the readable one. The flight log is the same content made durable - it is flushed per fault so a hard exit keeps its tail, it is not truncated when the game restarts, it also captures Unity and other mods' exceptions, and it is machine-readable. Nothing reaches the game log without also reaching it, so "send us CS2MP-flight.log" is now a complete answer. Detail and trace lines are buffered rather than flushed, and the next fault commits them, so turning a topic on no longer puts a disk write in every frame. Also: thirty "X ready." lines replaced by one startup line carrying the mod, protocol and game versions; the 30-second performance report demoted out of every player's log; expensive diagnostic probes now follow their own feature switch instead of the master one; resync reports log their full summary rather than just the reason.
5d3008f to
c620f29
Compare
Route every log line through one topic-tagged logger
1 participant
Stacked on #22 → #21 → #20. Merge those first; this targets
sync-integrationso the diff shows only the logging work.The problem
SyncLogwas added for exactly this purpose and had zero callers. Meanwhile:Mod.log.*calls and 97Mod.Verbosecalls went to the game logFlightRecorder.Notecalls went to the flight logkey=valueSo neither file was complete on its own, and the two had to be read side by side to reconstruct what happened.
One log, two files
Everything now goes through
SyncLog, and both files are written from that single path. The tier is chosen by what the line is, not by who wrote it:Detail— troubleshooting chatterTrace— compact breadcrumbEvent— a milestoneWarn/Error— something went wrongSeverity, not the settings, decides whether a switch is consulted. Connects, disconnects, world transfers, resyncs, dropped commands, quarantined batches and every fault are written with every switch off — nobody turns a switch on before the crash they did not know was coming. Verbose adds detail underneath those lines.
Why two files still
The game log is the readable one. The flight log is the same content made durable, and it earns the second file with four things the game log cannot do:
key=value)Nothing reaches the game log without also reaching the flight log, so "send us
CS2MP-flight.log" is now a complete answer.help/errors-and-warnings.mdsays so.Detail and trace lines are buffered rather than flushed, and the next fault commits them — turning a topic on no longer puts a disk write in every frame.
Per-feature switches, not one dev flag
LogTopicnames eighteen: session, transport, world transfer, resync, pipeline, nets, buildings, land, city, routes, the four zone economies, players, UI, startup, performance. Each gets its own switch on a new Logging options tab, grouped the way a player narrows a problem down, plus a Log Everything master for "I do not know which one".The enum lives in
Core/Diagnosticsso the portable networking and session code names the same topics without referencing a game assembly.IModLoggermirrors the same shape;ColossalModLoggeris the one seam. All nine locale files updated, key parity verified (294 keys).Prefixes belong to the logger
Call sites no longer write them.
[MP],[MP][OCC-DEV],[security],[upnp],[relay]and[compatibility]are gone; the logger attaches one[topic]tag, so every line is greppable and no two subsystems can drift into spelling the same tag differently.Other cleanups
"X ready."lines replaced by one startup line carrying the mod, protocol and game versionsRecord, which the mechanical pass would have turned into an always-on line)Summary()rather than just the reasonSetting.csreads a logging setting directly any moreVerification
No .NET SDK or CS2 game assemblies are available in this environment, so this has not been compiled — please build before merging.
What was checked instead:
.csfilesThat last check caught two real bugs introduced by the automated passes — a dropped
usingblock inCompanyStatsSyncSystem.csand an off-by-one that ate opening quotes — both fixed. It is careful, but it is not a compiler.