Completely fixed trigger()

[Ecconia]

Did anyone ever use trigger?

Following PR fixes almost everything not event-specific in the whole trigger system.
It allows the usage of trigger again.

Fixes done:
Fixing 4 inconsistent events (one causing a NPE).
Fixing EventBuilder which was also incompatible with all events except one.
-> Preventing not useful spamy developer debug on proper usage.
Rewriting ManualTrigger to support sending serverwide events without binding them first.

[Ecconia]

This PR has been created in collaboration with @Pieter12345
Sadly we seem to have slightly different opinions on how to solve the EventBuilder issue (and on PR's).
Ofc, I prefer my solution, since it does not require checking extends but instead fixes the 3 broken events - as long as everybody continues to create events properly it will work in future.

[Ecconia]

Additional things which will break if not fixed:
There are some events expecting to support trigger() but actually not implement the _instantiate method.

player_move player_teleport player_portal_travel vehicle_move

These would trigger the, in this PR prevented debug message, warning that no _instantiate method exists.

[Ecconia]

Has issue with one wrong import, issue resolved, lets ask travis for more issues.

[Pieter12345]

Given that the code works, I like the following changes:

• Added some "extends MCPlayerEvent" to interfaces where applicable.
• Removed static warmup() call, so methods are only cached when they are needed.
• Changed an error message, which is good, but could be improved by adding a test which ensures that every event implementation has an _instantiate method.
• Changed ManualTrigger(), which if it works as expected should fix the having-a-bind-requirement of trigger().
• Updated trigger() documentation to match the new ManualTrigger() implementation.

The one thing I do not like is:

• Changed the _instantiate code to return a BukkitMCEvent (extends BindableEvent) instead of a Bukkit Event implementation (extends nothing from CH).

Having the _instantiate method as it is ensures that some Bukkit Event object was instantiated and passed to the corresponding BukkitMCEvent class (and therefore indirectly ensures that some event can be broadcasted server-wide, as it has to have a Bukkit event object). It also disallows some _instantiate method to instantiate events from a different type, even though that would be very poor programming from the developer that writes that (or a copy-paste error perhaps). An argument against, on the other hand, is that making the _instantiate method return a BukkitMCEvent does not require reflection to pass the Bukkit Event to the BukkitMCEvent constructor, which is more efficient.

@LadyCailin and @PseudoKnight , this _instantiate design choice is something I'd like to hear your opinions about. If we can all agree on something, @Ecconia and I can figure out how to make our PR's compatible for merging.

[Ecconia]

• I agree on adding a compile test, instead of adding a runtime warning for missing _instantiate methods.
• Else I would be fine with your choice regards the return type of _instantiate.
  All, except one Event using _instantiate, currently return a BukkitMC*Event.
  Returning a Bukkit event just means to undo the code removal from me and adjusting all _instantiate methods.
  This moves the wrapping of BukkitMC*Events from _instantiate methods to the EventBuilder.

[LadyCailin]

<ramble>
So, before diving into the solution, it's useful to think about what the actual problem is, so let's step back and think this though.

The trigger() method is designed to trigger a synthesized event. In order to do this, we need to ultimately instantiate a new instance of the underlying event. The problem is, that we must of course instantiate a concrete instance of a class, (i.e. org.bukkit.event.player.PlayerJoinEvent), not a MCPlayerJoinEvent (or a BukkitMCPlayerJoinEvent for that matter). But in the core code, we actually only have an instance of MCPlayerJoinEvent, and we need to abstractly get ahold of the PlayerJoinEvent. We can rely on BukkitMCPlayerJoinEvent to do the conversion from CH abstraction layer to Bukkit terms, that's ok, so actually we just need a concrete reference to BukkitMCPlayerJoinEvent. But even still we need to have access to a static method in BukkitMCPlayerJoinEvent, which will return the new instance, or at least it needs to be able to return a builder for it. If we return a builder, that moves the problem one layer up, which might be useful anyways, but we still need to solve the original problem: We need to construct a new instance of an object, for which the abstract code does not know the signature required to construct it (or even what class it should instantiate), and we do not have concrete instances, we only have static methods. And: We would like to construct the code in such a way that if we screw it up, we get a compile error.
</ramble>

So, in general, this problem can probably be solved by modeling after the dependency injection pattern. In DI, you are able to have singleton instances. Unlike a traditional DI framework, we have class discovery, which makes instantiating instances way easier. So if we have a builder in BukkitMCPlayerJoinEvent, which is instantiated with a no-arg constructor, then we have a list of concrete builders for each abstract event subtype. These build method in these builders should extend a common build method which accepts a CArray, and converts that to a new PlayerJoinEvent (no need for MCPlayerJoinEvent to get involved here, I think) The methods for obtaining the builder in the first place will all have the same signature (just a different return type) so this can easily be checked as a unit test or compile error, to ensure that all event subtypes have this static method defined.

interface ConcreteEventBuilder<T extends ...Object Maybe?> {
    T build(CArray input);
}

interface BukkitConcreteEventBuilder<T extends Event> extends ConcreteEventBuilder<T>{ ... }

public static class BukkitMCPlayerJoinEvent ... {
    public static ConcreteEventBuilder<PlayerJoinEvent> () {
        return new BukkitConcreteEventBuilder<PlayerJoinEvent>() {
            PlayerJoinEvent build(CArray array) {
                return new PlayerJoinEvent(array, values, here);
            }
        }
    }
}

Then, in the BukkitAbstractEventMixin, you get the Event.

I think this will work. Any thoughts?

[LadyCailin]

End users do not know what a CREIllegalArgumentException is, it is an IllegalArgumentException.

[Ecconia]

Changed.