Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KH2: Reduce unnecessary packets sent/requested by the client #4035

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Mysteryem
Copy link
Contributor

What is this fixing or adding?

While connected to the game and AP server, the KH2 client would send both a Set message (for storing the current world int) and a LocationChecks message every 0.5 seconds, even if the value in the Set message was unchanged or if there were no newly checked locations to send to the server.

The Set message behaviour was identified from the PopTracker package that was listening to updates to the key via a SetNotify message, which would receive messages multiple times a second. This Set message was also requesting a reply from the server (SetReply), but not doing anything with this reply.

The LocationChecks message behaviour has only been identified by reading the code.

This patch changes the client to only send the Set message when the value to set changes and to only send the LocationChecks message when there are locations to send. The Set message also no longer requests a reply from the server.

Because the LocationChecks message is no longer always created, the finishedGame() function now reads ctx.sending directly rather than reading it from the create message.

How was this tested?

It has not been tested because I do not own KH2.

The client does not have any special handling of the SetReply messages
that would be received from the server in response to sending Set
messages with want_reply=True.
finishedGame() now reads ctx.sending directly instead of reading it from
`message[0]["locations"]`.
@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Oct 7, 2024
@Exempt-Medic Exempt-Medic added waiting-on: world-maintainer Issue/PR is waiting for feedback or approval by the maintainer of a world. is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. labels Oct 7, 2024
Copy link
Contributor

@palex00 palex00 left a comment

Choose a reason for hiding this comment

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

Hi, I was the one with the pack that detected this.

I can confirm that this change does not have the same problem anymore and sending checks, receiving checks, goaling and intended data storage stuff is still working. Bonafide fix.

@Exempt-Medic Exempt-Medic removed the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. waiting-on: world-maintainer Issue/PR is waiting for feedback or approval by the maintainer of a world.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants