hypherionmc / simple-rpc-public

Simple-RPC Issues, wiki and data github
https://www.curseforge.com/minecraft/mc-mods/simple-discord-rpc/
14 stars 1 forks source link

[BUG] SimpleRPC and CraterLib run network requests on the main thread & constant pings when joining through direct connect #82

Closed nvb-uy closed 4 months ago

nvb-uy commented 5 months ago

Describe the bug As stated in the title. When in multiplayer, both CraterLib and SimpleRPC execute update requests to the RPC server. Due to it being on the main thread it'll mess with the framerate of the game.   IMG_5654

As seen in this spark report https://spark.lucko.me/TWkGqG0Dlx - it is constantly pinging the server the player is in, which ultimately causes big lag spikes, this only seems to be happening if you access a server through direct connect.

To Reproduce Lag Spikes Steps to reproduce the behavior:

  1. Go to the multiplayer screen
  2. Join a server via direct connect
  3. You'll get constant lag spikes due to SimpleRPC pinging the server in the main thread

Expected behavior Network requests should not be done on the main thread, and the server shouldn't be constantly pinged when you join it via direct connect.

Removing both CraterLib and SimpleRPC resolves this issue.

nvb-uy commented 5 months ago

Versions:

Minecraft 1.20.1 NeoForge 47.1.99 SimpleRPC 3.2.2 CraterLib 1.0.2

nvb-uy commented 5 months ago

The spark profiler is also a spark client profiler with only ticks over 50 and profiling all threads

hypherionmc commented 5 months ago

So just to clarify, this is simple rpc doing it, and not craterlib.

The reason it pings the server, is because when doing direct connect, all the server data it needs to resolve the server variables, are missing or null, causing every single server placeholder to fail.

It's only supposed to do the ping once when you join, so that the data is filled, and then it should stop. If it continues doing it, then something else is going on internally. We've advised users in the past not to use direct connect, but to rather save the server first, because it avoids the ping.

We have a full rewrite of this mod planned, which will make it completely client side, removing all interaction with the server, because it has proven time and time again that it causes issues. With that, the ping will also be removed and placeholders that rely on this data will simply return dummy values.

hypherionmc commented 4 months ago

I'm going to merge this issue with #88 as it's caused by the same piece of code. A craterlib update will be release in the next 2 days to address the issue, but this problem does span back several versions of Simple RPC. The real cause of it has been discovered recently.

As far as well know, if only affects Direct connections and lan connections, but regardless, the piece of code is being removed with a dummy fallback