Dynmap-Forge/Fabric

Dynmap-Forge/Fabric

1M Downloads

Dynmap does not disable properly if using wrong platform

Thorinwasher opened this issue ยท 8 comments

commented

Issue Description: Dynmap does not disable properly, when on the wrong version of bukkit forks

  • Dynmap Version: 3.0<version<3.6
  • Server Version: 1.20.1
  • Steps to Replicate: Run dynmap 3.5 or lower for a unsupported platform, and have another plugin with dynmap as a dependency,

Sorry for ruining your template. Doing this in DynmapPlugin#onEnable should work right? (this is really an issue in 3.6 as well, what I noticed):

       if (helper == null) {
            Log.info("Dynmap is disabled (unsupported platform)");
            this.setEnabled(false); // Added this line
            return;
        }

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

commented

To clarify the issue: Dynmap refuses to load if run on an unrecognized platform/version. It says it has been disabled in the console, but isn't actually set to disabled. As such, from an add-on plugin's point of view, Dynmap claims it's available while in reality it isn't. This causes any add-on plugins to produce a NullPointerException stack trace when executing getMarkerAPI() because the Dynmap core variable this.core is null. Properly marking the plugin as disabled as shown in the issue description would fix this problem.

commented

we only support pure fabric, paper, spigot and forge, others are not supported and might error out and cause issues like you mentioned.

commented

This is an issue for any paper/spigot version that at the moment is not supported by dynmap.

commented

we only support pure fabric, paper, spigot and forge, others are not supported and might error out and cause issues like you mentioned.

This happens on pure fabric, paper, spigot and forge if the version string is higher than the versions hard-coded into Dynmap. If you are confused by your own error message, you should probably clarify it for whether it's the platform or version that Dynmap complains about, because in this case, the version is what Dynmap doesn't like, not the platform.
The message was generated on 1.20.1 Paper builds downloaded from papermc.io with Dynmap 3.5.

commented

That's cus those values don't match, that version of dynmap won't work on that platform version.

commented

Is this issue unclear or something?

Let me repeat: the issue is that dynmap does not disable itself in a scenario where it should be disabled. This messes up the behavior for all other plugins using dynmap as a dependency.

I have provided a simple solution, you really just need to copy and paste that and you're done.

Doing this small modification on dynmap makes dynmap more reliable to use for other developers and for users.

commented

If it's that easy do a pull request, but it's in a unsupported scenario so idk if it gets added.

commented

Okay, will do then