mChatSuite not properly implemented in the Chat compoment
sl1nk3 opened this issue ยท 7 comments
Hi,
I just switched some of my server plugins to Vault and found out the mChatSuite integration is somewhat broken (or actually, not working the way it should)
I was effectively getting NPE when calling
Chat.getPlayerPrefix(); I figured out nothing was wrong on my side, and btw, using mChatSuite's API directly worked fine.
So basically the issue in my case is that mChatSuite doesn't get the chance to initialize some of its components while Vault is hooking it, so Vault gets some null values instead of the objects it wants, also you're hooking with "mChat" instead of "mChatSuite" which doesn't help.
Here's a stripped down log to show what i'm talking about:
2012-07-01 06:01:26 [INFO] Starting minecraft server version 1.2.5
2012-07-01 06:01:26 [INFO] Loading properties
2012-07-01 06:01:26 [INFO] Starting Minecraft server on *:23000
2012-07-01 06:01:26 [INFO] This server is running CraftBukkit version git-Bukkit-1.2.5-R4.0-b2222jnks (MC: 1.2.5) (Implementing API version 1.2.5-R4.0)
2012-07-01 06:01:26 [INFO] [PermissionsBukkit] Loading PermissionsBukkit v1.6
2012-07-01 06:01:26 [INFO] [mChatSuite] Loading mChatSuite v1.2.5-b216jnks-R3
2012-07-01 06:01:26 [INFO] [iConomy] Loading iConomy v7.0
2012-07-01 06:01:26 [INFO] [Vault] Enabling Vault v1.2.16-b184
2012-07-01 06:01:27 [INFO] [Vault][Economy] iConomy 6 found: Waiting
2012-07-01 06:01:27 [INFO] [Vault][Permission] PermissionsBukkit hooked.
2012-07-01 06:01:27 [INFO] [Vault][Permission] PermissionsBukkit found: Waiting
2012-07-01 06:01:27 [INFO] [Vault][Permission] SuperPermissions loaded as backup permission system.
2012-07-01 06:01:27 [INFO] [Vault][Chat] mChatSuite hooked.
2012-07-01 06:01:27 [INFO] [Vault][Chat] mChatSuite found: Waiting
2012-07-01 06:01:27 [INFO] [Vault] Enabled Version 1.2.16-b184
2012-07-01 06:01:28 [INFO] [PermissionsBukkit] Enabling PermissionsBukkit v1.6
2012-07-01 06:01:28 [INFO] [PermissionsBukkit] Enabled successfully, 0 players registered
2012-07-01 06:01:28 [INFO] [Vault][Permission] PermissionsBukkit hooked.
2012-07-01 06:01:32 [INFO] Version: 131
2012-07-01 06:01:32 [INFO] [NarrowtuxLib] v1.3.1 by [narrowtux] enabled.
2012-07-01 06:01:32 [INFO] [mChatSuite] Enabling mChatSuite v1.2.5-b216jnks-R3
2012-07-01 06:01:32 [INFO] [mChatSuite] <Plugin> Vault v1.2.16-b184 hooked!.
2012-07-01 06:01:32 [INFO] [mChatSuite] <Permissions> PermissionsBukkit v1.6 hooked!.
2012-07-01 06:01:32 [INFO] [mChatSuite] mChatSuite v1.2.5-b216jnks-R3 is enabled! [31ms]
**// HERE : Vault should hook but it doesnt.**
2012-07-01 06:01:32 [INFO] [iConomy] Enabling iConomy v7.0
2012-07-01 06:01:32 [INFO] [iConomy - April Fools] Enabled (192 ms)
2012-07-01 06:01:32 [INFO] [iConomy] Hello, I'm Nijikokun. Yes, this is an April Fools joke, but '/money top' was fixed! Enjoy :) - Rare Version!
2012-07-01 06:01:32 [INFO] [Vault][Economy] iConomy 6 hooked.
2012-07-01 06:01:32 [INFO] [iConomy - April Fools] Purged accounts with default balance.What's needed to fix this :
in Chat_mChatSuite.java, line 70, onPluginEnable(), replace "mChat" with "mChatSuite" (or the name variable) and look for the other component instead of simply "this.chat.mChat" :
public void onPluginEnable(PluginEnableEvent event) {
// Check if any of the component is null
if (mChat == null || mReader == null || mWriter == null) {
Plugin chat = plugin.getServer().getPluginManager().getPlugin(name);
if (chat != null) {
this.chat.mChat = (mChatSuite) chat;
mReader = mChat.getReader();
mWriter = mChat.getWriter();
log.info(String.format("[%s][Chat] %s hooked.", plugin.getDescription().getName(), name));
}
}
}And I also replaced every occurence of "mChat" and "mChatSuite" with the variable name which is already there.
There you go :)
names of variables actually don't make any difference. the classes that it hooks are correct and changing the names from mChat to mChatSuite isn't going to do anything different here.
If you're attempting to call methods in Vault during the start-up process there is no guarantee that it will work. You may only access the Chat API after all plugins have finished loading.
Your report doesn't contain any necessary information to determine if there is actually an issue. I need the stack trace that you're getting, and when you're trying to call the Chat API.
Uh Oh - Unfortunately, the issue is still not fixed, of course the code snippet alone isn't helping much, but I tried as much as I could to add some broken gibberish informations around that you could perhaps take the time to read, huhu.
So yeah, I'll make it short, here's a diff against master, the important line is "mChat == null || mReader == null || mWriter == null", because as I said, Vault is hooking mChatSuite inside the constructor of the Chat_mChatSuite class, and correctly gets a reference to mChat, but for some reasons (sounds like an issue with mChatSuite) it gets null for the mWriter and mReader, so you have to check for those too in onPluginEnable :
@@ -48,12 +48,12 @@
// Load Plugin in case it was loaded before
if (mChat == null) {
- Plugin chat = plugin.getServer().getPluginManager().getPlugin("mChatSuite");
+ Plugin chat = plugin.getServer().getPluginManager().getPlugin(name);
if (chat != null && chat.isEnabled()) {
mChat = (mChatSuite) chat;
mReader = mChat.getReader();
mWriter = mChat.getWriter();
- log.info(String.format("[%s][Chat] %s hooked.", plugin.getDescription().getName(), "mChatSuite"));
+ log.info(String.format("[%s][Chat] %s hooked.", plugin.getDescription().getName(), name));
}
}
}
@@ -67,25 +67,26 @@
@EventHandler(priority = EventPriority.MONITOR)
public void onPluginEnable(PluginEnableEvent event) {
- if (this.chat.mChat == null) {
- Plugin chat = plugin.getServer().getPluginManager().getPlugin("mChatSuite");
+ // Check if any of the component is null
+ if (mChat == null || mReader == null || mWriter == null) {
+ Plugin chat = plugin.getServer().getPluginManager().getPlugin(name);
if (chat != null) {
- this.chat.mChat = (mChatSuite) chat;
+ mChat = (mChatSuite) chat;
mReader = mChat.getReader();
mWriter = mChat.getWriter();
- log.info(String.format("[%s][Chat] %s hooked.", plugin.getDescription().getName(), "mChatSuite"));
+ log.info(String.format("[%s][Chat] %s hooked.", plugin.getDescription().getName(), name));
}
}
}
@EventHandler(priority = EventPriority.MONITOR)
public void onPluginDisable(PluginDisableEvent event) {
- if (this.chat.mChat != null) {
- if (event.getPlugin().getDescription().getName().equals("mChatSuite")) {
- this.chat.mChat = null;
+ if (mChat != null) {
+ if (event.getPlugin().getDescription().getName().equals(name)) {
+ mChat = null;
mReader = null;
mWriter = null;
- log.info(String.format("[%s][Chat] %s un-hooked.", plugin.getDescription().getName(), "mChatSuite"));
+ log.info(String.format("[%s][Chat] %s un-hooked.", plugin.getDescription().getName(), name));
}
}
}mkay, sorry if my first post wasn't clear enough i'm not a native english speaker, "name" is actually a readonly string variable in Chat_mChatSuite.java containing the value "mChatSuite", so - for convenience - I thought it would be fine to replace every occurence of the string with this variable, but that's not really the point of the issue.
Here's the stacktrace describing the NPE I was talking about, indeed I forgot to include it, as you can see, i'm not attempting to call Vault methods during the startup process, it's NPEing on me while executing a command :
org.bukkit.command.CommandException: Unhandled exception executing command 'my_command' in plugin myPluginkthxbye
at org.bukkit.command.PluginCommand.execute(PluginCommand.java:42)
at org.bukkit.command.SimpleCommandMap.dispatch(SimpleCommandMap.java:166)
at org.bukkit.craftbukkit.CraftServer.dispatchCommand(CraftServer.java:479)
at net.minecraft.server.NetServerHandler.handleCommand(NetServerHandler.java:821)
at net.minecraft.server.NetServerHandler.chat(NetServerHandler.java:781)
at net.minecraft.server.NetServerHandler.a(NetServerHandler.java:764)
at net.minecraft.server.Packet3Chat.handle(Packet3Chat.java:34)
at net.minecraft.server.NetworkManager.b(NetworkManager.java:229)
at net.minecraft.server.NetServerHandler.a(NetServerHandler.java:113)
at net.minecraft.server.NetworkListenThread.a(NetworkListenThread.java:78)
at net.minecraft.server.MinecraftServer.w(MinecraftServer.java:567)
at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:459)
at net.minecraft.server.ThreadServerApplication.run(SourceFile:492)
Caused by: java.lang.NullPointerException
at net.milkbowl.vault.chat.plugins.Chat_mChatSuite.getPlayerPrefix(Chat_mChatSuite.java:106)
at net.milkbowl.vault.chat.Chat.getPlayerPrefix(Chat.java:70)so yeah, the NPE happens because inside getPlayerPrefix...
public String getPlayerPrefix(String world, String player) {
return mReader.getPrefix(player, InfoType.USER, world);
}well inside that function, mReader is null for the reasons I tried to describe in my first post.
To make it clearer, yes Vault isn't hooking into mChatSuite correctly because in onPluginEnable, you're trying to retrieve a plugin called "mChat", which doesn't exist because it's actually called "mChatSuite" (that's why I said it would be a good idea to replace every occurrence of the string with the readonly var called name).
Now if you look back at the code snippet I posted in my first post, you should hopefully understand what I'm talking about.
your code snippet didn't do a good job of actually showing what the issue was, thanks for the report. I've fixed the issue in the latest build.
well the mReader and mWriter shouldn't ever be null if the plugin is enabled, so I will not be 'fixing' this in Vault. report it to mChatSuite developer.