Recently, I made a major change to one of MGLib's subsystems to fix a semi-rare bug. Said bug occurred often enough to make it a point of concern, but just rare enough to make it difficult to determine the cause or really anything about it. I couldn't figure out why it was happening, and to this day, I still don't know.

I suppose I should elaborate on what the bug was. MGLib has a feature where it can override the vanilla Bukkit PlayerDeathEvent (which is ordinarily uncancellable) and instead call its own custom event so that the player effectively stays alive and the minigame using MGLib can do what it wishes instead. For example, TTT utilizes it to effectively turn the player into a sort of "ghost," where they become invisible, can fly, and can't interact with the world. However, because death events aren't cancellable, it has to use a workaround.

Before the rewrite, MGLib didn't even allow the death event to happen. It listened to the EntityDamageEvent, determined the amount of damage that would be dealt, and if it was greater than the player's total health, it would cancel it and throw the custom death event. This had some disadvantages though, the primary one being that Bukkit didn't include a method for getting the final damage from the event (with armor, enchantments, and potion effects taken into account) until a very recent build (CB #3095). Instead, it was only possible to retrieve the raw damage before adjustments were made. Therefore, the final damage had to be estimated, which was exceedingly difficult due to weird algorithms used by Minecraft, and, most prominently, because certain factors (such as enchantments) used random multipliers. Naturally, it was impossible to be 100% accurate using this method. While the new EntityDamageEvent#getFinalDamage() method could be used if the server supported it, the described estimation approach was required as a fallback for older (and I use that term very loosely) Craftbukkit builds.

But, I digress. The bug was that MGLib couldn't always foresee the death event. On a total of four occasions, we had a tester actually die instead of becoming a ghost. This presented a problem in that because the custom death event was never fired, the player was teleported out of the arena and into the main world's spawn, and was not removed from the round properly. There's a whole slew of other repercussions, but I won't get into those. My point is, the bug had to go, because it was in some cases game-breaking.

I suspected the bug was caused by EntityDamageEvents being fired in rapid succession resulting in a miscalculation. I should mention, by the way, that this happened even when the new damage methods were used. However, after I had added debug messages, we couldn't replicate the bug after an hour of playing. The next day, I made the decision to just rewrite the death override entirely, since it was clear that the bug wasn't being fixed any time soon if we stuck with the current approach. I also really wanted to get rid of the estimation code, since it was ugly, unreliable, and somewhat inaccurate.

The new approach involved allowing the PlayerDeathEvent to be fired, and then forcing the player to respawn. This couldn't be done with the Bukkit API, but through the use of packets, I was able to get it working after about an hour of effort. Basically, the PlayInClientCommand needed to be sent with the parameter EnumClientCommand.PERFORM_RESPAWN. This was sent, of course, after the respawn location was set to the same location the player died at. The implementation of this was rather difficult, seeing as the Bukkit safeguard on NMS and OBC classes forced me to use reflection to reference them, not to mention the protocol overhaul which came with Minecraft 1.7. This meant I had to detect which protocol version the server was using so I could fetch the correct class and provide the proper parameters. Also, reflection is just really confusing sometimes because it's inherently ambiguous. In the end, though, it simplified down to this (I'm excluding the utility classes used to get NMS classes while bypassing the safeguard):

Class<?> packetClass;
Object packet;
try {
    // attempt to get class as per new protocol
    packetClass = MGUtil.getMCClass("PacketPlayInClientCommand");
    packet = packetClass.getConstructor(MGUtil.getMCClass("EnumClientCommand"))
            .newInstance(Enum.valueOf((Class<? extends Enum>)MGUtil.getMCClass("EnumClientCommand"), "PERFORM_RESPAWN"));
}
catch (Exception ex){
    // couldn&#39;t get the 1.7 class, so the server must be using the old protocol
    // get the class as per the old protocol
    packetClass = MGUtil.getMCClass("Packet205ClientCommand");
    // the old class doesn&#39;t have an explicit constructor; only a default
    packet = packetClass.getConstructor().newInstance();
     // this is effectively the same thing as the EnumClientCommand parameter in the new protocol
    packetClass.getDeclaredField("a").set(packet, 1);
}
// get the EntityPlayer from the player involved in the event
Object nmsPlayer = MGUtil.getCraftClass("entity.CraftPlayer").getMethod("getHandle").invoke(e.getEntity(), new Object[0]);
// get the connection with which to send the packet
Object conn = nmsPlayer.getClass().getDeclaredField("playerConnection").get(nmsPlayer);
// each packet has it&#39;s own a() method in the PlayerConnection class
conn.getClass().getMethod("a", packetClass).invoke(conn, packet);

Once I got the new approach to stop throwing exceptions, it worked like a charm. In fact, I liked the hacky override better, because you would see the Minecraft menu background flash for just half a second before the player was forced to respawn. It sort of served to signify that the player had died. This was very good, seeing as testers had on more than one occasion not realized they were dead and were confused as to why they couldn't attack other players.

In the end, the new override code turned out to be worth the time and effort, because it was a more accurate and nicer-looking system. It's terribly hacky though, so I wouldn't recommend doing anything like this yourself unless you've worked with reflection before and absolutely know what you're doing. It's really stressful because of its ambiguity, not to mention you can't fix issues which would normally create a compile error until they throw an exception.