Improve custom tasmod server

[PancakeTAS]

• Implement asynchronous server
• Write packet and authentication system
• Start to update packets to new server
• Actually finish updating all packets to new server
• 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...

[ScribbleTAS]

Welp here is my review... I'll have to probably fix this myself, so it's more of a to do list for me... I will definitely rewrite the packets myself so fixing the things I pointed out will probably be enough if anyone feels inclined to fix them...

[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]

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?

[PancakeTAS]

idk what I just reviewed, there were only deletions hehe

[ScribbleTAS]

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

[PancakeTAS]

approved