search

badasintended / wthit

what the hell is that?
https://docs.bai.lol/wthit
Other
121 stars 21 forks source link

WTHIT config is not synced when Polymer is also installed (under certain conditions) #230

Closed unilock closed 11 months ago

unilock commented 1 year ago

Describe the issue

When joining a Fabric or Quilt server running behind a Velocity server proxy, WTHIT's config is not synced, leading to the client thinking the server doesn't have the mod installed at all, and thus disabling all related features.

Log output and crash report

client debug crash | server log

Additional context

This is likely due to WTHIT (or Bad Packets) attempting to send its configuration sync packets pre-GameJoinS2CPacket, which Velocity does not permit (despite it being technically possible in vanilla). Much more information can be found in this issue discussing QSL's registry sync attempting something similar: https://github.com/QuiltMC/quilted-fabric-api/issues/46

EDIT: This may also affect Forge servers, but it's not been tested. Note that Forge requires a bit more complicated setup to run behind Velocity - see here: https://github.com/adde0109/Proxy-Compatible-Forge

deirn commented 1 year ago

Do mods that use Fabric's ServerPlayConnectionEvents.JOIN to send packets, say, the Registry Sync module work fine? Fabric and Bad Packets has the same injection point for earliest packet, which is before the BRAND packet. That is after the GameJoinS2CPacket being sent to client.

unilock commented 1 year ago

Fabric's own Registry Sync module works fine with Velocity, as far as I can tell. Quilt's implementation does not (by itself), as mentioned.

Sorry, I must've misread Bad Packets' code as firing its event before the GameJoinS2CPacket is sent. The player handshake -> login -> play sequence isn't something I'm terribly familiar with.

Although the fact remains that WTHIT's config isn't being synced behind Velocity, for whatever other reason. Maybe I should try enabling packet debug logging via Log4j config?

I'm having some difficulty parsing where the injection points are for the mixins that fire Bad Packets' and Fabric API's "player join" events. Bad Packets' MixinServerGamePacketListenerImpl and Fabric API's ServerLoginNetworkHandlerMixin are the relevant classes, correct?

deirn commented 1 year ago

Fabric calls the JOIN event before the server sends the BRAND packet, here.

As Bad Packets don't support sending LOGIN packets (I don't think its possible to do so that compatible with platform impl), it sends the initial channel IDs in the PLAY stage instead. So the order of the call looks like this:

  1. Server sends GameJoinS2CPacket
  2. Client received GameJoinS2CPacket
  3. Client sends the initial channel ids
  4. Server received the initial channel ids
  5. The server "join" event called
  6. WTHIT sends its "handshake" packets (version, plugin config, blacklist)

it's called in here which is called by this mixin.

unilock commented 1 year ago

I recorded some debug logs.

When connecting directly to the backend server (without a server proxy in-between):

When connecting to the server through a server proxy (i.e. Velocity):

Hopefully you can make more sense from these than I can!

deirn commented 1 year ago

Hmm, it seems that on velocity the server doesn't receive the channel sync packet badpackets:channel_sync, and thus the callback not called...

Weird that it received the waila:version packet just fine.

unilock commented 1 year ago

I was able to get it "working" by substituting mcp/mobius/waila/Packets with an implementation that utilizes Fabric / QSL's networking API instead of bad packets, as seen in this branch (only for Quilt):
https://github.com/unilock/wthit/commit/b24d6d128e476b53ee0082de66620470f9eb3de3

I'm not completely sure what to make of it. I guess there is a difference between Fabric's (Side)PlayConnectionEvents.JOIN and bad packets' PacketSenderReadyCallback.register(Side)...?

deirn commented 1 year ago

Can you try this? badpackets-fabric-0.999-local.jar.zip

unilock commented 1 year ago

@deirn

Can you try this? (file)

Unfortunately that version of bad packets made no (apparent) difference, compared to the current officially released versions of bad packets and WTHIT. The behavior is identical to the original issue.

deirn commented 1 year ago

Can you try with only WTHIT, Fabric API, and Bad Packets? Finally went around and tested it myself, and it works fine, other than with the problem with Quilt server https://github.com/QuiltMC/quilted-fabric-api/issues/46, though I'd say it's more of Quilt's problem than mine.

Added velocity task for easier test in my fork here. Use an IDE (I use Intellij) to run it, though I need to use the gradle task for Forge.

gradlew :fabric:runServer
gradlew :runVelocity
gradlew :fabric:runClient
gradlew :quilt:runClient
gradlew :forge:runClient
unilock commented 1 year ago

Aha... OK, this is a bit embarrassing.

The latest release version of Bad Packets (0.2.1) and WTHIT (5.19.0) do work fine with a Fabric server running behind a Velocity proxy.

It seems that another mod I had installed, Polymer, was preventing the WTHIT config from being synced.

Unfortunately, Polymer is an (embedded) dependency of several mods in my non-testing modpack, and is also required to get Quilt running with Velocity at all, as mentioned in the issue on QFAPI's bug tracker that you linked.

It's likely that the issue stems from Polymer's packet "hacks" to get its own specialized registry sync working - more specifically, its "sendGameJoinBeforeSync" option, which is what makes Quilt work with Velocity. The relevant commit (to the 1.19.2 branch) for that feature can be found here: https://github.com/Patbox/polymer/commit/72d6efe09d890965325720cf7078625430b60610

(for clarity, running Quilt behind Velocity requires adding both Polymer and Patbox's quilt_velocity_patch to the Quilt server, and enabling the aforementioned config option of the former)

...That's all to say, I guess this is an incompatibility with Polymer rather than Velocity.

deirn commented 1 year ago

Alright, try this. badpackets-fabric-0.999-local.jar.zip

unilock commented 1 year ago

That appears to be working with the following mods:

Thanks! If that build of Bad Packets is ready for release, then this issue can be closed!

deirn commented 1 year ago

The change is pretty simple https://github.com/badasintended/badpackets/commit/18ec6599dfab1fdc01765fcaae4a5595cba657d7

To workaround Velocity dropping packets sent before GameJoinS2CPacket, Polymer and the Quilt patch send a fake join packet to the client. Bad Packets detects when the client receives the join packet to send the channel sync packet but fails since the server is not yet in the valid state to receive PLAY packets, and thus the callback is not called.

The workaround to the workaround (hah) is to send the sync packet after the client receives a PLAY packet (be it the brand packet or the sync from the server), which will guarantee the server is in the correct state.

unilock commented 11 months ago

Sorry, it seems I spoke too soon.

WTHIT does still work with Polymer, but when Polymer is configured to auto-host its own generated resource pack, the original symptom of this issue returns - effectively, WTHIT's config is not synced when the player joins the server.

To reproduce (with Minecraft 1.20.1):

Note that using the vanilla method to set up a static "server resource pack" (via server.properties) works fine.

(also note that this issue was never resolved on a heavily modded Quilt 1.19.2 server I'm also running, but that's probably a separate issue, as Polymer isn't auto-hosting a resource pack there)

deirn commented 11 months ago

It seems that the current workaround fail as Polymer sends a brand packet in its fake PLAY stage here. Maybe checking strictly for Bad Packets' channel sync packet will work.

deirn commented 11 months ago

Try this. badpackets-fabric-0.999-local.jar.zip

unilock commented 11 months ago

That does appear to be working!!

...on my heavily modded Quilt 1.19.2 server. The issue with Polymer's auto-generated resource pack is taking place on a 1.20.1 server - though I expect whatever changes were made would fix that as well. I'd be happy to test a 1.20.1 build of that version of Bad Packets, too, if you'd like.

deirn commented 11 months ago

Oops, missed the 1.20.1 sorry. Should also work as the code is the same on that part, but try this. badpackets-fabric-0.999-local.jar.zip

unilock commented 11 months ago

That build of Bad Packets fixed the 1.20.1 server! Thanks! Feel free to close this issue again :)