Switch to websockets#133
Conversation
|
In terms of progress, I have been focusing on the client, but I have been battling perfectionism and scope creep. As you can see, the changes also include a refactor to the authentication process. But I have stashes that include changes to the config and the connection gui to better enforce contracts (eg: that sync addresses MUST be prefixed with |
The point of all this hullabaloo is to prevent class-file collision. I believe Nexum uses the same websocket library? And who knows about other mods. The reason for the library is that java.net.http's websocket client is just too finicky.
Basically, it became too much of a hassle. While I do think it's preferable to use Java's standard library, it's nonetheless too simple and too idiosyncratic. For example, the each listener method takes a websocket parameter, implying an ability to use the same listener instance to manage multiple websockets, except that the interface provides no means to identify the passed-in websocket (such as its URI) without doing some nasty reflection. Nor does it provide a means to store relevant data onto the websocket instance (eg: the SyncClient or equivalent). This could *potentially* be solved with mixins, but that's iffy and likely will not work since all the jdk internal classes are likely loaded before the mixin application process even happens.
Recomputing each sync connection on every chunk load was definitely a choice xD
|
I believe it's approaching readiness for merging. Any testing and feedback would be appeciated. |
|
I've been unable to connect using the latest client and server builds together. The server doesn't even show a connect attempt, and I can't figure out why the client just seems to be voiding its attempt. |
|
When you switched the server over to websockets, you set the path to |
|
Yep, I even disabled it on the server and tried again without it. |
| "test": "true", | ||
| "start": "node -r source-map-support/register dist/main.js", | ||
| "start:dev": "node --experimental-transform-types --inspect src/main.ts" | ||
| "start:dev": "node --inspect -r source-map-support/register dist/main.js" |
There was a problem hiding this comment.
the typescript config already disables any non ES native features, it is identical
There was a problem hiding this comment.
Hmmm, I switched it back because it randomly started refusing to start. No stacktrace or anything. I switched the start:dev script back and it worked. Odd.
I think I literally fell asleep while writing this...
|
At this point I feel comfortable with merging it: there are some consequential changes I still need to make because of this, but they're outside the scope of this PR lest it become a near whole-mod refactor PR. |
2 participants
This PR intends to replace the TCP connection with a websocket connection, which has various benefits like deferring framing to the websocket protocol, deferring encryption to TLS, etc.