Moving players from config state to play state causes an error

[zedphi]

Stack trace

Server Log containing the errors:
https://paste.gg/p/anonymous/fb749d8c53cb422ea335f69a2ef75fac

Plugin and Datapack List

Actions to reproduce (if known)

1. Run the command /to-config as a player
2. On the console, run either startConfig or unconfig command

Paper version

[21:49:30 INFO]: This server is running Paper version 1.21.1-120-master@57c75a4 (2024-10-09T21:11:07Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are running the latest version

Other

Here's the full testplugin code:

package com.zedphi.testplugin;

import com.mojang.brigadier.Command;
import io.papermc.paper.command.brigadier.Commands;
import io.papermc.paper.plugin.lifecycle.event.types.LifecycleEvents;
import net.minecraft.server.MinecraftServer;
import net.minecraft.server.network.ServerConfigurationPacketListenerImpl;
import net.minecraft.server.network.ServerGamePacketListenerImpl;
import org.bukkit.craftbukkit.entity.CraftPlayer;
import org.bukkit.entity.Player;
import org.bukkit.plugin.java.JavaPlugin;

public class TestPlugin extends JavaPlugin {

    @Override
    public void onLoad(){
        getLifecycleManager().registerEventHandler(LifecycleEvents.COMMANDS.newHandler(event -> {
            var registrar = event.registrar();

            registrar.register(Commands.literal("to-config")
                                       .executes(sourceStack -> {
                                           switch (sourceStack.getSource().getSender()){
                                               case Player player -> switchToConfig(player);
                                               default -> throw new IllegalStateException("Unexpected value: " + sourceStack.getSource().getSender());
                                           }

                                           return Command.SINGLE_SUCCESS;
                                       })
                                       .build());

            registrar.register(Commands.literal("startConfig")
                                       .executes(sourceStack -> {
                                           startConfig();
                                           return Command.SINGLE_SUCCESS;
                                       })
                                       .build());

            registrar.register(Commands.literal("unconfig")
                                       .executes(sourceStack -> {
                                           unconfig();
                                           return Command.SINGLE_SUCCESS;
                                       })
                                       .build());

        }));
    }

    private void switchToConfig(Player player){
        ServerGamePacketListenerImpl gameListener = ((CraftPlayer) player).getHandle().connection;
        gameListener.switchToConfig();
    }

    private void startConfig(){
        MinecraftServer.getServer().getConnection().getConnections().forEach(connection -> {
            if (connection.getPacketListener() instanceof ServerConfigurationPacketListenerImpl configListener){
                configListener.startConfiguration();
            }
        });
    }

    private void unconfig(){
        MinecraftServer.getServer().getConnection().getConnections().forEach(connection -> {
            if (connection.getPacketListener() instanceof ServerConfigurationPacketListenerImpl configListener){
                configListener.returnToWorld();
            }
        });
    }

}

[lynxplay]

In my opinion, this is a wont-fix, at least for now. We actively did not expose any API for this yet as it is known to be rather broken on spigot and hence paper given the fact the ServerPlayer instance is re-used and is initialized early on the server side for the respective events.

Messing with internals like this is unsupported for a reason and, until someone finds the time to correctly implement this (which would be a rather large effort from my first glance) I don't think it is worth considering this a bug on our issue tracker. At best a feature request in our discussion section for proper API support during the configuration stage.

So more input on my opinion would be great tho, maybe this is an easy fix I am simply not seeing.

[electronicboy]

I think that this is generally just hard fork territory. We could maybe forcefully re-init stuff and stray further away from vanilla handling here, but that just sounds like an investment into code debt, which is better off spent elsewhere, such as fixing the actual underlying cause of this once we hard fork