Skip to content

fix: attach NPC data before ConfigurePrefab for dealers and suppliers too - #334

Merged
ifBars merged 2 commits into
ifBars:betafrom
r-melvin:fix/supplier-dealer-npc-data-before-configure
Oct 2, 2026
Merged

ifBars merged 2 commits into
ifBars:betafrom
r-melvin:fix/supplier-dealer-npc-data-before-configure

Conversation

@r-melvin

@r-melvin r-melvin commented Oct 1, 2026 •

Copy link
Copy Markdown

Summary

A supplier or dealer NPC that calls WithVoice in ConfigurePrefab never registers. Drug Expansion's Disco Davey, for example:

[WARNING] [NPC] [S1API] Failed to pre-register NPC prefab for DiscoDavey: Could not apply S1API voice 'tyler' because the custom NPC has no framework data.

ConfigurePrefab only had a framework data object for plain NPCs. A supplier or dealer is built from a donor prefab that can have none, and its own data is created after ConfigurePrefab. WithVoice throws without data, as documented. WithIdentity and WithIcon ignored the same failure.

The change. Any prefab whose NPC component has no data object now gets one before ConfigurePrefab, for every role. It matches the component's own type (NPC.DataRoleForComponent): plain, or dealer or supplier when the donor already is one. A dealer or supplier built from a plain donor still gets its own data afterwards, and identity and voice are re-applied to it from the prefab's identity component. Plain NPCs are unchanged.

Compatibility

  • Public/protected API: none changed.
  • Existing defaults and behavior: WithVoice keeps its contract. A dealer or supplier prefab now has data during ConfigurePrefab; the final prefab is unchanged.
  • Stable IDs, saves, and network payloads: none touched.

Validation

Mono

dotnet build S1API.sln -c MonoMelon --no-restore -p:AutomateLocalDeployment=false: 0 errors, 0 warnings. dotnet test ... -c MonoMelon: 742 passed (740 on beta plus 2 new: the data kind a donor gets).

IL2CPP

dotnet build S1API.sln -c Il2CppMelon --no-restore -p:AutomateLocalDeployment=false: 0 errors, 0 warnings. dotnet test ... -c Il2CppMelon: 722 passed (720 plus 2 new).

Runtime evidence

How it was tested. Automated runs on 0.4.7f7 load a save, teleport to each custom NPC, record whether its model is shown and its network state, return to the menu and load again. They used a combined build of #332 to #339.

  • Same mods on both runtimes: IL2CPP and Mono "Alternate", with BigWillyMod, The Big Pimpin, Drug Expansion, S1MAPI and SteamNetworkLib. IL2CPP ran without Polyfill, on the game's own interop assembly.
  • IL2CPP with about 60 mods: including Polyfill, Siesta and S1UMF.
  • Saves: an early save and a late one where The Big Pimpin's intro is played through.

I also played the IL2CPP case by hand.

Documentation

None needed.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 9347dcce-f3e8-4cfd-a26d-b499500dcd86

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

Comment @coderabbitai help to get the list of available commands.

… too

ConfigurePrefab only had a framework data object for plain NPCs. A supplier or dealer built from a donor with
none threw from WithVoice ("Could not apply S1API voice 'tyler' because the custom NPC has no framework
data", Drug Expansion's Disco Davey) and never registered. Any prefab whose NPC component has no data now
gets one before ConfigurePrefab, matching the component's own type; a role's own data still replaces it
afterwards and identity and voice are re-applied to it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@r-melvin
r-melvin force-pushed the fix/supplier-dealer-npc-data-before-configure branch from 79c36e4 to 08d0edb Compare October 1, 2026 23:16
@r-melvin
r-melvin marked this pull request as ready for review October 1, 2026 23:34
@ifBars ifBars added beta A game update on the beta & alternate-beta steam branches bug Something isn't working npcs Native game NPC system labels Oct 2, 2026
Repository owner deleted a comment from diffuin Bot Oct 2, 2026
Repository owner deleted a comment from diffuin Bot Oct 2, 2026
@ifBars

ifBars commented Oct 2, 2026

Copy link
Copy Markdown
Owner

@Diffuin review plz

@diffuin

diffuin Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Diffuin review

Verdict: Approve
Confidence: Medium
Findings: none

No actionable defects found. The change attaches role-appropriate framework data before ConfigurePrefab and preserves the existing plain-NPC path. Build, test, and live-runtime claims were not independently rerun because the checkout lacks the required .NET SDK and game runtime.

No actionable findings.

Evidence and validation

Evidence inspected

  • The beta game source confirms Dealer and Supplier inherit NPC and use role-specific data objects.
  • WithVoice requires framework data, while NPCPrefabIdentity reapplies configured identity and voice to the final component.
  • The base-to-head diff changes only NPC data-role handling and adds focused role-mapping tests.

Validation performed

  • Inspected the complete base-to-head diff.
  • Inspected repository guidance and relevant NPC lifecycle code.
  • Inspected beta and alternate stripped game source.
  • Ran git diff --check; no whitespace errors were reported.

Runtime validation remaining

  • Run MonoMelon and Il2CppMelon build and test commands with the required SDK and references.
  • Independently validate registration and save/load behavior in both runtimes.
Diffuin run details
  • Provider: codex
  • Model: gpt-5.6-luna
  • Reasoning: xhigh (Luna advisor: risk (high); baseline non-trivial pull request)
  • Elapsed: 441s
  • Codex thread: 01a0fa77-0823-79f1-a365-bc82e4b85bb5

AI notice: Generated with AI assistance and not guaranteed accurate. Verify findings and plans against the current source and runtime.

@ifBars

ifBars commented Oct 2, 2026

Copy link
Copy Markdown
Owner

I pushed 787e377 to trim the comments that repeated the implementation and test. Kept the donor-role ordering explanation and XML docs. No runtime changes in this follow-up. Full builds and suites pass: 742 Mono, 722 IL2CPP.

@ifBars

ifBars commented Oct 2, 2026 •

Copy link
Copy Markdown
Owner

Tested on 0.4.7f7 Mono and IL2CPP. Plain, dealer and supplier NPCs configured WithVoice and spawned through ConfigurePrefab/OnCreated on the first load and after reloading. Dealer and supplier components and contacts initialized correctly.

CI passed. The combined build of #332, #333, #334, #336 and #337 also passed all 770 Mono and 744 IL2CPP tests. Merged into beta.

@ifBars
ifBars merged commit bd687d4 into ifBars:beta Oct 2, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beta A game update on the beta & alternate-beta steam branches bug Something isn't working npcs Native game NPC system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants