Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ЦК v2 #928

Merged
merged 3 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions Content.Server/Backmen/Arrivals/Centcomm/CentComEventId.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
namespace Content.Server.Backmen.Arrivals.CentComm;

public enum CentComEventId : int
{
Noop = 0,
AddWorker,
AddOperator,
AddSecurity,
AddCargo
}
7 changes: 7 additions & 0 deletions Content.Server/Backmen/Arrivals/Centcomm/CentCommEvent.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
namespace Content.Server.Backmen.Arrivals.CentComm;

public sealed class CentCommEvent(EntityUid station,CentComEventId eventId) : HandledEntityEventArgs
{
public EntityUid Station { get; } = station;
public CentComEventId EventId { get; } = eventId;
}
101 changes: 101 additions & 0 deletions Content.Server/Backmen/Arrivals/Centcomm/CentCommSpawnSystem.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
using Content.Server.Spawners.Components;
using Content.Server.Station.Components;
using Robust.Shared.Map;
using Robust.Shared.Prototypes;
using Robust.Shared.Random;

namespace Content.Server.Backmen.Arrivals.CentComm;

public sealed class CentCommSpawnSystem : EntitySystem
{
[Dependency] private readonly IRobustRandom _random = default!;
public override void Initialize()
{
base.Initialize();

SubscribeLocalEvent<StationCentCommDirectorComponent, CentCommEvent>(OnCentCommEvent);
}

private void OnCentCommEvent(Entity<StationCentCommDirectorComponent> ent, ref CentCommEvent args)
{
if(args.Handled)
return;

switch (args.EventId)
{
case CentComEventId.AddWorker:
args.Handled = true;

AddWorker(args.Station);
break;
case CentComEventId.AddOperator:
args.Handled = true;

AddOperator(args.Station);
break;
case CentComEventId.AddSecurity:
args.Handled = true;

AddSecurity(args.Station);
break;
case CentComEventId.AddCargo:
args.Handled = true;

AddCargo(args.Station);
break;
default:
return;
}
}
Comment on lines +19 to +49
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider improving event handling robustness and readability.

A few suggestions to enhance this section:

  1. Add null check for args.Station
  2. Consider reducing repetition of args.Handled = true
  3. Fix spacing in the if condition

Here's a suggested improvement:

 private void OnCentCommEvent(Entity<StationCentCommDirectorComponent> ent, ref CentCommEvent args)
 {
-    if(args.Handled)
+    if (args.Handled || args.Station == default)
         return;
+
+    args.Handled = true;
     switch (args.EventId)
     {
         case CentComEventId.AddWorker:
-            args.Handled = true;
             AddWorker(args.Station);
             break;
         case CentComEventId.AddOperator:
-            args.Handled = true;
             AddOperator(args.Station);
             break;
         case CentComEventId.AddSecurity:
-            args.Handled = true;
             AddSecurity(args.Station);
             break;
         case CentComEventId.AddCargo:
-            args.Handled = true;
             AddCargo(args.Station);
             break;
         default:
+            args.Handled = false;
             return;
     }
 }
📝 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.

Suggested change
private void OnCentCommEvent(Entity<StationCentCommDirectorComponent> ent, ref CentCommEvent args)
{
if(args.Handled)
return;
switch (args.EventId)
{
case CentComEventId.AddWorker:
args.Handled = true;
AddWorker(args.Station);
break;
case CentComEventId.AddOperator:
args.Handled = true;
AddOperator(args.Station);
break;
case CentComEventId.AddSecurity:
args.Handled = true;
AddSecurity(args.Station);
break;
case CentComEventId.AddCargo:
args.Handled = true;
AddCargo(args.Station);
break;
default:
return;
}
}
private void OnCentCommEvent(Entity<StationCentCommDirectorComponent> ent, ref CentCommEvent args)
{
if (args.Handled || args.Station == default)
return;
args.Handled = true;
switch (args.EventId)
{
case CentComEventId.AddWorker:
AddWorker(args.Station);
break;
case CentComEventId.AddOperator:
AddOperator(args.Station);
break;
case CentComEventId.AddSecurity:
AddSecurity(args.Station);
break;
case CentComEventId.AddCargo:
AddCargo(args.Station);
break;
default:
args.Handled = false;
return;
}
}


private void SpawnEntity(EntityUid station, string protoId)
{
var point = FindSpawnPoint(station);
if (point == null)
{
Log.Warning($"Can't find spawn point for {station}");
return;
}
Spawn(protoId, point.Value);
}

[ValidatePrototypeId<EntityPrototype>]
private const string WorkerProto = "SpawnPointCMBKCCAssistant";
private void AddWorker(EntityUid station) => SpawnEntity(station, WorkerProto);

[ValidatePrototypeId<EntityPrototype>]
private const string OperatorProto = "SpawnPointCMBKCCOperator";
private void AddOperator(EntityUid station) => SpawnEntity(station, OperatorProto);

[ValidatePrototypeId<EntityPrototype>]
private const string SecurityProto = "SpawnPointCMBKCCSecOfficer";
private void AddSecurity(EntityUid station) => SpawnEntity(station, SecurityProto);

[ValidatePrototypeId<EntityPrototype>]
private const string CargoProto = "SpawnPointCMBKCCCargo";
private void AddCargo(EntityUid station) => SpawnEntity(station, CargoProto);

private EntityCoordinates? FindSpawnPoint(EntityUid station)
{
var stationData = CompOrNull<StationDataComponent>(station);
if (stationData == null)
return null;

var stationGrids = stationData.Grids;

var result = new List<EntityCoordinates>();

var q = EntityQueryEnumerator<SpawnPointComponent,TransformComponent>();
while (q.MoveNext(out var uid, out var spawnPoint, out var transform))
{
if(spawnPoint.SpawnType != SpawnPointType.LateJoin || transform.GridUid == null)
continue;
if(!stationGrids.Contains(transform.GridUid.Value))
continue;

result.Add(transform.Coordinates);
}

return result.Count == 0 ? null : _random.Pick(result);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
using Content.Server.Shuttles.Components;

namespace Content.Server.Backmen.Arrivals.CentComm;

public sealed class FtlCentComAnnounce : EntityEventArgs
{
public Entity<ShuttleComponent> Source { get; set; }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
using Robust.Shared.Serialization.TypeSerializers.Implementations.Custom;

namespace Content.Server.Backmen.Arrivals.CentComm;

[RegisterComponent]
public sealed partial class StationCentCommDirectorComponent : Component
{
/// <summary>
/// Keeps track of the internal event scheduler.
/// </summary>
[ViewVariables]
[DataField("nextEventTick", customTypeSerializer: typeof(TimeOffsetSerializer))]
public TimeSpan NextEventTick;
Comment on lines +8 to +13
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding validation and proper initialization for NextEventTick.

The property could benefit from additional safeguards:

  1. Add validation to prevent setting past times
  2. Initialize with a meaningful default value
  3. Consider making the setter private or internal

Here's a suggested improvement:

 [ViewVariables]
 [DataField("nextEventTick", customTypeSerializer: typeof(TimeOffsetSerializer))]
-public TimeSpan NextEventTick;
+public TimeSpan NextEventTick { get; internal set; } = TimeSpan.FromMinutes(5); // Or another appropriate default
+
+internal void SetNextEventTick(TimeSpan time)
+{
+    if (time < TimeSpan.Zero)
+        throw new ArgumentException("Event tick cannot be in the past", nameof(time));
+    NextEventTick = time;
+}
📝 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.

Suggested change
/// <summary>
/// Keeps track of the internal event scheduler.
/// </summary>
[ViewVariables]
[DataField("nextEventTick", customTypeSerializer: typeof(TimeOffsetSerializer))]
public TimeSpan NextEventTick;
/// <summary>
/// Keeps track of the internal event scheduler.
/// </summary>
[ViewVariables]
[DataField("nextEventTick", customTypeSerializer: typeof(TimeOffsetSerializer))]
public TimeSpan NextEventTick { get; internal set; } = TimeSpan.FromMinutes(5); // Or another appropriate default
internal void SetNextEventTick(TimeSpan time)
{
if (time < TimeSpan.Zero)
throw new ArgumentException("Event tick cannot be in the past", nameof(time));
NextEventTick = time;
}


/// <summary>
/// The schedule of events to occur.
/// </summary>
[ViewVariables]
[DataField("eventSchedule")]
public List<(TimeSpan timeOffset, CentComEventId eventId)> EventSchedule = new();
Comment on lines +15 to +20
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance EventSchedule safety and documentation.

The current implementation has several potential improvements:

  1. The list should be immutable from outside
  2. Time offsets should be validated
  3. Documentation should detail the tuple elements

Here's a suggested improvement:

 /// <summary>
 /// The schedule of events to occur.
+/// </summary>
+/// <remarks>
+/// Each tuple contains:
+/// - timeOffset: The time delay before the event should occur
+/// - eventId: The specific type of event to trigger
 /// </remarks>
 [ViewVariables]
 [DataField("eventSchedule")]
-public List<(TimeSpan timeOffset, CentComEventId eventId)> EventSchedule = new();
+public IReadOnlyList<(TimeSpan timeOffset, CentComEventId eventId)> EventSchedule => _eventSchedule;
+private readonly List<(TimeSpan timeOffset, CentComEventId eventId)> _eventSchedule = new();
+
+public void AddEvent(TimeSpan timeOffset, CentComEventId eventId)
+{
+    if (timeOffset < TimeSpan.Zero)
+        throw new ArgumentException("Time offset cannot be negative", nameof(timeOffset));
+
+    _eventSchedule.Add((timeOffset, eventId));
+}
📝 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.

Suggested change
/// <summary>
/// The schedule of events to occur.
/// </summary>
[ViewVariables]
[DataField("eventSchedule")]
public List<(TimeSpan timeOffset, CentComEventId eventId)> EventSchedule = new();
/// <summary>
/// The schedule of events to occur.
/// </summary>
/// <remarks>
/// Each tuple contains:
/// - timeOffset: The time delay before the event should occur
/// - eventId: The specific type of event to trigger
/// </remarks>
[ViewVariables]
[DataField("eventSchedule")]
public IReadOnlyList<(TimeSpan timeOffset, CentComEventId eventId)> EventSchedule => _eventSchedule;
private readonly List<(TimeSpan timeOffset, CentComEventId eventId)> _eventSchedule = new();
public void AddEvent(TimeSpan timeOffset, CentComEventId eventId)
{
if (timeOffset < TimeSpan.Zero)
throw new ArgumentException("Time offset cannot be negative", nameof(timeOffset));
_eventSchedule.Add((timeOffset, eventId));
}

}
42 changes: 37 additions & 5 deletions Content.Server/Backmen/Arrivals/CentcommSystem.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Numerics;
using Content.Server.Backmen.Arrivals.CentComm;
using Content.Server.Chat.Systems;
using Content.Server.GameTicking;
using Content.Server.GameTicking.Events;
Expand Down Expand Up @@ -32,15 +33,11 @@
using Robust.Shared.Player;
using Robust.Shared.Prototypes;
using Robust.Shared.Random;
using Robust.Shared.Timing;
using Robust.Shared.Utility;

namespace Content.Server.Backmen.Arrivals;

public sealed class FtlCentComAnnounce : EntityEventArgs
{
public Entity<ShuttleComponent> Source { get; set; }
}

public sealed class CentcommSystem : EntitySystem
{
[Dependency] private readonly PopupSystem _popup = default!;
Expand All @@ -58,6 +55,7 @@ public sealed class CentcommSystem : EntitySystem
[Dependency] private readonly MetaDataSystem _metaDataSystem = default!;
[Dependency] private readonly IRobustRandom _random = default!;
[Dependency] private readonly EntityLookupSystem _lookup = default!;
[Dependency] private readonly IGameTiming _gameTiming = default!;

public EntityUid CentComGrid { get; private set; } = EntityUid.Invalid;
public MapId CentComMap { get; private set; } = MapId.Nullspace;
Expand All @@ -83,6 +81,40 @@ public override void Initialize()
_stationCentComMapPool = _prototypeManager.Index<WeightedRandomPrototype>(StationCentComMapPool);
}

public override void Update(float frameTime)
{
base.Update(frameTime);

var curTime = _gameTiming.CurTime;

var q = EntityQueryEnumerator<StationCentCommDirectorComponent, StationSpawningComponent>();
while (q.MoveNext(out var stationUid, out var centcomDirector, out var stationSpawning))
{
if (!(centcomDirector.EventSchedule.Count > 0 && curTime >= centcomDirector.NextEventTick))
{
continue;
}

// Pop the event.
var curEvent = centcomDirector.EventSchedule[0];
centcomDirector.EventSchedule.RemoveAt(0);

// Add the next event's offset to the ticker.
if (centcomDirector.EventSchedule.Count > 0)
centcomDirector.NextEventTick = curTime + centcomDirector.EventSchedule[0].timeOffset;

Log.Info($"Running event: {curEvent}");

var ev = new CentCommEvent(stationUid, curEvent.eventId);
RaiseLocalEvent(stationUid, ev, true);
if (!ev.Handled)
{
Log.Warning($"Running event: {curEvent} is not handled");
}
}
}


private void OnLoadingMaps(LoadingMapsEvent ev)
{
if (_gameTicker.CurrentPreset?.IsMiniGame ?? false)
Expand Down
10 changes: 10 additions & 0 deletions Resources/Prototypes/Entities/Stations/nanotrasen.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@
categories: [ HideSpawnMenu ]
components:
- type: Transform
- type: StationCentCommDirector
eventSchedule: # 10min = 600sec
- 0: Noop # init event prevent from missfire
- 600: AddWorker #10min
- 600: AddWorker #20min
- 600: AddWorker #30min
- 600: AddOperator #40min
- 1: AddSecurity
- 1: AddSecurity
- 600: AddCargo

- type: entity
id: StandardStationArena
Expand Down
59 changes: 59 additions & 0 deletions Resources/Prototypes/_Backmen/CentComm/BKCCAssistant.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
- type: entity
id: SpawnPointCMBKCCAssistant
name: Ассистент ЦК
suffix: Спавнер, Директор Событий
parent: MarkerBase
components:
- type: GhostRole
allowMovement: true
allowSpeech: true
makeSentient: true
name: Ассистент ЦК
description: Разнорабочий призванный для поддержки экипажа ЦК и для поддержки самой станции цк в текущем секторе.
rules: Вы обязаны выполнять поручения экипажа станции цк, опеспечивать станцию цк в рабочем состоянии, также запрещено покидать станцию цк!
raffle:
settings: short
requirements:
- !type:DepartmentTimeRequirement
department: Engineering
time: 108000
- !type:DepartmentTimeRequirement
department: Medical
time: 108000
- !type:DepartmentTimeRequirement
department: Civilian
time: 108000
job: BKCCAssistant
- type: GhostRoleMobSpawner
prototype: MobHumanCMBKCCAssistant
- type: Sprite
sprite: Markers/jobs.rsi
layers:
- state: green
- sprite: Mobs/Animals/regalrat.rsi
state: icon
Comment on lines +33 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Inappropriate sprite configuration

The spawner is using regalrat.rsi as part of its sprite layers, which seems inappropriate for a Central Command Assistant role. Consider using a more appropriate icon that reflects the professional nature of the role.



- type: entity
name: Ассистент ЦК
suffix: Спавнер, Директор Событий
parent: MobHumanCombine
id: MobHumanCMBKCCAssistant
components:
- type: Icon
sprite: Markers/jobs.rsi
state: ai
- type: AutoImplant
implants:
- MindShieldImplant
- FreedomImplant
- type: SpecForce
actionBssActionName: ActionCentcomFtlAction
- type: NpcFactionMember
factions:
- CentralCommand
- type: Loadout
prototypes: [CentComInternGear]
- type: RandomHumanoidAppearance
randomizeName: true
- type: AntagImmune
61 changes: 61 additions & 0 deletions Resources/Prototypes/_Backmen/CentComm/BKCCCargo.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
- type: entity
id: SpawnPointCMBKCCCargo
name: Карго ЦК
suffix: Спавнер, Директор Событий
parent: MarkerBase
components:
- type: GhostRole
allowMovement: true
allowSpeech: true
makeSentient: true
name: Карго ЦК
description: A Central Command Cargo Officer responsible for managing cargo operations.
rules: |
- Maintain Central Command cargo protocols
- Coordinate with station cargo department
- Follow Central Command chain of command
raffle:
settings: short
requirements:
- !type:DepartmentTimeRequirement
department: Cargo
time: 126000
- !type:RoleTimeRequirement
role: JobCentralCommandAssistant
time: 21600
- !type:DepartmentTimeRequirement
department: CentCom
time: 43200
Comment on lines +20 to +28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Based on the search results, I can now provide a comprehensive analysis of the time requirements across the codebase. Let me generate the final response:

Time requirements for CentComm Cargo role are significantly higher than other similar roles

The current time requirements for CentComm Cargo (35h Cargo, 6h CC Assistant, 12h CentCom) are disproportionate compared to other CentComm roles:

  • CentComm Operator: 2h CC Assistant, 4h CentCom, 1.7h Silicons
  • CentComm Security: 30h Security, 1h CC Assistant
  • Other department roles typically require 5-20 hours

Recommend adjusting the time requirements to be more in line with other CentComm roles:

  • Reduce Cargo requirement from 126000 (35h) to ~72000 (20h)
  • Reduce CC Assistant from 21600 (6h) to ~7200 (2h)
  • Reduce CentCom from 43200 (12h) to ~14400 (4h)
🔗 Analysis chain

Verify the time requirements for role eligibility

The current time requirements seem exceptionally high:

  • Cargo department: 35 hours
  • CC Assistant role: 6 hours
  • CentCom department: 12 hours

These high requirements might severely limit the pool of eligible players.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for similar ghost role time requirements across the codebase
rg -U "DepartmentTimeRequirement|RoleTimeRequirement" -A 2 | grep -E "time:|department:|role:"

Length of output: 34960

job: BKCCCargo
- type: GhostRoleMobSpawner
prototype: MobHumanCMBKCCCargo
- type: Sprite
sprite: Markers/jobs.rsi
layers:
- state: green
- sprite: Mobs/Animals/regalrat.rsi
state: icon

- type: entity
name: Карго ЦК
suffix: Директор Событий
parent: MobHumanCombine
id: MobHumanCMBKCCCargo
components:
- type: Icon
sprite: Markers/jobs.rsi
state: ai
- type: AutoImplant
implants:
- MindShieldImplant
- FreedomImplant
- type: SpecForce
actionBssActionName: ActionCentcomFtlAction
- type: NpcFactionMember
factions:
- CentralCommand
- type: Loadout
prototypes: [CCCargo]
- type: RandomHumanoidAppearance
randomizeName: true
- type: AntagImmune
Loading
Loading