Event Question

[Soren025]

With events being registered by passing an object like so...

public class Listener {
    @subscribe
    public void onSomething(SomethingEvent event) {

    }
}

game.getEventManager().register(new Listener()));

However would it be possible / good idea to implement it such that if an instance of Class<?> were passed, then we could do this as well.

public class Listener {
    @subscribe
    public static void onSomething(SomethingEvent event) {

    }
}

game.getEventManager().register(Listener.class));

What do you think?

[maxov]

You're not even saving any characters of code, and instantiating listener classes using reflection seems pretty useless, considering it happens once. Is it really too hard to do new Listener()?

[Soren025]

A use case I had for something like this was something along the lines of this, back with bukkit where referencing player directly was never a smart thing to do.

public class PlayerRelatedThing {

    private Player player;

    @subscribe
    private static void onPlayerJoin(PlayerJoinEvent event) {
        myPlugin.getPlayerRelatedThingManager.get(enent.getPlayer()).player = event.getPlayer();
    }

    @subscribe
    private static void onPlayerQuit(PlayerQuitEvent event) {
         myPlugin.getPlayerRelatedThingManager.get(enent.getPlayer()).player = null;
    }
}

Because static methods can alter private variables of instances, something like above would be useful.

This is also under the assumption that private methods work as they did in Bukkit

[maxov]

I think you just gave a way too specific a use case for something that doesn't really seem related. Just instantiate your listener class like normal, the player will be initialized to null. Also listener methods don't have to be static, I don't know how that thought was put in your head.

EDIT: Didn't read through the full way, missed last sentence. Private methods are a feature of the Java programming language and not of any Minecraft-related plugin API.

[Soren025]

Ware did you read that I thought listener methods had to be static?

And instantiating a listener class like normal means that I would need to provide access to manipulate the player reference publicly. Something I was attempting to avoid in the above use case...

EDIT: "Didn't read through the full way, missed last sentence. Private methods are a feature of the Java programming language and not of any Minecraft-related plugin API"

I meant private methods could be registered as event handlers in bukkit...

[Soren025]

@gratimax tbh I am not satisfied with your response, seeing as how once again you gave me a "no, do it the other way" response...

How about an actual reason to shut down an idea.

And to assume I thought private methods were unique to bukkit... wtf

[maxov]

@Soren025

Ware did you read that I thought listener methods had to be static?

I didn't read it, I inferred it because you kept having static listener methods where they weren't necessary and even inconvenient. If this was the wrong assumption to make, then I'm genuinely sorry. Take the statement as a warning that you probably shouldn't have static event handlers on your event listeners.

And instantiating a listener class like normal means that I would need to provide access to manipulate the player reference publicly.

No...? The only reason that your reference has to be public in this case is because of the frankly convoluted way you're managing your player instances.

Something I was attempting to avoid in the above use case...

I don't really consider your "use case" to be a "use case" because it's really just a specific way you're dealing with events. A well-thought-out use case wouldn't be a specific way of managing players, it would be doing something that wouldn't be possible or would be extremely inconvenient if your suggestion wouldn't be accepted. So far you haven't provided anything compelling.

The Sponge API shouldn't have to cater to every single "use case" of a plugin developer because of the way they want to design things. We don't have to baby-feed you utilities because of your specific choice of design patterns(or lack thereof). As our unofficial motto goes, "We can't stop stupid but we can darn well try our best." The pattern you're using is really bizarre and isn't the best way to do it. This is mainly because of separation of concerns: your listener class shouldn't manage data for you, it's an event listener. Externalize that behavior to some class where it's actually reasonable. Otherwise your entire plugin will become convoluted and hard to maintain, especially for someone who isn't you.

Because static methods can alter private variables of instances, something like above would be useful.

Figure out how and where to instantiate listener classes for your own use case. If it's too difficult or seems unwieldy, then you're doing the separation of concerns wrong.

I wrote up some more separated classes as an example. Forgive me if it doesn't compile, it's pretty late here:

public class PlayerListener {

  private MyPlayerManager myPlayerManager;

  public PlayerListener(MyPlayerManager playerManager) {
    myPlayerManager = playerManager;
  }

  @Subscribe
  public void onPlayerJoin(PlayerJoinEvent event) {
    myPlayerManager.trackPlayer(event.getPlayer());
  }

  @Subscribe
  public void onPlayerQuit(PlayerQuitEvent event) {
     myPlayerManager.untrackPlayer(event.getPlayer());
  }

}

public class MyPlayerManager {

  private List<Player> trackedPlayers = new ArrayList<Player>();

  public void trackPlayer(Player player) {
    if(!trackedPlayers.contains(player))
      trackedPlayers.add(player);
  }

  public void untrackPlayer(Player player) {
    if(trackedPlayers.contains(player))
      trackedPlayers.remove(player);
  }

  public List<Player> getTracked() {
    return trackedPlayers.clone();
  }

}

I would like to take a quick detour and note that more private methods or fields != better. It is true that you want the least amount of coupling between classes as possible, but this shouldn't be to the point where you're using convoluted ways to internalize your business logic into one class(and bending the API you are using to your will in order to do this). It is good to have surface area between your classes as with my suggestion above, even if you introduce some coupling. This is thanks to the single responsibility principle, one of the foundations of object-oriented design: classes should do one thing only and do it well. Of course, there's a delicate balance here, but this is part of what makes programming fun, at least for me. Delegating behavior to where it's needed is sometimes a hard thing to figure out.

How about an actual reason to shut down an idea.

You want "actual" reasons for why I won't accept this suggestion? Here you go:

1. The problem with adding this is it just creates API pollution. Unfortunately, not every possible addition to an API is useful. As I explained earlier, the entire point of the Sponge API is to create a useful set of utilities and Minecraft-related interfaces and not to baby-feed plugin developers with their plugin designs. While we may differ on the definition of "useful", I strongly consider your suggestion to not be useful due to your very specific "use case".

2. One of the pinnacles of event-driven architecture is the principle of event delegation, where events come from one entry-point and are delegated to classes that actually do work with them. Almost every plugin in existence has this kind of event dataflow diagram:

   

   In the "use case" that you provided, you're keeping the event listener, event delegation, and eventual work to be done in the same place and same class. You're messing up the separation of concerns.

   EDIT: It may be vague what "plugin class" means in this case. I mean classes that are in your plugin that do business logic, not the classes where the @Plugin annotation belongs.

3. As established above, if you're mucking up your separation of concerns you're doing it wrong. Heck, having a reflection-based method for creating listener classes encourages this bad practice because it makes it easier. There is no reason why you would even need static methods to be event handlers unless you're doing some private-reference voodoo in your example.

And to assume I thought private methods were unique to bukkit... wtf

I try to not take anything for granted. I probably understood it wrong, but here is what you said earlier:

This is also under the assumption that private methods work as they did in Bukkit

I'm probably missing some data. Private methods for what? Event Listeners? Work how? The vagueness of your statement made me think you were talking about private methods in general.

[Soren025]

Thank you for the detailed explanation. Arguably overkill but leaves me satisfied.

as for your confusion for "This is also under the assumption that private methods work as they did in Bukkit" I mean't that that you could put @EventHandler over a private method in a bukkit listener and it would register. Probably wasn't the clearest sentence but given we were talking about event handlers I figured it would be obvious.

[ryantheleach]

I don't understand why it would be appropriate to call a private method from the sponge-api to handle an event?

Am I misunderstanding something about Java, or would this not even be possible without reflection / an inner class adaptor to register it or expose it somehow?

[Deamon5550]

@ryantheleach Its possible with reflection so long as you call Method.setAccessible(Z).

[nightpool]

Okay I'm not really sure where most of these comments are coming from—the only thing I see in the OP is a suggestion that people be allowed to subscribe static methods (i.e. methods on a class, that's where passing the class rather then the instance come from) to an event.

Private methods, API pollution, and creating Listener instance classes (remember, these are static methods) have nothing to do with it.

@gratimax do you have an objection to the possibility of subscribing static methods to events?

[maxov]

@nightpool I rejected this suggestion because there was no compelling use case. All my comments are about how the "use case" that was given was not actually a use case but more a specific problem that the OP wanted handled by the Sponge API. So I wrote about how it wasn't, what an actual use case would be, and how to solve OP's particular problem.

Private methods, API pollution, and creating Listener instance classes (remember, these are static methods) have nothing to do with it.

There is no reason to want to register listener methods on static methods on a class, because the only reason that a developer may want to do it is if they're mucking up the separation of concerns in their plugin as with the "use case" that was provided. The OP was trying to keep player management completely private, in the process mixing in event handling to the same class. Give me a compelling reason why static method handlers should be a thing and I'll listen.

do you have an objection to the possibility of subscribing static methods to events?

There is no check or safeguard against static methods currently when listener instances are passed into the event manager. This is a "feature" of the Sponge implementation that should probably not be leveraged. So if you really desired that static handler, instantiate your listener class and pass it in! Nobody's stopping you. However, passing in a Listener class, meaning that all of your handlers are static methods, is not a supported feature for the reasons above.