Skip to content

Commit

Permalink
Fix a bunch of issues in configuration networking
Browse files Browse the repository at this point in the history
  • Loading branch information
OroArmor committed Mar 28, 2024
1 parent 9566847 commit 9b95c66
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
@ApiStatus.Internal
public abstract class AbstractNetworkAddon<H> {
protected final GlobalReceiverRegistry<H> receiver;
protected final Logger logger;
public final Logger logger;
// A lock is used due to possible access on netty's event loops and game thread at same times such as during dynamic registration
private final ReadWriteLock lock = new ReentrantReadWriteLock();
// Sync map should be fine as there is little read write competition
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.quiltmc.qsl.networking.api.ServerConfigurationConnectionEvents;
import org.quiltmc.qsl.networking.api.ServerConfigurationNetworking;
import org.quiltmc.qsl.networking.api.ServerConfigurationTaskManager;
import org.quiltmc.qsl.networking.api.ServerPlayNetworking;
import org.quiltmc.qsl.networking.impl.NetworkingImpl;
import org.quiltmc.qsl.networking.impl.server.ServerConfigurationNetworkAddon;
import org.quiltmc.qsl.networking.impl.server.ServerNetworkingImpl;
Expand Down Expand Up @@ -108,7 +109,7 @@ private record CommonRegisterConfigurationTask(ServerConfigurationNetworkAddon a

@Override
public void start(Consumer<Packet<?>> sender) {
this.addon.sendPayload(this.addon.createRegisterPayload());
addon.sendPayload(new CommonRegisterPayload(addon.getNegotiatedVersion(), CommonRegisterPayload.PLAY_PHASE, ServerPlayNetworking.getGlobalReceivers()));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public <T extends CustomPayload> boolean handle(T payload) {
boolean handled = super.handle(payload);
if (handled && payload.id().equals(NetworkingImpl.REGISTER_CHANNEL)) {
if (((ServerConfigurationTaskManager) this.handler).getCurrentTask() instanceof SendChannelsTask) {
ServerConfigurationConnectionEvents.ADD_TASKS.invoker().onAddTasks(this.handler, this.server);
((ServerConfigurationTaskManager) this.handler).finishTask(SendChannelsTask.TYPE);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright 2021 The Quilt Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.quiltmc.qsl.networking.impl.server;

import net.minecraft.network.ServerConfigurationPacketHandler;

public interface ServerConfigurationPacketHandlerKnowingTask {
void setServerConfigurationPacketHandler(ServerConfigurationPacketHandler handler);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.quiltmc.qsl.networking.mixin;

import java.util.function.Consumer;

import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Unique;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;

import net.minecraft.network.ServerConfigurationPacketHandler;
import net.minecraft.network.configuration.ConfigurationTask;
import net.minecraft.network.configuration.JoinWorldConfigurationTask;
import net.minecraft.network.packet.Packet;

import org.quiltmc.qsl.networking.api.ServerConfigurationNetworking;
import org.quiltmc.qsl.networking.api.ServerConfigurationTaskManager;
import org.quiltmc.qsl.networking.impl.server.ServerConfigurationPacketHandlerKnowingTask;
import org.quiltmc.qsl.networking.impl.server.ServerNetworkingImpl;
import org.quiltmc.qsl.networking.mixin.accessor.ServerConfigurationPacketHandlerAccessor;

@Mixin(JoinWorldConfigurationTask.class)
public abstract class JoinWorldConfigurationTaskMixin implements ConfigurationTask, ServerConfigurationPacketHandlerKnowingTask {

@Unique
private ServerConfigurationPacketHandler handler;

@Inject(method = "start", at = @At("HEAD"), cancellable = true)
private void ensureNoTasksLeft(Consumer<Packet<?>> task, CallbackInfo ci) {
if (!((ServerConfigurationPacketHandlerAccessor) this.handler).getTasks().isEmpty()) {
// TODO: Throw this error?
// throw new RuntimeException("Not all tasks have been completed by the configuration handler!");
ServerNetworkingImpl.getAddon(handler).logger.error("Not all tasks have been completed by the configuration handler! Tasks remaining: {}", ((ServerConfigurationPacketHandlerAccessor) this.handler).getTasks());
((ServerConfigurationTaskManager) this.handler).addTask(this);
((ServerConfigurationTaskManager) this.handler).finishTask(JoinWorldConfigurationTask.TYPE);
ci.cancel();
}
}

@Override
public void setServerConfigurationPacketHandler(ServerConfigurationPacketHandler handler) {
this.handler = handler;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

import com.llamalad7.mixinextras.injector.ModifyReceiver;
import com.llamalad7.mixinextras.injector.WrapWithCondition;
import com.llamalad7.mixinextras.injector.wrapoperation.Operation;
import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation;
import org.jetbrains.annotations.Nullable;
import org.spongepowered.asm.mixin.Final;
import org.spongepowered.asm.mixin.Mixin;
Expand All @@ -31,10 +33,12 @@
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
import org.spongepowered.asm.mixin.injection.callback.LocalCapture;

import net.minecraft.network.ClientConnection;
import net.minecraft.network.ServerConfigurationPacketHandler;
import net.minecraft.network.configuration.ConfigurationTask;
import net.minecraft.network.configuration.JoinWorldConfigurationTask;
import net.minecraft.network.listener.AbstractServerPacketHandler;
import net.minecraft.network.packet.Packet;
import net.minecraft.network.packet.s2c.common.DisconnectS2CPacket;
Expand All @@ -47,6 +51,7 @@
import org.quiltmc.qsl.networking.impl.NetworkHandlerExtensions;
import org.quiltmc.qsl.networking.impl.server.SendChannelsTask;
import org.quiltmc.qsl.networking.impl.server.ServerConfigurationNetworkAddon;
import org.quiltmc.qsl.networking.impl.server.ServerConfigurationPacketHandlerKnowingTask;

// We want to apply a bit earlier than other mods which may not use us in order to prevent refCount issues
@Mixin(value = ServerConfigurationPacketHandler.class, priority = 900)
Expand Down Expand Up @@ -138,6 +143,30 @@ private Queue<ConfigurationTask> modifyTaskQueue(Queue<ConfigurationTask> origin
return originalQueue;
}

@WrapOperation(method = "startConfiguration", at = @At(value = "NEW", target = "()Lnet/minecraft/network/configuration/JoinWorldConfigurationTask;"))
private JoinWorldConfigurationTask setHandlerStart(Operation<JoinWorldConfigurationTask> constructor) {
var task = constructor.call();
((ServerConfigurationPacketHandlerKnowingTask) task).setServerConfigurationPacketHandler((ServerConfigurationPacketHandler) (Object) this);
return task;
}

@WrapOperation(method = "rejoinWorld", at = @At(value = "NEW", target = "()Lnet/minecraft/network/configuration/JoinWorldConfigurationTask;"))
private JoinWorldConfigurationTask setHandlerRejoin(Operation<JoinWorldConfigurationTask> constructor) {
var task = constructor.call();
((ServerConfigurationPacketHandlerKnowingTask) task).setServerConfigurationPacketHandler((ServerConfigurationPacketHandler) (Object) this);
return task;
}

@Inject(method = "startNextTask", at = @At(value = "INVOKE", target = "Lnet/minecraft/network/configuration/ConfigurationTask;start(Ljava/util/function/Consumer;)V"), locals = LocalCapture.CAPTURE_FAILHARD)
public void beforeTaskStart(CallbackInfo ci, ConfigurationTask task) {
this.addon.logger.debug("Starting task with type: {}", task.getType());
}

@Inject(method = "finishCurrentTask", at = @At("HEAD"))
public void beforeTaskStart(ConfigurationTask.Type type, CallbackInfo ci) {
this.addon.logger.debug("Finishing task type: {}", type);
}

@Inject(method = "onDisconnected", at = @At("HEAD"))
private void handleDisconnection(Text reason, CallbackInfo ci) {
this.addon.handleDisconnect();
Expand All @@ -155,16 +184,20 @@ public Packet<?> createDisconnectPacket(Text message) {

@Override
public void addTask(ConfigurationTask task) {
this.addon.logger.debug("Adding task with type: {}", task.getType());
this.tasks.add(task);
}

@Override
public void addPriorityTask(ConfigurationTask task) {
this.addon.logger.debug("Adding priority task with type: {}", task.getType());
this.priorityTasks.add(task);
}

@Override
public void addImmediateTask(ConfigurationTask task) {
this.addon.logger.debug("Adding immediate task with type: {}", task.getType());

if (this.immediateTask != null) {
throw new RuntimeException(
"Cannot add an immediate task of type: \"" + task.getType()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright 2021 The Quilt Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.quiltmc.qsl.networking.mixin.accessor;

import java.util.Queue;

import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.gen.Accessor;

import net.minecraft.network.ServerConfigurationPacketHandler;
import net.minecraft.network.configuration.ConfigurationTask;

@Mixin(ServerConfigurationPacketHandler.class)
public interface ServerConfigurationPacketHandlerAccessor {
@Accessor
Queue<ConfigurationTask> getTasks();
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"CustomPayloadC2SPacketMixin",
"CustomPayloadS2CPacketMixin",
"EntityTrackerEntryMixin",
"JoinWorldConfigurationTaskMixin",
"PacketBundleMixin",
"PlayerManagerMixin",
"ServerConfigurationPacketHandlerMixin",
Expand All @@ -17,6 +18,7 @@
"accessor.CustomPayloadC2SPacketAccessor",
"accessor.CustomPayloadS2CPacketAccessor",
"accessor.EntityTrackerAccessor",
"accessor.ServerConfigurationPacketHandlerAccessor",
"accessor.ServerLoginNetworkHandlerAccessor",
"accessor.ThreadedChunkManagerAccessor"
],
Expand Down

0 comments on commit 9b95c66

Please sign in to comment.