Skip to content

Fix disconnect after joining, building desync, and a few crashes#2686

Merged
dartasen merged 4 commits into
SubnauticaNitrox:masterfrom
Jacqueb-1337:fix/server-entity-spawn-race-conditions
May 10, 2026
Merged

Fix disconnect after joining, building desync, and a few crashes#2686
dartasen merged 4 commits into
SubnauticaNitrox:masterfrom
Jacqueb-1337:fix/server-entity-spawn-race-conditions

Conversation

@Jacqueb-1337
Copy link
Copy Markdown
Contributor

@Jacqueb-1337 Jacqueb-1337 commented Mar 21, 2026

Second player was disconnecting ~8 seconds after joining because LiteNetLib's default 5s timeout was never overridden in release builds. Also fixed building desync when joining mid-session, a few packet processors crashing on missing entities instead of skipping, and a race condition where two players loading the same area could end up with missing entities.


TLDR

  • BatchEntitySpawner / WorldEntityManager: concurrent batch loads could race and return empty entity lists; switched to ConcurrentDictionary<Lazy> to deduplicate
  • EntityDestroyedPacketProcessor: was sending the destruction packet back to the sender instead of other players
  • PlayerHeldItemChangedProcessor, ToggleLightsProcessor, Items.Dropped: were throwing on missing entities/players; now early-return
  • BuildingHandler.InitializeOperations: no longer overwrites a local op ID with a stale server snapshot
  • BuildingDesyncWarningProcessor: now resets LocalOperations alongside LastOperationId
  • BuildingManager.UpdateBase: both failure paths now call NotifyPlayerDesync
  • LiteNetLibServer / LiteNetLibClient: added #else DisconnectTimeout = 30000 for release builds

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 21, 2026

Test Results

251 tests  ±0   250 ✅ ±0   8s ⏱️ -1s
  1 suites ±0     1 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit dfee52c. ± Comparison against base commit 5dfb5c3.

♻️ This comment has been updated with latest results.

@Jacqueb-1337 Jacqueb-1337 changed the title fix: server race conditions, client packet guards, building desync, and disconnect timeout Fix disconnect after joining, building desync, and a few crashes Mar 21, 2026
Copy link
Copy Markdown
Collaborator

@Measurity Measurity left a comment

Choose a reason for hiding this comment

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

Hi thanks for the work.

Can you rebase from master branch so duplicate changes aren't in the PR diff?

(This probably happened because your previous PR was squashed, instead of merged, into master.)

@Measurity Measurity added this to the 1.9 milestone Mar 21, 2026
@Jacqueb-1337 Jacqueb-1337 force-pushed the fix/server-entity-spawn-race-conditions branch from 5a1eb21 to 0215669 Compare March 21, 2026 12:13
@Jacqueb-1337
Copy link
Copy Markdown
Contributor Author

Whoops, sorry about that. Forgot to sync after the merge. Rebased now.

@Jacqueb-1337
Copy link
Copy Markdown
Contributor Author

@Measurity anything else you'd like me to change?

@Measurity
Copy link
Copy Markdown
Collaborator

yes please. See #2686 (comment)

Comment thread Nitrox.Server.Subnautica/Models/Communication/LiteNetLibServer.cs
@Jacqueb-1337 Jacqueb-1337 force-pushed the fix/server-entity-spawn-race-conditions branch from 0215669 to 0aaa203 Compare March 26, 2026 17:31
@Jacqueb-1337
Copy link
Copy Markdown
Contributor Author

Done!

…esync + client packet guards

- LiteNetLibServer/Client: set DisconnectTimeout=30000ms in release (was default 5000ms)
  Prevents false disconnects ~8s after initial sync completes when post-sync
  game loading briefly stalls LiteNetLib ping delivery.

- BuildingManager.UpdateBase: call NotifyPlayerDesync on ghost/base entity not
  found paths (previously dropped packet silently instead of warning client)

- BuildingDesyncWarningProcessor: reset LocalOperations=0 on desync warning so
  next build uses correct serverOp+1 operation ID

- BuildingHandler.InitializeOperations: only call ResetToId when tracker is
  behind, preventing stale join-time snapshot from clobbering a tracker already
  advanced by relayed packets during the sync wait window

- PlayerHeldItemChangedProcessor: guard against packet arriving before
  RemotePlayerInitialSyncProcessor runs (was throwing OptionalEmptyException)

- ToggleLightsProcessor: guard with TryGetObjectFrom instead of RequireObjectFrom
  (was throwing on missing object)

- Items.Dropped: null guard PrefabIdentifier.ClassId before use (was NullRef
  causing silent dropped-item desync)
@Jacqueb-1337 Jacqueb-1337 force-pushed the fix/server-entity-spawn-race-conditions branch from 0aaa203 to 2e83938 Compare March 26, 2026 17:35
@Jacqueb-1337
Copy link
Copy Markdown
Contributor Author

@Measurity I'm now realizing I made conflicting changes to the branch instead of just accepting the review with your changes. I apologize, I'm still a bit new to git. Where would you like for me to go from here?

Comment thread NitroxClient/Communication/Packets/Processors/PlayerHeldItemChangedProcessor.cs Outdated
@Measurity
Copy link
Copy Markdown
Collaborator

@Measurity I'm now realizing I made conflicting changes to the branch instead of just accepting the review with your changes. I apologize, I'm still a bit new to git. Where would you like for me to go from here?

That's fine. As long as the code is right I don't mind the method to get there. Continue as you prefer.

@dartasen dartasen requested a review from tornac1234 April 4, 2026 18:13
@dartasen
Copy link
Copy Markdown
Member

dartasen commented Apr 6, 2026

Did you test that InGame ?

@Jacqueb-1337
Copy link
Copy Markdown
Contributor Author

Did you test that InGame ?

I did indeed. Still had a couple issues, but couldn't tell if they were related or if it was something else.

@CherrieTheShifter

This comment was marked as spam.

Copy link
Copy Markdown
Collaborator

@Coding-Hen Coding-Hen left a comment

Choose a reason for hiding this comment

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

LGTM however I think we still want to do the opPlayer change, though there is an argument it's not feature effecting but more code clarity

@Measurity Measurity force-pushed the fix/server-entity-spawn-race-conditions branch 2 times, most recently from 49a7428 to dfee52c Compare May 10, 2026 00:42
Copy link
Copy Markdown
Collaborator

@Coding-Hen Coding-Hen left a comment

Choose a reason for hiding this comment

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

LGTM

@dartasen dartasen merged commit a2e1cdc into SubnauticaNitrox:master May 10, 2026
3 checks passed
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.

7 participants