Skip to content

fix(npc): bind customer deal attendance on co-op clients - #392

Open
DooDesch wants to merge 1 commit into
ifBars:stablefrom
DooDesch-Mods:fix/coop-attend-deal-owner
Open

DooDesch wants to merge 1 commit into
ifBars:stablefrom
DooDesch-Mods:fix/coop-attend-deal-owner

Conversation

@DooDesch

@DooDesch DooDesch commented Nov 3, 2026 •

Copy link
Copy Markdown

Linked issue

Fixes #391

Contributor checklist

  • I linked an issue opened before this PR, or explained an allowed exception above.
  • I checked existing issues and PRs for duplicate or overlapping work.
  • This PR addresses one focused problem or request.

Summary

Co-op clients can now complete scheduled deals with custom customers. Before, the client's copy of the customer never offered the handover.

Clients never run the custom NPC instance constructor; their NPC is a clone of the S1API template. The owners the host sets in InitializeBehaviourComponents were missing there: the "Customer attend deal" behaviour had Behaviour.beh and _customer null. The host's Enable/Activate RPCs then threw inside the behaviour, FishNet dropped the exception, and the behaviour stayed disabled on the client.

  • NPC.CreateFromClientNetworkSpawn now runs RepairBehaviourOwnership on the clone and then NPCCustomer.BindDealAttendance.
  • NPCCustomer.BindDealAttendance (internal) points the attend behaviour at the NPC's NPCBehaviour and Customer, and sets Customer._attendDealBehaviour when it is missing. Each binding it had to set is logged once per NPC: Bound deal attendance for '<id>': set customer, customer link. A missing NPCBehaviour or attend behaviour is logged as a warning.

Compatibility

  • Public/protected API: none changed. New internal NPCCustomer.BindDealAttendance.
  • Existing defaults and behavior: host unchanged. Clients set the owners that were null; behaviours that already had the right owner are left as they are.
  • Stable IDs, saves, and network payloads: unchanged. No new RPCs; the behaviour stack and its indices are the same.

Validation

Mono

Build and tests: dotnet test S1API.Tests -c MonoMelon, 862/862 passed. Not run in the Mono game.

IL2CPP

Build and tests: dotnet test S1API.Tests -c Il2CppMelon, 825/825 passed.

Runtime evidence

Schedule I 0.4.7f12, IL2CPP, MelonLoader 0.7.3, host and client on one machine (two installs, LocalLobby). Mods: S1API, Personnel (custom customers from packs) with its own client workaround switched off, a debug probe. The host books a scheduled deal for the custom customer devtest_customer_cleo and for a vanilla customer (control); the probe then reads the attend behaviour on both sides.

S1API build client cleo: owner / Customer client cleo: attend enabled, ready for handover host cleo vanilla control (both sides)
3.2.1 (release) null / null no / no yes / yes yes / yes
stable a5a7e2a null / null no / no yes / yes yes / yes
this PR set / set yes / yes yes / yes yes / yes

Client log with this PR: [NPCCustomer] Bound deal attendance for 'devtest_customer_cleo': set customer, customer link.

Documentation

None.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue where NPC customers spawned over the network might not be correctly associated with their deal attendance, helping attendance tracking work as expected. Missing or invalid attendance links are now reported for troubleshooting.

Clients get custom NPCs as clones of the S1API template and never run the
instance constructor, so the owners the host sets there were missing. On a
client the "Customer attend deal" behaviour of a custom customer had
Behaviour.beh and _customer null. The host's Enable/Activate RPCs then threw
inside the behaviour, FishNet dropped the exception, and the customer never
waited at the drop: scheduled deals offered no handover on the client.

CreateFromClientNetworkSpawn now repairs the behaviour owners of the clone and
binds the attend behaviour to the NPC's NPCBehaviour and Customer, and the
Customer to the behaviour. Each binding that had to be set is logged.
@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

Client network-spawn hydration now repairs NPC behavior ownership and binds CustomerAttendDealBehaviour when a GameObject exists. The helper handles missing components, uses runtime-specific assignment paths for IL2CPP and Mono, and logs failures. NPC finalization still runs afterward.

Changes: Client NPC deal-attendance binding

Layer / File(s) Summary
Repair deal-attendance references
S1API/Entities/NPCCustomer.cs
BindDealAttendance locates the customer and attendance behavior. It repairs references between the behavior, its owner, and the customer. IL2CPP uses direct assignments; Mono uses reflection. Missing components and failed assignments are logged.
Bind attendance during client hydration
S1API/Entities/NPC.cs
After client network-spawn hydration, the code skips binding and logs when the wrapper has no GameObject. Otherwise, it repairs behavior ownership and invokes BindDealAttendance. Exceptions are logged, and finalization still runs afterward.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

  • ifBars/S1API#384: Changes client network-spawn wrapper creation and initialization, which this PR extends with behavior ownership repair and deal-attendance binding.

Suggested reviewers: ifbars

Merge Risk: Minimal · up to a37fb

No actionable defect is established in the client deal-attendance binding change. It is mergeable after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title names the main change: binding customer deal attendance so that co-op clients can complete scheduled deals with custom customers. It follows a conventional commit format, is one sentence, an…
Description check Passed The description follows the template. It links issue #391 with 'Fixes', checks all three contributor boxes, and gives a clear summary of the cause and the fix. It covers compatibility (no public API c…
Linked Issues check Passed [#391] Requirement: on co-op clients, the custom customer's CustomerAttendDealBehaviour must have a valid owner (Behaviour.beh) and _customer, so the host's Enable/Activate RPCs do not throw, and the …
Out of Scope Changes check Passed The diff touches two files: S1API/Entities/NPC.cs (+19 lines) and S1API/Entities/NPCCustomer.cs (+82 lines).

- NPC.cs: adds the client-only repair call and the null-GameObject debug message, wrapped …
Full details: Title check

Explanation

The title names the main change: binding customer deal attendance so that co-op clients can complete scheduled deals with custom customers. It follows a conventional commit format, is one sentence, and matches the objectives, which describe the same fix in NPC.CreateFromClientNetworkSpawn and NPCCustomer.BindDealAttendance.

Full details: Description check

Explanation

The description follows the template. It links issue #391 with 'Fixes', checks all three contributor boxes, and gives a clear summary of the cause and the fix. It covers compatibility (no public API change, no ID, save, or network payload change) and gives Mono and IL2CPP test results plus runtime evidence in a table. The Documentation section says 'None', which the template allows. Minor gap: the Mono section says the game was not run, so Mono runtime behaviour is unverified. The template's Validation section asks for build, tests, and in-game scenarios, and this is partly met.

Full details: Linked Issues check

Explanation

[#391] Requirement: on co-op clients, the custom customer's CustomerAttendDealBehaviour must have a valid owner (Behaviour.beh) and _customer, so the host's Enable/Activate RPCs do not throw, and the customer is ready for handover.

Code evidence at reviewed head (a37fb41):

  • NPC.CreateFromClientNetworkSpawn calls RepairBehaviourOwnership(gameObject, GetPreferredNpcComponent(gameObject)), then NPCCustomer.BindDealAttendance(gameObject, GetSafeNpcId()). This runs on the client clone path, which was the missing owner-assignment step named in the issue.
  • NPCCustomer.BindDealAttendance sets attendance.beh to the NPC's NPCBehaviour when beh is null or points elsewhere. It sets attendance._customer when null. It sets customer._attendDealBehaviour when null. IL2CPP assigns directly; Mono uses ReflectionUtils with error logging on failure.
  • Missing NPCBehaviour or CustomerAttendDealBehaviour returns with a warning. A non-customer NPC returns early without a warning.

The fix targets the null owner and null _customer values that the issue identifies. The issue states that setting these owners after client creation fixes the problem, and this change does that at the same point. The PR reports runtime results (attend.enabled=yes, readyForHandover=yes on the client, host and vanilla control unchanged). These results are author-reported. I did not run the game. Test results reported: 862/862 Mono and 825/825 IL2CPP.

No unmet coding requirement is found.

Full details: Out of Scope Changes check

Explanation

The diff touches two files: S1API/Entities/NPC.cs (+19 lines) and S1API/Entities/NPCCustomer.cs (+82 lines).

  • NPC.cs: adds the client-only repair call and the null-GameObject debug message, wrapped in try/catch. The call is in CreateFromClientNetworkSpawn, which the fix for #391 targets. Host paths are not changed.
  • NPCCustomer.cs: adds the internal BindDealAttendance helper. It binds the attend behaviour owner, customer link, and _customer. It logs warnings and one binding message per NPC. This directly supports the #391 fix.

No public or protected API change is in the diff. No new RPC, payload, save field, or behaviour-stack index change is in the diff. The helper and its logging are necessary parts of the fix, not unrelated work.

No change is found that is unrelated to #391.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 2 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 974be913-703c-43b4-b24d-16fdcc80f500
Commits

Reviewing files that changed from the base of the PR and between a5a7e2a and a37fb41.

Files selected for processing: 2
  • S1API/Entities/NPC.cs
  • S1API/Entities/NPCCustomer.cs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Co-op clients never offer the handover for scheduled deals with custom customers

1 participant