Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow calling JavaPlugin#getCommand during onEnable for Paper plugins #11914

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Disallow calling JavaPlugin#getCommand during onEnable for Paper plugins #11914

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
43e439e
Disallow calling JavaPlugin#getCommand during onEnable for Paper plugins
NoahvdAa Jan 5, 2025
b0bb828
Comment cleanup
NoahvdAa Jan 5, 2025
729fe47
forgot that's a thing
NoahvdAa Jan 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 33 additions & 29 deletions paper-api/src/main/java/org/bukkit/plugin/java/JavaPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.bukkit.plugin.PluginBase;
import org.bukkit.plugin.PluginDescriptionFile;
import org.bukkit.plugin.PluginLoader;
import org.bukkit.plugin.PluginLogger;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

Expand All @@ -41,30 +40,27 @@ public abstract class JavaPlugin extends PluginBase {
private Server server = null;
private File file = null;
private PluginDescriptionFile description = null;
private io.papermc.paper.plugin.configuration.PluginMeta pluginMeta = null; // Paper
private io.papermc.paper.plugin.configuration.PluginMeta pluginMeta = null;
private File dataFolder = null;
private ClassLoader classLoader = null;
private boolean naggable = true;
private FileConfiguration newConfig = null;
private File configFile = null;
private Logger logger = null; // Paper - PluginLogger -> Logger
// Paper start - lifecycle events
private Logger logger = null;
@SuppressWarnings("deprecation")
private final io.papermc.paper.plugin.lifecycle.event.LifecycleEventManager<org.bukkit.plugin.Plugin> lifecycleEventManager = org.bukkit.Bukkit.getUnsafe().createPluginLifecycleEventManager(this, () -> this.allowsLifecycleRegistration);
private boolean allowsLifecycleRegistration = true;
// Paper end
private boolean isBeingEnabled = false;

public JavaPlugin() {
// Paper start
if (this.getClass().getClassLoader() instanceof io.papermc.paper.plugin.provider.classloader.ConfiguredPluginClassLoader configuredPluginClassLoader) {
configuredPluginClassLoader.init(this);
} else {
throw new IllegalStateException("JavaPlugin requires to be created by a valid classloader.");
}
// Paper end
}

@Deprecated(forRemoval = true) // Paper
@Deprecated(forRemoval = true)
protected JavaPlugin(@NotNull final JavaPluginLoader loader, @NotNull final PluginDescriptionFile description, @NotNull final File dataFolder, @NotNull final File file) {
final ClassLoader classLoader = this.getClass().getClassLoader();
if (classLoader instanceof PluginClassLoader) {
Expand Down Expand Up @@ -94,7 +90,7 @@ public final File getDataFolder() {
*/
@NotNull
@Override
@Deprecated(forRemoval = true) // Paper
@Deprecated(forRemoval = true)
public final PluginLoader getPluginLoader() {
return loader;
}
Expand All @@ -111,8 +107,7 @@ public final Server getServer() {
}

/**
* Returns a value indicating whether or not this plugin is currently
* enabled
* Returns a value indicating whether this plugin is currently enabled
*
* @return true if this plugin is enabled, otherwise false
*/
Expand Down Expand Up @@ -278,22 +273,25 @@ protected final ClassLoader getClassLoader() {
*
* @param enabled true if enabled, otherwise false
*/
@org.jetbrains.annotations.ApiStatus.Internal // Paper
public final void setEnabled(final boolean enabled) { // Paper
@org.jetbrains.annotations.ApiStatus.Internal
public final void setEnabled(final boolean enabled) {
if (isEnabled != enabled) {
isEnabled = enabled;

if (isEnabled) {
try { // Paper - lifecycle events
onEnable();
} finally { this.allowsLifecycleRegistration = false; } // Paper - lifecycle events
this.isBeingEnabled = true;
try {
onEnable();
} finally {
this.allowsLifecycleRegistration = false;
this.isBeingEnabled = false;
}
} else {
onDisable();
}
}
}

// Paper start
private static class DummyPluginLoaderImplHolder {
private static final PluginLoader INSTANCE = net.kyori.adventure.util.Services.service(PluginLoader.class)
.orElseThrow();
Expand All @@ -303,16 +301,15 @@ public final void init(@NotNull PluginLoader loader, @NotNull Server server, @No
this.pluginMeta = description;
}
public final void init(@NotNull Server server, @NotNull PluginDescriptionFile description, @NotNull File dataFolder, @NotNull File file, @NotNull ClassLoader classLoader, @Nullable io.papermc.paper.plugin.configuration.PluginMeta configuration, @NotNull Logger logger) {
// Paper end
this.loader = DummyPluginLoaderImplHolder.INSTANCE; // Paper
this.loader = DummyPluginLoaderImplHolder.INSTANCE;
this.server = server;
this.file = file;
this.description = description;
this.dataFolder = dataFolder;
this.classLoader = classLoader;
this.configFile = new File(dataFolder, "config.yml");
this.pluginMeta = configuration; // Paper
this.logger = logger; // Paper
this.pluginMeta = configuration;
this.logger = logger;
}

/**
Expand All @@ -339,9 +336,18 @@ public List<String> onTabComplete(@NotNull CommandSender sender, @NotNull Comman
*
* @param name name or alias of the command
* @return the plugin command if found, otherwise null
* @throws UnsupportedOperationException if this plugin is a paper plugin and the method is called in {@link #onEnable()}
*/
@Nullable
public PluginCommand getCommand(@NotNull String name) {
if (this.isBeingEnabled && !(pluginMeta instanceof PluginDescriptionFile)) {
throw new UnsupportedOperationException("""
You are trying to call JavaPlugin#getCommand on a Paper plugin during startup:
you are probably trying to get a command you tried to define in paper-plugin.yml.
Comment on lines +345 to +346
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these first lines really needed? I feel like the last two are enough or am I wrong?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to be extra clear, since the majority of people asking about this in paper-dev seem to be new to plugin development (or development in general), so I wanted to make sure they go to exactly the right place.
That being said, the stacktrace will also show the right location, so maybe it's indeed too verbose?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the 2 lines basically say the same.
Maybe something like this?

Paper plugins do not support YAML-based command declarations!
You are probably trying to get a command via JavaPlugin#getCommand that you defined in paper-plugin.yml.
This method of command declaration is not supported when using Paper plugins.
Please check the documentation for more information on how to define commands in Paper plugins: https://docs.papermc.io/paper/dev/getting-started/paper-plugins#commands

Also do you guys rename Paper plugins to Lifecycle plugins? Maybe reflect that already?

RE:

That being said, the stacktrace will also show the right location, so maybe it's indeed too verbose?

Without wanting to make assumptions i think that "new to plugin development (or development in general)" wont know how to read a stacktrace. So we should not consider that as a factor here. As you said you want to be extra clear and I think being a bit more verboste makes sense.

Paper plugins do not support YAML-based command declarations!
Please check the documentation for more information on how to define commands in Paper plugins: https://docs.papermc.io/paper/dev/getting-started/paper-plugins#commands
""");
}
String alias = name.toLowerCase(Locale.ROOT);
PluginCommand command = getServer().getPluginCommand(alias);

Expand Down Expand Up @@ -429,10 +435,10 @@ public static <T extends JavaPlugin> T getPlugin(@NotNull Class<T> clazz) {
throw new IllegalArgumentException(clazz + " does not extend " + JavaPlugin.class);
}
final ClassLoader cl = clazz.getClassLoader();
if (!(cl instanceof io.papermc.paper.plugin.provider.classloader.ConfiguredPluginClassLoader configuredPluginClassLoader)) { // Paper
throw new IllegalArgumentException(clazz + " is not initialized by a " + io.papermc.paper.plugin.provider.classloader.ConfiguredPluginClassLoader.class); // Paper
if (!(cl instanceof io.papermc.paper.plugin.provider.classloader.ConfiguredPluginClassLoader configuredPluginClassLoader)) {
throw new IllegalArgumentException(clazz + " is not initialized by a " + io.papermc.paper.plugin.provider.classloader.ConfiguredPluginClassLoader.class);
}
JavaPlugin plugin = configuredPluginClassLoader.getPlugin(); // Paper
JavaPlugin plugin = configuredPluginClassLoader.getPlugin();
if (plugin == null) {
throw new IllegalStateException("Cannot get plugin for " + clazz + " from a static initializer");
}
Expand All @@ -455,20 +461,18 @@ public static <T extends JavaPlugin> T getPlugin(@NotNull Class<T> clazz) {
public static JavaPlugin getProvidingPlugin(@NotNull Class<?> clazz) {
Preconditions.checkArgument(clazz != null, "Null class cannot have a plugin");
final ClassLoader cl = clazz.getClassLoader();
if (!(cl instanceof io.papermc.paper.plugin.provider.classloader.ConfiguredPluginClassLoader configuredPluginClassLoader)) { // Paper
throw new IllegalArgumentException(clazz + " is not provided by a " + io.papermc.paper.plugin.provider.classloader.ConfiguredPluginClassLoader.class); // Paper
if (!(cl instanceof io.papermc.paper.plugin.provider.classloader.ConfiguredPluginClassLoader configuredPluginClassLoader)) {
throw new IllegalArgumentException(clazz + " is not provided by a " + io.papermc.paper.plugin.provider.classloader.ConfiguredPluginClassLoader.class);
}
JavaPlugin plugin = configuredPluginClassLoader.getPlugin(); // Paper
JavaPlugin plugin = configuredPluginClassLoader.getPlugin();
if (plugin == null) {
throw new IllegalStateException("Cannot get plugin for " + clazz + " from a static initializer");
}
return plugin;
}

// Paper start - lifecycle events
@Override
public final io.papermc.paper.plugin.lifecycle.event.@NotNull LifecycleEventManager<org.bukkit.plugin.Plugin> getLifecycleManager() {
return this.lifecycleEventManager;
}
// Paper end - lifecycle events
}
Loading