PacketDistributor crashes when given a custom Level implementation

[MehVahdJukaar]

Basically this.

Assumption is that a Level impl that does NOT return a ServerChunk cache (which would be hard to do for custom fake levels impls) IS a ClientLevel which is not the case.
It should instead check if level is client side and just throw of that is true.

Tested on neoforge 21.1.57

[AterAnimAvis]

It uses the ServerChunkCache to actually send the packet to the correct targets.
The wording may not be completely accurate in the case of passing in a fake level but it is correct for normal usage.

So are you asking for the method to no-op on fake levels?

[MehVahdJukaar]

yes it should only throw if passed a non client side level. Right now it crashes for any level that does not return a ServerChunkCache

[AterAnimAvis]

Calling the method with a Level that doesn't contain a ServerChunkCache is incorrect, and something you shouldn't be doing, we prehaps could update the wording of the exception, but removing it (or limiting it to solely client side levels) sounds like rather a counter-intuitive thing to do.

[MehVahdJukaar]

I am not directly doing it. I am passing a fake level around and something (from mods) is eventually calling a send packet after they check level.isClientSide which is completely reasonable.
What is wrong here is the assumption that a level that does not returns a ServerChunkCache is a client level as my post says. Method should no op there instead. A server level WILL always return a ServerChunkCache but the opposite is not true

[Shadows-of-Fire]

Could you show your relevant custom Level implementation for this case? Does it not have access to the server to supply the ServerChunkCache? (since it sounds like it sets isClient to false).

[Shadows-of-Fire]

Based on the current context, I think the following logic is more appropriate, but I'm intrigued by this custom server level implementation which apparently also has a custom chunk source implementation behind it.

public static void sendToPlayersTrackingEntity(Entity entity, CustomPacketPayload payload, CustomPacketPayload... payloads) {
    if (entity.level().isClientSide) {
        throw new IllegalStateException("Cannot send clientbound payloads on the client");
    } else if (entity.level().getChunkSource() instanceof ServerChunkCache chunkCache) {
        chunkCache.broadcast(entity, makeClientboundPacket(payload, payloads));
    }
    // Silently ignore custom Level implementations which may not return ServerChunkCache.
}

[MehVahdJukaar]

This is my level impl. Just returns an empty chunk source https://github.com/MehVahdJukaar/Moonlight/blob/1.20/common/src/main/java/net/mehvahdjukaar/moonlight/core/misc/FakeLevel.java

[neoforged-releases]

🚀 This issue has been resolved in NeoForge version 21.3.44-beta, as part of #1707.