Skip to content

Add Sit schedule action and duration support #52

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.

Sign up for GitHub

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

Merged
ifBars merged 1 commit into from
Mar 7, 2026
Merged

Add Sit schedule action and duration support #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion S1API/Entities/NPCPrefabBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ private void PrecreateActionsForSpecs(List<IScheduleActionSpec> specs)

var mgr = EnsureScheduleManager();

int walkTo = 0, stayInBuilding = 0, locationDialogue = 0, locationBasedAction = 0, useVending = 0, driveToCarPark = 0, dealSignal = 0, useATM = 0;
int walkTo = 0, stayInBuilding = 0, locationDialogue = 0, locationBasedAction = 0, useVending = 0, driveToCarPark = 0, dealSignal = 0, useATM = 0, sit = 0;
bool requiresSmokeBreak = false, requiresGraffiti = false, requiresDrinking = false, requiresHoldItem = false;
for (int i = 0; i < specs.Count; i++)
{
Expand Down Expand Up @@ -898,6 +898,7 @@ private void PrecreateActionsForSpecs(List<IScheduleActionSpec> specs)
else if (s is DriveToCarParkSpec) driveToCarPark++;
else if (s is EnsureDealSignalSpec) dealSignal = Math.Max(dealSignal, 1);
else if (s is UseATMSpec) useATM++;
else if (s is SitSpec) sit++;
}

if (requiresSmokeBreak)
Expand Down Expand Up @@ -937,6 +938,7 @@ private void PrecreateActionsForSpecs(List<IScheduleActionSpec> specs)
EnsurePrefabAction<S1NPCsSchedules.NPCSignal_UseVendingMachine>(useVending, "UseVending");
EnsurePrefabAction<S1NPCsSchedules.NPCSignal_DriveToCarPark>(driveToCarPark, "DriveToCarPark");
EnsurePrefabAction<S1NPCsSchedules.NPCSignal_UseATM>(useATM, "UseATM");
EnsurePrefabAction<S1NPCsSchedules.NPCEvent_Sit>(sit, "Sit");
}

private void EnsurePrefabAction<T>(int count, string namePrefix) where T : S1NPCsSchedules.NPCAction
Expand Down
9 changes: 8 additions & 1 deletion S1API/Entities/Schedule/ActionSpecs/SitSpec.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,16 @@ namespace S1API.Entities.Schedule
public sealed class SitSpec : IScheduleActionSpec
{
/// <summary>
/// Gets or sets the start time for this action, in minutes from midnight.
/// Gets or sets the start time for this action, in 24-hour time (e.g. 830 for 8:30 AM).
/// </summary>
public int StartTime { get; set; }

/// <summary>
/// Gets or sets the duration of the sit action in minutes.
/// If not set (0 or negative), defaults to the time remaining until the next scheduled action.
/// </summary>
public int DurationMinutes { get; set; }
Comment on lines +33 to +37
Copy link

@coderabbitai coderabbitai bot Mar 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix the DurationMinutes XML docs.

The summary here says non-positive values fall back to "the time remaining until the next scheduled action", but Line 107 hard-codes a 60 minute fallback. That makes the public contract misleading for anyone constructing SitSpec directly.

📝 Proposed doc fix 
 /// <summary>
 /// Gets or sets the duration of the sit action in minutes.
-/// If not set (0 or negative), defaults to the time remaining until the next scheduled action.
+/// If not set (0 or negative), defaults to 60 minutes.
 /// </summary>
 public int DurationMinutes { get; set; }
📝 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>
/// Gets or sets the duration of the sit action in minutes.
/// If not set (0 or negative), defaults to the time remaining until the next scheduled action.
/// </summary>
public int DurationMinutes { get; set; }
/// <summary>
/// Gets or sets the duration of the sit action in minutes.
/// If not set (0 or negative), defaults to 60 minutes.
/// </summary>
public int DurationMinutes { get; set; }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@S1API/Entities/Schedule/ActionSpecs/SitSpec.cs` around lines 33 - 37, The XML
doc for SitSpec.DurationMinutes is inaccurate: the summary claims non-positive
values default to "the time remaining until the next scheduled action" but the
implementation uses a hard-coded 60-minute fallback; update the DurationMinutes
summary to state that non-positive values default to 60 minutes (or
alternatively change the implementation to derive from the next scheduled action
if that behavior is desired). Specifically, edit the XML comment on the
DurationMinutes property in SitSpec to reflect the actual fallback of 60 minutes
(or make the implementation and any helper method that reads DurationMinutes
conform to the documented behavior).


/// <summary>
/// Gets or sets the optional display name for this action. Defaults to "Sit".
/// </summary>
Expand Down Expand Up @@ -98,6 +104,7 @@ void IScheduleActionSpec.ApplyTo(NPCSchedule schedule)
}

action.WarpIfSkipped = WarpIfSkipped;
action.Duration = DurationMinutes > 0 ? DurationMinutes : 60;
}

private S1AvatarAnimation.AvatarSeatSet ResolveSeatSet(NPCSchedule schedule)
Expand Down
6 changes: 4 additions & 2 deletions S1API/Entities/Schedule/NPCScheduleBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ public PrefabScheduleBuilder WalkTo(UnityEngine.Vector3 destination, int startTi
/// Adds a seating action that moves the NPC to an available seat within the specified seat set.
/// </summary>
/// <param name="seatSetName">The GameObject name of the <c>AvatarSeatSet</c> to use.</param>
/// <param name="startTime">The time when this action should start, in minutes from midnight (0-1439).</param>
/// <param name="startTime">The time when this action should start, in 24-hour time (e.g. 830 for 8:30 AM).</param>
/// <param name="durationMinutes">Duration of the sit action in minutes. Defaults to 60.</param>
Comment on lines +67 to +68
Copy link

@coderabbitai coderabbitai bot Mar 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Align the startTime docs across PrefabScheduleBuilder.

This overload now documents HHmm, but the other scheduling methods in this same public API still say "minutes from midnight (0-1439)". Since they all feed the same StartTime concept, the mixed contract is easy for mod authors to misread and can lead to silently mis-scheduled actions.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@S1API/Entities/Schedule/NPCScheduleBuilder.cs` around lines 67 - 68, The XML
docs for startTime are inconsistent: PrefabScheduleBuilder describes startTime
as HHmm while other scheduling methods use "minutes from midnight"; update all
related XML <param name="startTime"> comments (including in
PrefabScheduleBuilder and NPCScheduleBuilder scheduling overloads that feed the
shared StartTime concept) to use the same format—preferably HHmm to match the
new overload—so mod authors see a consistent contract; locate all methods that
set or accept StartTime (e.g., PrefabScheduleBuilder.* overloads,
NPCScheduleBuilder methods referencing StartTime, and any overloads with
durationMinutes) and change their param text to the unified HHmm description.

/// <param name="warpIfSkipped">Whether the NPC should be warped to the seat if the action is skipped. Default is <c>false</c>.</param>
/// <param name="name">Optional custom name for this action; defaults to "Sit".</param>
/// <returns>This builder instance for method chaining.</returns>
Expand All @@ -73,14 +74,15 @@ public PrefabScheduleBuilder WalkTo(UnityEngine.Vector3 destination, int startTi
/// lookup scenarios (GUIDs, transform paths, direct references) instantiate <see cref="SitSpec"/> manually and
/// add it via <see cref="Add(IScheduleActionSpec)"/>.
/// </remarks>
public PrefabScheduleBuilder SitAtSeatSet(string seatSetName, int startTime, bool warpIfSkipped = false, string name = null)
public PrefabScheduleBuilder SitAtSeatSet(string seatSetName, int startTime, int durationMinutes = 60, bool warpIfSkipped = false, string name = null)
{
if (!string.IsNullOrEmpty(seatSetName))
{
_specs.Add(new SitSpec
{
SeatSetName = seatSetName,
StartTime = startTime,
DurationMinutes = durationMinutes,
WarpIfSkipped = warpIfSkipped,
Name = name
});
Expand Down
Loading