Improve custom tasmod server

[PancakeTAS]

• [x] Implement asynchronous server
• [x] Write packet and authentication system
• [x] Start to update packets to new server
• [x] Actually finish updating all packets to new server
• [x] Write new connection configuration: WONTFIX until game can launch again

[PancakeTAS]

here is the tasmod server as promised. I've done half of the packets but this process is way to difficult for me cuz I have no idea what I'm doing. I've left you eclipse templates in discord and you can look at the code for examples, it should be self explanatory. You might need to rewrite packets with nbt components to not have nbt components, or wrap a ByteBuf (Unpooled.buffer i believe) around it - at that point just write a util.

To contribute to this pull request you'll have to pullrequest enhancements/betterserver on my fork.

[PancakeTAS]

Also FRICK that's a lot of merge conflicts.

[PancakeTAS]

Quick guide on the packet system: You can send a message to all clients using TASmod.server.writeAll. You can send a message to the server using TASmodClient.client.write. If you want to send a message to one specific client, you can get the client by it's player uuid using TASmod.server.getClient and then use it's write method.

For the first two you can use the template "write" and "writeAll" in the eclipse file, it puts a nice error logging block around it for you, and makes it easier to create the packets. It looks like this:

try {
    // packet ${index}: 
    TASmodClient.client.write(ByteBuffer.allocate(${cursor}).putInt(${index}));
} catch (Exception e) {
    TASmod.LOGGER.error("Unable to send packet to server: {}", e);
}

This is where you change "index" to a new packet index counting up from the latest one in Client.java. Then the cursor will jump to allocate(), this is where you calculate how many bytes this packet uses. That would be 4 bytes for an integer (being the id itself) and then add whatever else you need. Pretty simple. Then add a comment to the preplaced comment block describing the packet

To handle the packet go into Client.java and find the serverside/clientside register method and put "handle" in there. It's pretty self explanatory again, it should look like this:

// packet ${index}:
this.handlers.put(${index}, buf -> {
    ${cursor}
});

Put your handling to where the cursor is after entering the index and put your handling stuff in there.

Here you can decide to keep it in the Client class and make it super long... or make a method (NOT A NEW CLASS!) in the class it will act in.

You can see why this is painful lol...

[PancakeTAS]

I left my reviews too. Before you go rework all of this I would like to try one more code structuring idea I wanted to for a while now. I'll address both comments in my commits probably tomorrow.

[PancakeTAS]

Absolutely no idea if this works, won't be able to test until the game launches again

[PancakeTAS]

your turn

[ScribbleTAS]

Alright, as it's tradition one person does something, the other rewrites it to their needs.

And since this is mostly my project for once, I will employ some dark arts to make my life easier:

⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⡀⢀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀
⠀⠀⠀⠀⠀⢀⣠⣴⠾⠟⠛⠛⠙⠛⠻⠷⣦⣄⡀⠀⠀⠀⠀⠀
⠀⠀⠀⢀⣴⣿⣿⣄⠀⠀⠀⠀⠀⠀⠀⠀⣠⣿⣿⣦⡀⠀⠀⠀
⠀⠀⣰⡿⠃⠘⣿⡙⠷⣦⣀⠀⠀⢀⣴⠿⢋⣿⠃⠘⢿⣆⠀⠀
⠀⣰⡿⠁⠀⠀⢹⣇⠀⠈⢙⣷⣾⡛⠁⠀⣼⠏⠀⠀⠈⢿⣇⠀
⠀⣿⠃⠀⠀⠀⠀⢿⣄⣴⠟⠉⠈⠻⢶⣴⡟⠀⠀⠀⠀⠘⣿⡄
⢸⡟⠀⠀⠀⠀⣠⡾⣯ Builders ⣿⠿⣦⣀⠀⠀⠀⢿⡇
⠸⣇⠀⣀⣴⠟⠉⠀⢻⡆⠀⠀⠀⠀⣼⠇⠀⠈⠻⢷⣤⡀⣸⡇
⠀⣿⡾⠿⠷⠶⠶⠶⠾⣿⠶⠶⠶⢶⡿⠶⠶⠶⠶⠶⠿⣿⣿⠀
⠀⠘⣿⡄⠀⠀⠀⠀⠀⠸⣇⠀⠀⣾⠃⠀⠀⠀⠀⠀⢠⣿⠃⠀
⠀⠀⠘⢿⣤⡀⠀⠀⠀⠀⢻⡄⢰⡟⠀⠀⠀⠀⠀⣤⡿⠃⠀⠀
⠀⠀⠀⠀⠙⢷⣦⣄⠀⠀⠘⣷⣿⠃⠀⠀⣠⣴⡾⠋⠀⠀⠀⠀
⠀⠀⠀⠀⠀⠀⠈⠙⠻⠷⠶⣿⣿⠶⠿⠟⠋⠁⠀⠀⠀⠀⠀

Yes you heard that right, I'm using them for constructing Bytbuffers.
I don't like having to worry about the SecureList and whatnot so I made this tiny builder to help with my endeavour.

Interfaces and lamdas for packet handlers?

Thats right, I am trying to make a system that supports both!
So you have all the advantages in one system and nobody complains...

Interface method

public class TickSyncClient implements ClientPacketHandler{

    public static final AtomicBoolean shouldTick = new AtomicBoolean(true);

    @Override
    public PacketID[] getAcceptedPacketIDs() {
        return new TASmodPackets[] {TASmodPackets.TICKSYNC};
    }

    @Override
    public void onClientPacket(PacketID id, ByteBuffer buf, UUID clientID) {
        TASmodPackets pid = (TASmodPackets) id;

        switch (pid){
            case TICKSYNC:  // Only for demo purposes, you don't need a switch case here, since it's only one packet...
                shouldTick.set(true);
                break;
            default:
                break;
        }
    }

Implement ClientPacketHandler and/or ServerPacketHandler, register it in the TASmod init method with PacketHandlerRegistry.register() and you are good to go. Supports multiple packethandlers in one method, if you specify more IDs in getAcceptedPacketIDs().

My goal was to group similar packet handlers into one method, instead of spreading all functionality across multiple classes and methods. Hopefully helps with code readability

Lambda method

public enum TASmodPackets implements PacketID {
    TICKSYNC,
    TICKRATE_SET,
    TICKRATE_ADVANCE,
    SAVESTATE_LOAD,
    SAVESTATE_SAVE,
    /**
     * <p>CLIENT ONLY.
     * <p>Opens or closes the savestate screen on the client
     */
    SAVESTATE_SCREEN(Side.CLIENT, (buf, clientID) -> {
        Minecraft mc = Minecraft.getMinecraft();
        if (!(mc.currentScreen instanceof GuiSavestateSavingScreen))
            mc.displayGuiScreen(new GuiSavestateSavingScreen());
        else
            mc.displayGuiScreen(null);
    });
}

Instead of making 2 enums for client and server, I'll only put one enum here and specifying the side.

Also the lambda method takes precedence over the interface method, specifying something in TICKSYNC will overwrite the class I set earlier.

Next up: Fixing all errors

[PancakeTAS]

Thats right, I am trying to make a system that supports both!

Selling my implementation as if it was yours huh? noted 😂

Yes you heard that right, I'm using them for constructing Bytbuffers.

Oh just make sure to use @Builder from lombok, it does all the work for you ^ﻌ^ฅ♡

[ScribbleTAS]

Selling my implementation as if it was yours huh? noted 😂

Right... I've credited you in the commit message and forgot it here... This was more of a comment for you...

[ScribbleTAS]

Alright all errors are fixed and the mod is buildable again...

However now I need to go back and fix all issues with the packets and maybe refactor some networking paths. While I'm at it, I can try to do #148 Especially Savestates need a bit of a rewrite

• Ticksync
• Savestates
• Implement Tickratechanger
• Refactor packets so they can also be called in the main menu
• TASmod.getServerInstance() also needs a bit of rethinking.

[ScribbleTAS]

Note to self: Replace OpenGuiEvents with a scheduler and have that run when the main menu opens

[ScribbleTAS]

Note to self:
Implement tickratechanger packets

[ScribbleTAS]

Welcome to another episode of

Scribbles cursed code snippets

In todays episode we have:

Switch case statements inside switch case statements

    public void onClientPacket(PacketID id, ByteBuffer buf, UUID clientID) {
        TASmodPackets packet = (TASmodPackets) id;

        switch (packet) {
        case TICKRATE_CHANGE:
            float tickrate = TASmodBufferBuilder.readFloat(buf);
            changeClientTickrate(tickrate);
            break;
        case TICKRATE_ADVANCE:
            advanceClientTick();
            break;
        case TICKRATE_ZERO:
            TickratePauseState state = TASmodBufferBuilder.readTickratePauseState(buf);

            switch (state) {
            case PAUSE:
                pauseClientGame(true);
                break;
            case UNPAUSE:
                pauseClientGame(false);
                break;
            case TOGGLE:
                togglePauseClient();
            default:
                break;
            }
            break;

        default:
            throw new PacketNotImplementedException(packet, this.getClass());
        }
    }

[PancakeTAS]

please indent the case and default one further from the switch itself 🙏 Also if I see one undocumented switch case I will fire you

[ScribbleTAS]

please indent the case and default one further from the switch itself 🙏 Also if I see one undocumented switch case I will fire you

But the Enums are documented ._. And funny enough, this is the default eclipse indentation... I just did Ctrl+F on it and eclipse doesn't indent it...

[PancakeTAS]

https://github.com/PancakeTAS/BackgroundLed/blob/develop/arduino.ino

i want this kind of documentation /j

[ScribbleTAS]

Wut? Does that mean remove all custom exceptions? Should I add more? Earlier you were completely fine with them, pointing out the serial version uuid... And I want your reason as to why... Too many classes again? They have helped me already, having a contructor for ypur messages is really nice...

The custom exceptions will stay... I also have some for savestatemod and others

[PancakeTAS]

Should I add more?

wtf lol

Nah I recently changed my mind again. I don't like custom exceptions because they have no use other than to save a single sentence comment. If they would provide different data in the stacktrace sure, but these are just... eh

[ScribbleTAS]

Nah I recently changed my mind again. I don't like custom exceptions because they have no use other than to save a single sentence comment. If they would provide different data in the stacktrace sure, but these are just... eh

Now I'm intrigued of how to provide different data in the stacktrace, because for networking this does sound useful...

[ScribbleTAS]

wtf?? so many better ways to do this for example wrap a Object/DataOutputStream around a ByteArrayOutputStream and write 100
lines less :)

@PancakeTAS Alright what do you mean with that... Before I do that wrong again, I'll just ask you. Because I admit, I don't like my solution either, but so far nothing else worked

[PancakeTAS]

i will take a look today... if I don't forget

[ScribbleTAS]

Right, now the issue is that the uuid we have in the main menu is not necessarily the uuid that we have on the server... So we may connect to the custom server with a different uuid than to the vanilla server, which throws things off...

[PancakeTAS]

Which UUID?

[ScribbleTAS]

So, the custom server starts in the main menu and "authenticates" with the session uuid... for indev it uses a random uuid. Once you join a minecraft server, the entityplayermp gets a random uuid as well... So when i try to send something to a client for savestates, i can't because the player uuid is not the same as the client uuid

[ScribbleTAS]

Note to self:

• [x] Move TASStateServer to PlaybackControllerServer
• [x] Reorder EventHandler and PacketHandler registering in TASmod.class and TASmodClient.class to be split

[ScribbleTAS]

Write new connection configuration: WONTFIX until game can launch again

@PancakeTAS What do you mean by that?

[PancakeTAS]

well what we discussed - connecting to the server at game launch using a configuration file

[ScribbleTAS]

Ah so it was that

[ScribbleTAS]

Testing

Here is my todo list for testing everything. Also add trace logging

Server

• [x] TickSyncServer

• PlaybackControllerServer

  • [x] Statesync
  • [x] StatesyncInitial (Needs multiplayer server)

• SavestateHandlerServer

  • [x] Savestate
  • [x] Loadstate

• TickrateChangerServer

  • [x] Change
  • [x] Unpause
  • [x] Toggle

• KillTheRNGHandler

  • [x] KilltheRNG

    Client

• PlaybackControllerClient

  • [x] Save
  • [x] Load
  • [x] Play
  • [x] Record
  • [x] Fullplay
  • [x] Fullrecord
  • [x] Restartandplay
  • [x] Playuntil
  • [x] Clearinputs
  • [x] Teleport

• SavestateHandlerClient

  • [x] PlayerMotion
  • [x] Screen
  • [x] Unloadchunks

[ScribbleTAS]

Todo Prevent sending packets, when no connection is open

[ScribbleTAS]

I should invest some time in a proper disconnect system that checks when a socket is closed... Also a disconnect packet would be quite neat I think.

[ScribbleTAS]

@PancakeTAS I added a timeout system to this mess that currently checks every 10 seconds, if a client is still alive... Maybe I'll make that tick based, so that tickrate 0 and low tickrates will not cause timeouts...

I made an onClose callback that removes clients from the list of clients in Server.java... It's also for Events and whatnot...

And disconnect() actually sends a packet to the server now that tells the server to close... Tested it and disconnecting and connecting seems to be working!

Maybe you can take a look at this rq as the connectivity expert

[ScribbleTAS]

Note to self: Remove ticksync connection when in main menu and connected to singleplayer

Add keepalive packet...

[ScribbleTAS]

Note to self: Check out Ticksync and when it's enabled again to see if it's correct. Don't forget that!

Afterwards implement SyncStatePacket... When sending that to the server I got an error idk why...

[ScribbleTAS]

Welcome to Scribble losing his mind again doing very dumb mistakes.
I'm getting this error:

[14:34:11][Thread-19/ERROR] (Common/THROWING[ EXCEPTION ]) Throwing
java.nio.BufferUnderflowException: null
    at java.nio.Buffer.nextGetIndex(Buffer.java:510) ~[?:1.8.0_372]
    at java.nio.HeapByteBuffer.getShort(HeapByteBuffer.java:323) ~[?:1.8.0_372]
    at com.minecrafttas.tasmod.networking.TASmodBufferBuilder.readTASState(TASmodBufferBuilder.java:81) ~[main/:?]
    at com.minecrafttas.tasmod.playback.PlaybackControllerServer.onServerPacket(PlaybackControllerServer.java:73) ~[main/:?]

Great. This tells me that the packet I am sending inbetween the server and client has no data and it fails to read that data.

So you start checking every instance where you send that particular packet. Nope, everything is correct here.
Then you notice that when you join singleplayer it works. But if you quit to menu and go to singleplayer again, then it fails.

Hmmm... Then you find out that the packet is sent twice? Then you check the connection. Nope only one connection.
Then you finally find the issue:

public void onServerInit(MinecraftServer server) {
    playbackControllerServer=new PlaybackControllerServer();
    PacketHandlerRegistry.register(playbackControllerServer);
}

So everytime I join singleplayer, I register a new packethandler.
I am checking if that object is already registered in the registry

if (!REGISTRY.contains(handler)) {
    REGISTRY.add(handler);
} else {
    Common.LOGGER.warn("Trying to register packet handler {}, but it is already registered!", handler.getClass().getName());
}

but due to the new keyword, this is a new object so it happily registers 2 instances of the same class

[PancakeTAS]

buffer underflow exception doesn't necessarily mean there's no data at all, it just means that in the buffer, the position has exceeded the limit of the buffer. In other words, you read everything from the buffer and now it is empty. That's why you see me call flip() (or something like that) when reading or sending a packet, it resets the position and puts the limit to the current position. Naturally if you try to read the same buffer twice without calling flip, you'll underflow the buffer (though idk why it's called underflow).

Anyways I'll use your small little hiccup as an argument for why registries are a terrible idea :3

By the way, I'm pretty sure the contains() method called equals() on every object, to see if they are equal. If you want to prevent bugs like this in the future, you can override that method in the class and implement your own is equal check.

[ScribbleTAS]

I'm pretty sure the contains() method called equals() on every object

Nice idea, however, the object that I'm registering is an interface, meaning I'd have to override all implementations or do some abstract class bs to get this done the same way.

Instead I opted for this

private static boolean containsClass(PacketHandlerBase handler) {
    for(PacketHandlerBase packethandler : REGISTRY) {
        if(packethandler.getClass().equals(handler.getClass())) {
            return true;
        }
    }
    return false;
}

And low and behold, look I've done the same thing twice

[ScribbleTAS]

Note to self:

• Make firing events better.
• Savestates take 3 ticks to load? -> Let the world tick?
• You can't unpause from tickrate 0 in gui screens?

[ScribbleTAS]

@PancakeTAS How many months did this take? idk, but pls review