Skip to content

Commit

Permalink
Mirror: Unify Content's EntitySystem logging (#240)
Browse files Browse the repository at this point in the history 
## Mirror of PR #26216: [Unify `Content`'s `EntitySystem`
logging](space-wizards/space-station-14#26216)
from <img src="https://avatars.githubusercontent.com/u/10567778?v=4"
alt="space-wizards" width="22"/>
[space-wizards](https://github.com/space-wizards)/[space-station-14](https://github.com/space-wizards/space-station-14)

###### `eeaea6c25b496106eb741e93738f2ab8503949ba`

PR opened by <img
src="https://avatars.githubusercontent.com/u/27449516?v=4"
width="16"/><a href="https://github.com/LordCarve"> LordCarve</a> at
2024-03-17 20:05:08 UTC

---

PR changed 13 files with 43 additions and 82 deletions.

The PR had the following labels:
- Status: Needs Review


---

<details open="true"><summary><h1>Original Body</h1></summary>

> <!-- Please read these guidelines before opening your PR:
https://docs.spacestation14.io/en/getting-started/pr-guideline -->
> <!-- The text between the arrows are comments - they will not be
visible on your PR. -->
> 
> ## About the PR
> <!-- What did you change in this PR? -->
> Log things via `Log` with generated sawmill name rather than an
explicitly set one for all Content `EntitySystem`s, except the ones with
`Rule`s. In all cases the explicit `_sawmill` was redundant.
> 
> ## Why / Balance
> <!-- Why was it changed? Link any discussions or issues here. Please
discuss how this would affect game balance. -->
> - Bringing consistency to logs (all logs originating from
`EntitySystem` should be recognizible in the logs `system.something`).
> - Logs' sawmills match system class name to assist with debugging.
> - Less likelihood of someone building a new `EntitySystem` to
copy-paste the unnecessary `_sawmill` and instaed use the appropriate
inherited `Log` instead.
> 
> ## Technical details
> <!-- If this is a code change, summarize at high level how your new
code works. This makes it easier to review. -->
> This addresses **_just_** `Content`'s `EntitySystem`s.
> 
> `GameRuleSystem` **and all classes inheriting from it are excluded.**
I want to include both the calling system name and the rule name in the
sawmill name - this however requires engine changes. I thus opted to cut
out that part and made this a `Content`-only PR.
> 
> Also, even with the rule changes, the `GameRule`s themselves will be
excluded pending antag refactor, because currently they are a _hot-hot_
mess and I'm sure it's preferable by everyone for me to wait for it to
cool down before introducing merge conflicts unnecessarily.
> 
> Log Sawmill changes:
> **System class | current master sawmill | this PR's sawmill**
> `VaporSystem.cs`: `vapor` -> `system.vapor`
> `DeviceListSystem.cs`: `devicelist` -> `system.device_list`
> `ForensicScannerSystem.cs`: `forensic.scanner` ->
`system.forensic_scanner`
> `MappingSystem.cs`: `autosave` -> `system.mapping`
> `MechSystem.cs`: `mech` -> `system.mech`
> `LagCompensationSystem.cs`: `lagcomp` -> `system.lag_compensation`
> `NpcFactionSystem.cs`: `faction` -> `system.npc_faction`
> `EmergencyShuttleSystem.cs`: `shuttle.emergency` ->
`system.emergency_shuttle`
> `EventManagerSystem.cs`: `events` -> `system.event_manager`
> `VendingMachineSystem.cs`: `vending` -> `system.vending_machine`
> `SharedAnomalySystem.cs`: `anomaly` -> `system.anomaly`
> `SharedDeviceLinkSystem.cs`: `devicelink` -> `system.device_link`
> `ThirstSystem.cs`: `thirst` -> `system.thirst`
> 
> Manually tested most (all that I had doubts about) and confirmed that
`LagCompensationSystem`'s `Log.Level` is getting correctly set like
this.
> 
> ## Media
> <!-- 
> PRs which make ingame changes (adding clothing, items, new features,
etc) are required to have media attached that showcase the changes.
> Small fixes/refactors are exempt.
> Any media may be used in SS14 progress reports, with clear credit
given.
> 
> If you're unsure whether your PR will require media, ask a maintainer.
> 
> Check the box below to confirm that you have in fact seen this (put an
X in the brackets, like [X]):
> -->
> 
> - [X] I have added screenshots/videos to this PR showcasing its
changes ingame, **or** this PR does not require an ingame showcase
> 
> ## Breaking changes
> <!--
> List any breaking changes, including namespace, public
class/method/field changes, prototype renames; and provide instructions
for fixing them. This will be pasted in #codebase-changes.
> -->
> Log sawmill name changes. Any analyzers/other software that processes
logs needs to be adjusted to new sawmills. Full list of changes in PR -
section Technical details.


</details>

Co-authored-by: SimpleStation14 <Unknown>
  • Loading branch information
@SimpleStation14
SimpleStation14 authored May 12, 2024
1 parent 6d736ae commit cc41b00
Show file tree
Hide file tree
Showing 13 changed files with 45 additions and 83 deletions.
6 changes: 2 additions & 4 deletions Content.Server/Chemistry/EntitySystems/VaporSystem.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,10 @@ internal sealed class VaporSystem : EntitySystem

private const float ReactTime = 0.125f;

private ISawmill _sawmill = default!;

public override void Initialize()
{
base.Initialize();
_sawmill = Logger.GetSawmill("vapor");

SubscribeLocalEvent<VaporComponent, StartCollideEvent>(HandleCollide);
}

Expand Down Expand Up @@ -128,7 +126,7 @@ private void Update(float frameTime, Entity<VaporComponent> ent, Entity<Solution

if (reaction > reagentQuantity.Quantity)
{
_sawmill.Error($"Tried to tile react more than we have for reagent {reagentQuantity}. Found {reaction} and we only have {reagentQuantity.Quantity}");
Log.Error($"Tried to tile react more than we have for reagent {reagentQuantity}. Found {reaction} and we only have {reagentQuantity.Quantity}");
reaction = reagentQuantity.Quantity;
}

Expand Down
7 changes: 2 additions & 5 deletions Content.Server/DeviceNetwork/Systems/DeviceListSystem.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Linq;
using System.Linq;
using Content.Server.DeviceNetwork.Components;
using Content.Shared.DeviceNetwork;
using Content.Shared.DeviceNetwork.Components;
Expand All @@ -12,8 +12,6 @@ namespace Content.Server.DeviceNetwork.Systems;
[UsedImplicitly]
public sealed class DeviceListSystem : SharedDeviceListSystem
{
private ISawmill _sawmill = default!;

[Dependency] private readonly NetworkConfiguratorSystem _configurator = default!;

public override void Initialize()
Expand All @@ -23,7 +21,6 @@ public override void Initialize()
SubscribeLocalEvent<DeviceListComponent, BeforeBroadcastAttemptEvent>(OnBeforeBroadcast);
SubscribeLocalEvent<DeviceListComponent, BeforePacketSentEvent>(OnBeforePacketSent);
SubscribeLocalEvent<BeforeSaveEvent>(OnMapSave);
_sawmill = Logger.GetSawmill("devicelist");
}

private void OnShutdown(EntityUid uid, DeviceListComponent component, ComponentShutdown args)
Expand Down Expand Up @@ -154,7 +151,7 @@ private void OnMapSave(BeforeSaveEvent ev)
// TODO full game saves.
// when full saves are supported, this should instead add data to the BeforeSaveEvent informing the
// saving system that this map (or null-space entity) also needs to be included in the save.
_sawmill.Error(
Log.Error(
$"Saving a device list ({ToPrettyString(uid)}) that has a reference to an entity on another map ({ToPrettyString(ent)}). Removing entity from list.");
}

Expand Down
10 changes: 3 additions & 7 deletions Content.Server/Forensics/Systems/ForensicScannerSystem.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,10 @@ public sealed class ForensicScannerSystem : EntitySystem
[Dependency] private readonly SharedAudioSystem _audioSystem = default!;
[Dependency] private readonly MetaDataSystem _metaData = default!;

private ISawmill _sawmill = default!;

public override void Initialize()
{
base.Initialize();

_sawmill = Logger.GetSawmill("forensics.scanner");

SubscribeLocalEvent<ForensicScannerComponent, AfterInteractEvent>(OnAfterInteract);
SubscribeLocalEvent<ForensicScannerComponent, AfterInteractUsingEvent>(OnAfterInteractUsing);
SubscribeLocalEvent<ForensicScannerComponent, BeforeActivatableUIOpenEvent>(OnBeforeActivatableUIOpen);
Expand All @@ -57,7 +53,7 @@ private void UpdateUserInterface(EntityUid uid, ForensicScannerComponent compone
component.PrintReadyAt);

if (!_uiSystem.TrySetUiState(uid, ForensicScannerUiKey.Key, state))
_sawmill.Warning($"{ToPrettyString(uid)} was unable to set UI state.");
Log.Warning($"{ToPrettyString(uid)} was unable to set UI state.");
}

private void OnDoAfter(EntityUid uid, ForensicScannerComponent component, DoAfterEvent args)
Expand Down Expand Up @@ -180,7 +176,7 @@ private void OnPrint(EntityUid uid, ForensicScannerComponent component, Forensic
{
if (!args.Session.AttachedEntity.HasValue)
{
_sawmill.Warning($"{ToPrettyString(uid)} got OnPrint without Session.AttachedEntity");
Log.Warning($"{ToPrettyString(uid)} got OnPrint without Session.AttachedEntity");
return;
}

Expand All @@ -200,7 +196,7 @@ private void OnPrint(EntityUid uid, ForensicScannerComponent component, Forensic

if (!HasComp<PaperComponent>(printed))
{
_sawmill.Error("Printed paper did not have PaperComponent.");
Log.Error("Printed paper did not have PaperComponent.");
return;
}

Expand Down
14 changes: 6 additions & 8 deletions Content.Server/Mapping/MappingSystem.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.IO;
using System.IO;
using Content.Server.Administration;
using Content.Shared.Administration;
using Content.Shared.CCVar;
Expand Down Expand Up @@ -32,7 +32,6 @@ public sealed class MappingSystem : EntitySystem
/// <returns></returns>
private Dictionary<MapId, (TimeSpan next, string fileName)> _currentlyAutosaving = new();

private ISawmill _sawmill = default!;
private bool _autosaveEnabled;

public override void Initialize()
Expand All @@ -44,7 +43,6 @@ public override void Initialize()
"autosave <map> <path if enabling>",
ToggleAutosaveCommand);

_sawmill = Logger.GetSawmill("autosave");
Subs.CVar(_cfg, CCVars.AutosaveEnabled, SetAutosaveEnabled, true);
}

Expand All @@ -69,7 +67,7 @@ public override void Update(float frameTime)

if (!_mapManager.MapExists(map) || _mapManager.IsMapInitialized(map))
{
_sawmill.Warning($"Can't autosave map {map}; it doesn't exist, or is initialized. Removing from autosave.");
Log.Warning($"Can't autosave map {map}; it doesn't exist, or is initialized. Removing from autosave.");
_currentlyAutosaving.Remove(map);
return;
}
Expand All @@ -79,7 +77,7 @@ public override void Update(float frameTime)

var path = Path.Combine(saveDir, $"{DateTime.Now.ToString("yyyy-M-dd_HH.mm.ss")}-AUTO.yml");
_currentlyAutosaving[map] = (CalculateNextTime(), name);
_sawmill.Info($"Autosaving map {name} ({map}) to {path}. Next save in {ReadableTimeLeft(map)} seconds.");
Log.Info($"Autosaving map {name} ({map}) to {path}. Next save in {ReadableTimeLeft(map)} seconds.");
_map.SaveMap(map, path);
}
}
Expand All @@ -105,17 +103,17 @@ public void ToggleAutosave(MapId map, string? path=null)
{
if (!_mapManager.MapExists(map) || _mapManager.IsMapInitialized(map))
{
_sawmill.Warning("Tried to enable autosaving on non-existant or already initialized map!");
Log.Warning("Tried to enable autosaving on non-existant or already initialized map!");
_currentlyAutosaving.Remove(map);
return;
}

_sawmill.Info($"Started autosaving map {path} ({map}). Next save in {ReadableTimeLeft(map)} seconds.");
Log.Info($"Started autosaving map {path} ({map}). Next save in {ReadableTimeLeft(map)} seconds.");
}
else
{
_currentlyAutosaving.Remove(map);
_sawmill.Info($"Stopped autosaving on map {map}");
Log.Info($"Stopped autosaving on map {map}");
}
}

Expand Down
6 changes: 1 addition & 5 deletions Content.Server/Mech/Systems/MechSystem.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,11 @@ public sealed partial class MechSystem : SharedMechSystem
[Dependency] private readonly SharedPopupSystem _popup = default!;
[Dependency] private readonly UserInterfaceSystem _ui = default!;

private ISawmill _sawmill = default!;

/// <inheritdoc/>
public override void Initialize()
{
base.Initialize();

_sawmill = Logger.GetSawmill("mech");

SubscribeLocalEvent<MechComponent, InteractUsingEvent>(OnInteractUsing);
SubscribeLocalEvent<MechComponent, EntInsertedIntoContainerMessage>(OnInsertBattery);
SubscribeLocalEvent<MechComponent, MapInitEvent>(OnMapInit);
Expand Down Expand Up @@ -343,7 +339,7 @@ public override bool TryChangeEnergy(EntityUid uid, FixedPoint2 delta, MechCompo
_battery.SetCharge(battery!.Value, batteryComp.CurrentCharge + delta.Float(), batteryComp);
if (batteryComp.CurrentCharge != component.Energy) //if there's a discrepency, we have to resync them
{
_sawmill.Debug($"Battery charge was not equal to mech charge. Battery {batteryComp.CurrentCharge}. Mech {component.Energy}");
Log.Debug($"Battery charge was not equal to mech charge. Battery {batteryComp.CurrentCharge}. Mech {component.Energy}");
component.Energy = batteryComp.CurrentCharge;
Dirty(component);
}
Expand Down
8 changes: 3 additions & 5 deletions Content.Server/Movement/Systems/LagCompensationSystem.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,10 @@ public sealed class LagCompensationSystem : EntitySystem
// Max ping I've had is 350ms from aus to spain.
public static readonly TimeSpan BufferTime = TimeSpan.FromMilliseconds(750);

private ISawmill _sawmill = Logger.GetSawmill("lagcomp");

public override void Initialize()
{
base.Initialize();
_sawmill.Level = LogLevel.Info;
Log.Level = LogLevel.Info;
SubscribeLocalEvent<LagCompensationComponent, MoveEvent>(OnLagMove);
}

Expand Down Expand Up @@ -87,13 +85,13 @@ private void OnLagMove(EntityUid uid, LagCompensationComponent component, ref Mo

if (coordinates == default)
{
_sawmill.Debug($"No long comp coords found, using {xform.Coordinates}");
Log.Debug($"No long comp coords found, using {xform.Coordinates}");
coordinates = xform.Coordinates;
angle = xform.LocalRotation;
}
else
{
_sawmill.Debug($"Actual coords is {xform.Coordinates} and got {coordinates}");
Log.Debug($"Actual coords is {xform.Coordinates} and got {coordinates}");
}

return (coordinates, angle);
Expand Down
16 changes: 7 additions & 9 deletions Content.Server/NPC/Systems/NpcFactionSystem.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ public sealed partial class NpcFactionSystem : EntitySystem
[Dependency] private readonly EntityLookupSystem _lookup = default!;
[Dependency] private readonly IPrototypeManager _protoManager = default!;

private ISawmill _sawmill = default!;

/// <summary>
/// To avoid prototype mutability we store an intermediary data class that gets used instead.
/// </summary>
Expand All @@ -25,7 +23,7 @@ public sealed partial class NpcFactionSystem : EntitySystem
public override void Initialize()
{
base.Initialize();
_sawmill = Logger.GetSawmill("faction");

SubscribeLocalEvent<NpcFactionMemberComponent, ComponentStartup>(OnFactionStartup);
SubscribeLocalEvent<PrototypesReloadedEventArgs>(OnProtoReload);

Expand Down Expand Up @@ -70,7 +68,7 @@ public void AddFaction(EntityUid uid, string faction, bool dirty = true)
{
if (!_protoManager.HasIndex<NpcFactionPrototype>(faction))
{
_sawmill.Error($"Unable to find faction {faction}");
Log.Error($"Unable to find faction {faction}");
return;
}

Expand All @@ -91,7 +89,7 @@ public void RemoveFaction(EntityUid uid, string faction, bool dirty = true)
{
if (!_protoManager.HasIndex<NpcFactionPrototype>(faction))
{
_sawmill.Error($"Unable to find faction {faction}");
Log.Error($"Unable to find faction {faction}");
return;
}

Expand Down Expand Up @@ -214,13 +212,13 @@ public void MakeFriendly(string source, string target)
{
if (!_factions.TryGetValue(source, out var sourceFaction))
{
_sawmill.Error($"Unable to find faction {source}");
Log.Error($"Unable to find faction {source}");
return;
}

if (!_factions.ContainsKey(target))
{
_sawmill.Error($"Unable to find faction {target}");
Log.Error($"Unable to find faction {target}");
return;
}

Expand Down Expand Up @@ -256,13 +254,13 @@ public void MakeHostile(string source, string target)
{
if (!_factions.TryGetValue(source, out var sourceFaction))
{
_sawmill.Error($"Unable to find faction {source}");
Log.Error($"Unable to find faction {source}");
return;
}

if (!_factions.ContainsKey(target))
{
_sawmill.Error($"Unable to find faction {target}");
Log.Error($"Unable to find faction {target}");
return;
}

Expand Down
9 changes: 3 additions & 6 deletions Content.Server/Shuttles/Systems/EmergencyShuttleSystem.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,6 @@ public sealed partial class EmergencyShuttleSystem : EntitySystem
[Dependency] private readonly TransformSystem _transformSystem = default!;
[Dependency] private readonly UserInterfaceSystem _uiSystem = default!;

private ISawmill _sawmill = default!;

private const float ShuttleSpawnBuffer = 1f;

private bool _emergencyShuttleEnabled;
Expand All @@ -75,7 +73,6 @@ public sealed partial class EmergencyShuttleSystem : EntitySystem

public override void Initialize()
{
_sawmill = Logger.GetSawmill("shuttle.emergency");
_emergencyShuttleEnabled = _configManager.GetCVar(CCVars.EmergencyShuttleEnabled);
// Don't immediately invoke as roundstart will just handle it.
Subs.CVar(_configManager, CCVars.EmergencyShuttleEnabled, SetEmergencyShuttleEnabled);
Expand Down Expand Up @@ -391,7 +388,7 @@ private void AddCentcomm(StationCentcommComponent component)
{
if (component.MapEntity != null || component.Entity != null)
{
_sawmill.Warning("Attempted to re-add an existing centcomm map.");
Log.Warning("Attempted to re-add an existing centcomm map.");
return;
}

Expand All @@ -416,7 +413,7 @@ private void AddCentcomm(StationCentcommComponent component)

if (string.IsNullOrEmpty(component.Map.ToString()))
{
_sawmill.Warning("No CentComm map found, skipping setup.");
Log.Warning("No CentComm map found, skipping setup.");
return;
}

Expand Down Expand Up @@ -491,7 +488,7 @@ private void AddEmergencyShuttle(EntityUid uid, StationEmergencyShuttleComponent

if (shuttle == null)
{
_sawmill.Error($"Unable to spawn emergency shuttle {shuttlePath} for {ToPrettyString(uid)}");
Log.Error($"Unable to spawn emergency shuttle {shuttlePath} for {ToPrettyString(uid)}");
return;
}

Expand Down
Loading

0 comments on commit cc41b00

Please sign in to comment.