Skip to content

Conversation

@Pospelove
Copy link
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

@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.

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.

1 participant