[1.16.4] SimplImpl sends packet to client instead server

[S-Spirit]

I implementing screen, where user may select option and server should be notified about it. Simple case. I found tutorial and implemented this feature uses SimpleImp. Maybe I missed something - packet from server sends to client successfully, but when I tries to use sendToServer client get message instead server.

 

My network class

public class ModNetworking {

    private static final String PROTOCOL_VERSION = "1.0";
    private static SimpleChannel INSTANCE;
    private static int ID = 0;

    public static int nextId () { return ID++; }

    public static void registerMessages() {
        INSTANCE = NetworkRegistry.newSimpleChannel(new ResourceLocation(ArtOfFood.MOD_ID, "mod_channel"),
                () -> PROTOCOL_VERSION,
                PROTOCOL_VERSION::equals,
                PROTOCOL_VERSION::equals);

        INSTANCE.messageBuilder(SSetConceptResultSlotPacket.class, nextId())
                .encoder(SSetConceptResultSlotPacket::writePacketData)
                .decoder(SSetConceptResultSlotPacket::new)
                .consumer(SSetConceptResultSlotPacket::processPacket)
                .add();

        INSTANCE.messageBuilder(CUpdateConceptPacket.class, nextId())
                .encoder(CUpdateConceptPacket::writePacketData)
                .decoder(CUpdateConceptPacket::new)
                .consumer(CUpdateConceptPacket::processPacket)
                .add();
    }

    public static void sendToClient(Object packet, ServerPlayerEntity player) {
        INSTANCE.sendTo(packet, player.connection.netManager, NetworkDirection.PLAY_TO_CLIENT);
    }

    public static void sendToServer(Object packet) {
        INSTANCE.sendToServer(packet);
    }
}

 

Problem packet is CUpdateConceptPacket:

public class CUpdateConceptPacket {

    private String conceptId;

    public CUpdateConceptPacket(String conceptId) {
        this.conceptId = conceptId;
    }

    public void processPacket(Supplier<NetworkEvent.Context> ctx) {
        PlayerEntity playerEntity = Minecraft.getInstance().player;
        if (playerEntity.openContainer instanceof IUpdateConceptPacketHandler) {
            IUpdateConceptPacketHandler handler = (IUpdateConceptPacketHandler) playerEntity.openContainer;
            handler.handleConceptUpdate(conceptId);
        }
        ctx.get().setPacketHandled(true);
    }

    public CUpdateConceptPacket(PacketBuffer buf) {
        this.readPacketData(buf);
    }

    public void readPacketData(PacketBuffer buf) {
        this.conceptId = buf.readString();
    }

    public void writePacketData(PacketBuffer buf) {
        buf.writeString(this.conceptId);
    }

    public String getConceptId() {
        return this.conceptId;
    }
}

 

Minimalistic handler:

public interface IUpdateConceptPacketHandler {

    void handleConceptUpdate(String conceptId);
}

 

This is how I call it:

@OnlyIn(Dist.CLIENT)
public void setConcept(MBConcept concept) {
    this.concept = concept;
    CUpdateConceptPacket packet = new CUpdateConceptPacket(concept.conceptId);
    ModNetworking.sendToServer(packet);
}

 

And get client call (see screen, at debug value of playerEntity is ClientPlayerEntity)

P.S. Can't find spoiler for hiding code listings. Is it available here? And ofcouse I tried to investigate Vanila examples few times but didn't get results.

Edited January 19, 2021 by S-Spirit

[S-Spirit]

Added Forge version to title. Does anyone meet this network issue?

[S-Spirit]

7 hours ago, diesieben07 said:

• Do not use OnlyIn.
• Minecraft#player is always the client player. By accessing it on a packet thats sent to the server, you are reaching across logical sides, which will just crash on dedicated servers and cause strange behavior in single player.

Thank you very much! You was right - problem was related to code Minecraft.getInstance().player. I Have read page about networking few times and missed this example of common beginners mistakes...

But how I should get opened container at server side? I added list of listeners into ModNetworking 

private static final NonNullList<WeakReference<Object>> serverListeners = NonNullList.create();

public static void addServerListener(Object obj) {
	serverListeners.add(new WeakReference(obj));
}

public static void removeServerListener(Object obj) {
    serverListeners.remove(obj);
}

public static <T> Stream<T> listeners(Class<T> listenerClass) {
    serverListeners.removeIf(r -> r.get() == null);
    return serverListeners.stream()
            .filter(r -> listenerClass.isAssignableFrom(r.get().getClass()))
            .map(r -> ((T) r.get()));
}

 

At packet processing ModNetworking provides listeners of necessary type:

public void processPacket(Supplier<NetworkEvent.Context> ctx) {
    Stream<IUpdateConceptPacketHandler> listeners = ModNetworking.listeners(IUpdateConceptPacketHandler.class);
    listeners.forEach( l -> l.handleConceptUpdate(conceptId));
    ctx.get().setPacketHandled(true);
}

 

My container add itself like a listener in constructor and remove at the finalize(). This solution works, but is it brilliant idea or ugly implementation of something, which platform already may provide to me?

 

P.S. Why you notify me to not use @OnlyIn? As I understood, it should be written everywhere, in case when method should be called only from client. Is it wrong vision?

[DaemonUmbra]

Who told you to use OnlyIn everywhere?

[S-Spirit]

38 minutes ago, diesieben07 said:

On the server NetworkEvent.Context#getSender gives you the player that sent the packet.

Thanks, got it!

 

8 hours ago, DaemonUmbra said:

Who told you to use OnlyIn everywhere?

No one. It was my own conclusion from documentation and tutorials. When you asked, I read about @OnlyIn one more time, and see - my conclusion was fully wrong. I even thinked @OnlyIn work with logical sides, but specification says - physical. So I should make a refactorng. Thanks for get my attention.