Skip to content

refact(skymp5-server): introduce GetActorToSendTo to make packets routing explicit#2553

Merged
Pospelove merged 3 commits intoskyrim-multiplayer:mainfrom
Pospelove:refac
Apr 16, 2026
Merged

refact(skymp5-server): introduce GetActorToSendTo to make packets routing explicit#2553
Pospelove merged 3 commits intoskyrim-multiplayer:mainfrom
Pospelove:refac

Conversation

@Pospelove
Copy link
Copy Markdown
Contributor

@Pospelove Pospelove commented Nov 12, 2025

Important

Refactor message sending in skymp5-server by introducing GetActorToSendTo() in MpActor to handle offline actors and fix the "invisible chat" bug.

  • Behavior:
    • Introduced GetActorToSendTo() in MpActor to determine the correct actor for message sending, considering hosters.
    • Applied GetActorToSendTo().SendToUser() in ActionListener.cpp, MpActor.cpp, and MpObjectReference.cpp to fix the "invisible chat" bug.
  • FormCallbacks:
    • Added GetUserIdFn to FormCallbacks to retrieve user ID from MpActor.
  • Refactoring:
    • Replaced direct SendToUser() calls with GetActorToSendTo().SendToUser() in multiple functions across MpActor.cpp and MpObjectReference.cpp.
  • Misc:
    • Updated CreateFormCallbacks() in PartOne.cpp to include getUserId function.

This description was created by Ellipsis for fbbea00. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Copy Markdown

@ellipsis-dev ellipsis-dev Bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to fbbea00 in 1 minute and 13 seconds. Click for details.
  • Reviewed 243 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 8 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. PartOne.cpp:190
  • Draft comment:
    In SetUserActor the hoster mapping for an actor is cleared (using worldState.hosters.erase). Make sure this removal is safe against concurrent updates and that it doesn’t inadvertently remove valid hoster state.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. PartOne.cpp:703
  • Draft comment:
    The lambda for getUserId in CreateFormCallbacks returns st->UserByActor(actor). Ensure callbacks and actor pointers are always valid to prevent runtime exceptions.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. PartOne.cpp:544
  • Draft comment:
    In SetPrivateKey and SignedJS the OpenSSLSigner is attached and a signature is appended to JS code. Verify that the private key is securely handled and that the client properly parses the appended signature comment.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. PartOne.cpp:904
  • Draft comment:
    In TickDeferredMessages and packet history playback, the packet history buffer is accessed using saved offsets and lengths. Ensure that the buffer bounds are validated to avoid potential out‐of‐bounds access.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. MpObjectReference.cpp:1840
  • Draft comment:
    In SendInventoryUpdate the BitStream’s raw data is reinterpreted for sending. Consider adding extra validation on the stream data pointer and size to ensure that the data is valid when sending.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. MpObjectReference.cpp:59
  • Draft comment:
    In PreparePropertyMessage_ the code accesses base.rec to retrieve the record type. Add a safeguard in case LookupById returns a null record so that baseRecordType is handled gracefully.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. MpObjectReference.cpp:1822
  • Draft comment:
    In MoveOnGrid, objects recalc grid positions on every position update. Consider caching grid cell calculations if many objects update frequently to avoid performance bottlenecks.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
8. MpObjectReference.cpp:1182
  • Draft comment:
    In ApplyChangeForm a call to std::terminate is used if setProperty has been called already. Verify that killing the process in this scenario is intended, or consider a less drastic error handling strategy.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_2xlqVyMMf7QcaYGZ

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@Pospelove Pospelove changed the title Refac refact(skymp5-server): introduce GetActorToSendTo Apr 16, 2026
@Pospelove Pospelove changed the title refact(skymp5-server): introduce GetActorToSendTo refact(skymp5-server): introduce GetActorToSendTo to make packets routing explicit Apr 16, 2026
@Pospelove Pospelove merged commit 79ed812 into skyrim-multiplayer:main Apr 16, 2026
15 checks passed
@Pospelove Pospelove deleted the refac branch April 16, 2026 20:26
F02K pushed a commit to F02K/skymp that referenced this pull request Apr 23, 2026
F02K pushed a commit to F02K/skymp that referenced this pull request Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant