Add Sit schedule action and duration support#52
Conversation
Track and emit sit actions for NPC prefabs and add configurable duration to Sit specs. NPCPrefabBuilder now counts SitSpec instances and ensures a corresponding prefab action (NPCEvent_Sit). SitSpec gained a DurationMinutes property and sets action.Duration (defaulting to 60 minutes when not positive); StartTime doc was clarified to 24-hour format. PrefabScheduleBuilder.SitAtSeatSet was updated to accept a durationMinutes parameter and pass it into the SitSpec.
📝 WalkthroughWalkthroughThe changes introduce configurable duration for NPC sit actions. A new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@S1API/Entities/Schedule/ActionSpecs/SitSpec.cs`:
- Around line 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).
In `@S1API/Entities/Schedule/NPCScheduleBuilder.cs`:
- Around line 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.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ca515e71-a739-42db-8bda-a7de2c52a6e6
📒 Files selected for processing (3)
S1API/Entities/NPCPrefabBuilder.csS1API/Entities/Schedule/ActionSpecs/SitSpec.csS1API/Entities/Schedule/NPCScheduleBuilder.cs
| /// <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; } |
There was a problem hiding this comment.
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.
| /// <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).
| /// <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> |
There was a problem hiding this comment.
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.
2 participants
Track and emit sit actions for NPC prefabs and add configurable duration to Sit specs. NPCPrefabBuilder now counts SitSpec instances and ensures a corresponding prefab action (NPCEvent_Sit). SitSpec gained a DurationMinutes property and sets action.Duration (defaulting to 60 minutes when not positive); StartTime doc was clarified to 24-hour format. PrefabScheduleBuilder.SitAtSeatSet was updated to accept a durationMinutes parameter and pass it into the SitSpec.
Release Notes
SitSpecinstances and ensuring a correspondingNPCEvent_Sitprefab action is createdDurationMinutesproperty onSitSpec, with a default duration of 60 minutes when a non-positive value is providedPrefabScheduleBuilder.SitAtSeatSet()method signature to accept adurationMinutesparameter (defaults to 60)SitSpec.StartTimedocumentation to specify 24-hour format input