Skip to content

Conversation

@perryprog
Copy link
Contributor

About the PR

This fixes a rare edge case where if the client handles some event that depends on identity in response to that entity's spawning it would end up dying to a NRE.

Why / Balance

Bugs bad.

Technical details

IdentityEntitySlot is ensured on ComponentInit, so it's possible to have stuff that depends on identity occur while the IdentityComponent is still in its Added lifestage. This changes the nullability of the slot to accurately represent that.

This does mean there's an extreme edge case where Identity.Name/Entity can return the real identity buuuuuut I feel like that's probably fine. (As far as I know this could only really happen on an entity entering a client's PVS for the first time.)

Media

Identity still works image image

Requirements

Breaking changes

  • IdentityComponent.IdentityEntitySlot is now optional.

Changelog

wawaa

@PJBot PJBot added S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. size/S Denotes a PR that changes 10-99 lines. labels Aug 3, 2025
@perryprog perryprog added T: Bugfix Type: Bugs and/or bugfixes P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. D2: Medium Difficulty: A good amount of codebase knowledge required. A: General Interactions Area: General in-game interactions that don't relate to another area. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Aug 3, 2025
@github-actions github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Sep 23, 2025
@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link
Member

@slarticodefast slarticodefast left a comment

Choose a reason for hiding this comment

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

I went ahead and fixed the merge conflict.
Seems good to me!

@PJBot PJBot added the S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. label Oct 21, 2025
@github-actions github-actions bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Oct 21, 2025
@slarticodefast
Copy link
Member

Aaaaaah, those Heisentests 😠
4th time for sure

@slarticodefast slarticodefast added this pull request to the merge queue Oct 21, 2025
Merged via the queue into space-wizards:master with commit 5a0a984 Oct 21, 2025
13 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A: General Interactions Area: General in-game interactions that don't relate to another area. D2: Medium Difficulty: A good amount of codebase knowledge required. P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. size/S Denotes a PR that changes 10-99 lines. T: Bugfix Type: Bugs and/or bugfixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants