Dynmap-Forge/Fabric

Dynmap-Forge/Fabric

888k Downloads

[Bug] Forge mod throws invalid error on player join that should be suppressed.

TheNuclei opened this issue ยท 12 comments

commented

Issue Description: Dynmap 3.1-beta-2-forge is missing server only code from forge to clear clients from showing "Incompatible FML modded server" when a server has dynmap installed. Dynmap functions wonderfully running on the valhelsia2 1.15.2 server pack, though 1 biome seems to not render correctly that's an insignificant issue. The Valhelsia team advised that this needs to be implemented for server side only mods so as not to throw an error on clients: https://mcforge.readthedocs.io/en/1.15.x/concepts/sides/#writing-one-sided-mods

  • Dynmap Version: 3.1-beta-2-forge
  • Server Version: forge 1.15.2 build
  • Pastebin of Configuration.txt: https://pastebin.com/UT2EunqQ
  • Server Host (if applicable): Selfhosted
  • Pastebin of crashlogs or other relevant logs: No relevant logs
  • Other Relevant Data/Screenshots: No Relevant data
  • Steps to Replicate: Install dynmap on any forge 1.15.2 server

[x] I have looked at all other issues and this is not a duplicate
[x] I have been able to replicate this

commented

Wait, so the valhelsia team has included dynmap in the client side modpack? I'm not disagreeing that this simple check should be in place but all the dynmap documentation (admittedly a bit out of date and hard to navigate) makes it clear this is a server side plugin/mod only.

I'll look into the docs and see if there is an easy way to add this to the repo

commented

Oh no, they have not included it in the client side modpack. I installed it on my server in addition to the standard valhelsia2 mods as a separate addition. Because it's in the server end and not on the client end without the "server side only" code set, it causes the modchecker on the client side to say there's a mod incompatability. At least that's my understanding of it.

I only brought up valhelsia as one of their team mentioned where to find the code when I asked them if there was a way to disable the FML incompatability error.

I updated my post to make it more clear that I've installed it on the server only.

commented

Ah, that makes sense. I'll let @mikeprimm comment as to why/why not that isn't already implemented, there may be a valid reason. If not the solution looks to be a one or two line of code change so that hopefully will be an easy patch

commented

The mod is server side only - installing it in a client mod pack is unsupported. Addling logic to not throw errors when in the client is pointless as the mod is not going to work. The mod is marked as server side only, and clients can attach without the mod without problem.

commented

It throws an error when a client without this mod joins a server with this mod

commented

Log spam

commented

Only on player joins as far as I know

commented

Log spam, or something that actually matters?

commented

Just did it on 1.15.2 and 1.16.1 - no problem

commented

If you are talking about this:

[23:36:39] [Netty Server IO #1/ERROR] [minecraft/ArgumentTypes]: Could not serialize net.minecraftforge.server.command.ModIdArgument@364a9d89 (class net.minecraftforge.server.command.ModIdArgument) - will not be sent to client!
[23:36:39] [Netty Server IO #1/ERROR] [minecraft/ArgumentTypes]: Could not serialize net.minecraftforge.server.command.EnumArgument@68be9c16 (class net.minecraftforge.server.command.EnumArgument) - will not be sent to client!

I'm seeing this on a pristine Forge 1.15.2 during client login - no Dynmap, no other mods at all - The classes in question are actually Forge custom parameters for the Forge specific commands (like 'mods') that seem to not have the needed serializers - nothing to do with Dynmap. And the 'one sided' stuff being talked about, as far as code handling, is for client-only one sided - the existing code already handles declaring itself as server only, allowing the clients to log in without having the corresponding mods - nobody would be able to log in if that were not already in place, without dynmap being in their client (and that isn't the case).

commented

OK - I've added the sidedness code for the DISPLAYTEST, which should help with the client looking compatible with the server pre-connection (that is, during the version check on the servers menu). It's a cosmetic thing, since it never blocked attempting to log in, but it should be tidy.

commented

Since mike added the code to suppress the error in beta-3 I'm going to close this