Bstn1802 / AutoReconnect

Minecraft fabric mod which automatically reconnects the client to the last known server after disconnection
https://www.curseforge.com/minecraft/mc-mods/autoreconnect
GNU Lesser General Public License v3.0
29 stars 36 forks source link

1.19.3 Support #38

Closed cunhar closed 1 year ago

cunhar commented 1 year ago

Hello,

Do you know what is wrong with the 1.19.3 and could I help update it. I really like your mod and have been using it for years to keep a server bot working. Without this mod we lose connection too often.

TechPro424 commented 1 year ago

Yes please update it

TechPro424 commented 1 year ago

@Bstn1802 publish 1.19.3 to CF please

Bstn1802 commented 1 year ago

I found an issue while testing and I manged to fix it quickly but it's not very clean, so I'll try to improve that and hope that I don't find more issues

TechPro424 commented 1 year ago

I found an issue while testing and I manged to fix it quickly but it's not very clean, so I'll try to improve that and hope that I don't find more issues

What was the issue and what fix did you implement?

Bstn1802 commented 1 year ago

Clicking the cancel button on the disconnect screens (which would cancel the countdown, remove the cancel button and enlargen the reconnect button) would also trigger the reconnect button (since that is right below it after that) I currently fixed that by disabling the reconnect button and re-enabling it after a delay, but it's currently just a Thread with a sleep(100) in it xD You don't happen to know if there's something fabric is providing to schedule something for the next tick? I think that would be exactly what I need. I think the events are handled differently now, maybe due to the new ui elements and that's why putting the reconnect button over the cancel button while clicking it also triggers that, but it would not if that happens in the next tick (or at least the re-enabling part) because I think events as well as pretty much everything else is handled per tick, not entirely sure, but I think so. EDIT: Now using AutoReconnect.EXECUTOR_SERVICE (exposed it) and schedule re-enabling with a delay of 10ms, which already seems sufficient, but raises the question if events are really tick based and what is really the issue behind it. Will do further investigation and testing alongside to hopefully find no more issues.

TechPro424 commented 1 year ago

Clicking the cancel button on the disconnect screens (which would cancel the countdown, remove the cancel button and enlargen the reconnect button) would also trigger the reconnect button (since that is right below it after that) I currently fixed that by disabling the reconnect button and re-enabling it after a delay, but it's currently just a Thread with a sleep(100) in it xD You don't happen to know if there's something fabric is providing to schedule something for the next tick? I think that would be exactly what I need. I think the events are handled differently now, maybe due to the new ui elements and that's why putting the reconnect button over the cancel button while clicking it also triggers that, but it would not if that happens in the next tick (or at least the re-enabling part) because I think events as well as pretty much everything else is handled per tick, not entirely sure, but I think so. EDIT: Now using AutoReconnect.EXECUTOR_SERVICE (exposed it) and schedule re-enabling with a delay of 10ms, which already seems sufficient, but raises the question if events are really tick based and what is really the issue behind it. Will do further investigation and testing alongside to hopefully find no more issues.

👍 Please keep me updated!

You don't happen to know if there's something fabric is providing to schedule something for the next tick?

I don't know, but I will search to see if it can be done in Fabric

TechPro424 commented 1 year ago

You don't happen to know if there's something fabric is providing to schedule something for the next tick?

https://discord.com/channels/507304429255393322/523633816078647296/1083014200814751755 (use !!timer command) in the Fabric Discord

Fabric itself does not include APIs to schedule something in the future.

DO NOT use threads or java.util.Timer. (This can cause a crash!) Instead:

  • If you are making a block do something in the future: world.scheduleBlockTick + override scheduledTick in your Block.
  • If you are making a custom tickable stuff (usually block entity/entity) do something in the future/periodically: see below, but instead of Mixin just implement yourself
  • If you are making a vanilla tickable thing (world, server, etc) do something in the future/periodically: use the following mixin.
@Mixin(StuffToTick.class) // ServerWorld, MinecraftServer, etc
public class StuffTimer implements StuffTimerAccess {
   @Unique
   private long ticksUntilSomething;

   @Inject(method = "tick", at = @At("TAIL"))
   private void onTick(CallbackInfo ci) { // Fix parameters as needed
       if (--this.ticksUntilSomething == 0L) {
           doSomething();
           // If you want to repeat this, reset ticksUntilSomething here.
       }
   }

   @Override
   public void yourmod_setTimer(long ticksUntilSomething) {
       this.ticksUntilSomething = ticksUntilSomething;
   }
}
public interface StuffTimerAccess {
   void yourmod_setTimer(long ticksUntilSomething);
}

Usage:

MinecraftServer server;
((StuffTimerAccess) server).yourmod_setTimer(100L); // do something after 100 ticks

Maybe this will help you out

TechPro424 commented 1 year ago

@Bstn1802 Any updates on this?

Bstn1802 commented 1 year ago

Things weren't working correctly for some reason, kind of frustrating and I haven't had the time yet to fix it. Either I get to do that in the near future or I might just release it with that quick fix which is not as bad but not good either, it should work tho. If there's no other issues it could actually work

TechPro424 commented 1 year ago

Things weren't working correctly for some reason, kind of frustrating and I haven't had the time yet to fix it. Either I get to do that in the near future or I might just release it with that quick fix which is not as bad but not good either, it should work tho. If there's no other issues it could actually work

Even in the new PR? If so, can you please create a branch on GH which contains your quick fix so I can understand the problem better?

TechPro424 commented 1 year ago

Things weren't working correctly for some reason, kind of frustrating and I haven't had the time yet to fix it. Either I get to do that in the near future or I might just release it with that quick fix which is not as bad but not good either, it should work tho. If there's no other issues it could actually work

Even in the new PR? If so, can you please create a branch on GH which contains your quick fix so I can understand the problem better?

@Bstn1802

TechPro424 commented 1 year ago

Things weren't working correctly for some reason, kind of frustrating and I haven't had the time yet to fix it. Either I get to do that in the near future or I might just release it with that quick fix which is not as bad but not good either, it should work tho. If there's no other issues it could actually work

Even in the new PR? If so, can you please create a branch on GH which contains your quick fix so I can understand the problem better?

@Bstn1802

(Also maybe we should make a new issue for 1.19.4, opening one)

Bstn1802 commented 1 year ago

@TechPro424 I totally forgot I didn't make a release for 1.19.3 at all, thought I at least made one with that quick fix I had. Sorry I haven't answered to any of the messages here. I am now working on the 1.20 release, just merged the PR, I think I might just skipp 1.19.3/4 then. Also browsing through open issues and came across this one too, I accidentally cleared my notifications some time ago, usually leave everything there as kind of a todo list. Anyways, for 1.20 it doesn't even seem to be a problem, at least it works fine and I don't see any changes regarding the cancel button in the PR I just merged and I apparently never actually pushed the one I had, which was to just delay the removal of the cancel button by like 100ms or so, so that it wouldn't trigger the reconnect button, that would otherwise be extended below it instantly. Apparently event handling has been changed again. Maybe mojang saw that as an issue too.