Skip to content

Update Java-Client to current network protocol #27

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

Merged
merged 11 commits into from
Jun 7, 2025

Conversation

mattman107
Copy link
Contributor

Updated

  1. Client protocol version
  2. List of SetPacket operations
  3. Bumped dependancy versions to latest

Added

  1. Per-message compression
  2. Basic implementation of UpdateHintPacket.

1. Client protocol version
2. List of SetPacket operations
3. Bumped dependancy versions to latest
# Added
1. Per-message compression
2. Basic implementation of UpdateHintPacket.
@mattman107 mattman107 marked this pull request as ready for review April 26, 2025 23:52
Copy link
Collaborator

@cjmang cjmang left a comment

Choose a reason for hiding this comment

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

The per message deflate looks correct based on looking at the documentation of the org.java_websocket library.

Also removed uncessary assignments of new datapackages
@cjmang
Copy link
Collaborator

cjmang commented May 5, 2025

Tested the client as of commit b9aee87 with a local dev branch of StS Mod.

Observations:

  1. StS registers a client version of 0.6.1 when connecting to AP.
  2. AP no longer issues a warning about not supporting per message compression.
  3. StS can still send/receive items.
  4. StS writes datapackages into the appropriate place on Windows. Presumably it also reads from the appropriate place on Windows.
  5. While StS was running, after having connected to AP once, I deleted the datapackage cache. When reconnecting StS did not save the datapackages from the server. I had to restart StS. I do not know yet whether this is due to StS Mod behavior or the Java Client, or both.

So overall this PR as it stands seems to work fine for StS.

Copy link
Collaborator

@cjmang cjmang left a comment

Choose a reason for hiding this comment

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

One behavior issue on windows, and a nit. You can disregard the nit if you want.

String appData = System.getenv("LOCALAPPDATA");
String userHome = System.getProperty("user.home");
if(appData == null || appData.isEmpty()) {
windowsDataPackageCache = Paths.get(userHome, "appdata","local","Archipelago","cache","datapackage");
Copy link
Collaborator

Choose a reason for hiding this comment

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

On windows the environment variable for user home is USERPROFILE.

Copy link
Contributor Author

@mattman107 mattman107 Jun 4, 2025

Choose a reason for hiding this comment

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

So this:
String userHome = System.getProperty("user.home")
needs to be this:
String userHome = System.getProperty("USERPROFILE")

Is that right?

@cjmang
Copy link
Collaborator

cjmang commented Jun 5, 2025

LGTM

@cjmang cjmang merged commit cfe649b into ArchipelagoMW:main Jun 7, 2025
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.

2 participants