Proposal: central flight mediation plugin

This will be primarily of interest to anyone who's writing a plugin that allows player flight by whatever means, but I'd welcome constructive input from anyone.

The Problem

While it's possible for any plugin to enable or disable flight per-player (player.setAllowFlight() and player.setFlying()), these calls are basically a global per-player state. They neither know nor care about which plugins want to enable or disable flight. This poses a problem if more than one plugin wants to enable/disable player flight - conflicts arise.

Example: my ChessCraft & Checkers plugins allow player flight while players are within the boundaries of a game board, but not otherwise. This is a problem if a server is also using Essentials (the /fly command) or FlightPack (fuellable jetpacks). For example, a player with a fuelled jetpack might fly over a chessboard - when flying out of the chessboard boundary, ChessCraft will disable flight for the player, not knowing that the player should still be allowed to fly because of his jetpack. I could hook into FlightPack (or vice versa) and check that, but that really doesn't scale well when there are multiple plugins which might allow flight.

Proposed Solution

A central plugin (let's call it FlightController) which mediates flight requests from other plugins, and as long as at least one plugin wants to allow flight for a player, ensures that they player can actually fly. Plugins would hook into FlightController, and instead of calling player.setAllowFlight({true|false}) would simply call something like FlightController.getController().setAllowFlight(plugin, player, {true|false});

FlightController would then keep track of which plugin wants the player to be allowed to fly, and as long as at least one plugin says yes, would keep setAllowFlight(true) for the player. If no plugins want to enable flight, then player.setFlightAllowed(false) would be called.

Admittedly, plugins would have to be modified to use FlightController, but the intention is that the modification should be very minor - basically, hooking the plugin and making the one-line change described above. A few more API calls might be useful too (checking which plugins currently allow flight, forcibly grounding a player, firing cancellable events when other plugins want to enable/disable flight...). And probably some admin-level commands to get flight status on players (whether they can fly, which plugins are currently allowing flight, etc).

Any thoughts? Would such a plugin actually be worth creating? Are there any pitfalls (subtle or glaring) that I've overlooked?

 

I like this idea! I'd hook into it, assuming I could do so in a soft-dependency kind of way.

 

NathanWolf yes, it would certainly be a requirement that plugins could soft-depend on it - code should basically look something like this:

PHP:

// your plugin would set flightControllerAvailable to true if the
// FlightController plugin can be hooked OK
if (flightControllerAvailable) {
  if (FlightController.getController().setFlightAllowed(myPlugin, player, true)) {
    // player may now fly
  } else {
    // some other plugin must have cancelled the request to allow flight
    // warn the player gracefully somehow
  }
} else {
  // FlightController not available - use direct Bukkit call
  player.setFlightAllowed(true);
}


FlightController would also need to operate gracefully with plugins which unilaterally enable or disable flight for a player. This will take some thought but it should be possible to do...

 

Hm. My problem with this is:
How would you judge whether or not to enable flight, if one plugin says "yes" and the second one says "no"?
Because when a plugin changes the flight state of a player, it usually does it for a reason that the developer pretended to be a good one.
Only evaluating the most recent state change would not be sufficient, because both of those plugins do not know anything from each other.
So both may have a good reason to request a state change. Each of them in it's own context.

 

Sounds like writing a separate plugin is a bit overkill. How about using the Metadata system for once? Then each plugin would only have to add a simple class to handle all the logic:
https://gist.github.com/aadnk/8223697

The key here is that all changes will be ordered by priority and time:

Code:java

private FlightController controller = new FlightController(this);
 
@Override
public void onEnable() {
    controller.setControllerListener(this);
}
 
@Override
public boolean onCommand(CommandSender sender, Command command, String label, String[] args) {
    if (sender instanceof Player) {
        Player player = (Player) sender;
 
        if (!player.getAllowFlight())
            controller.changeFlight(player, true);
        else
            controller.yieldControl(player, false);
    }
    return true;
}
 
@Override
public void onChanged(Player player, Plugin controller) {
    System.out.println("New controller " + controller + " for player " + player);
}

In your example, I would have FlightPack register an allow change with a HIGH priority, as it is a command-triggered ability. Then, when the player happens to fly into the chess arena, ChessCraft would register an allow change with NORMAL priority, which would leave the flyable value unchanged. When the player then leaves the arena, ChessCraft would change to deny flyable, but with a much lower priority, such as LOW or LOWEST.

This is a neat way for plugins to "undo" its changes, and allow plugins with a similar priority to take effect. The changeFlight() method will implicitly remove the previous change that had a higher priority.

Consider what happens if the user disables the jetpack inside the Chess arena. If FlighPack changes to deny, but on a LOW priority, ChessCraft will still take priority as its change is on NORMAL. Only when the player leaves the arena, will he or she loose the flight ability.

Finally, note that change requests are also ordered by the time they were submitted, so if plugin A sets the flight ability to FALSE on NORMAL, and plugin B submits TRUE on NORMAL right afterwards, plugin B will take priority and the player will be permitted to fly.

 

Comphenix this is actually rather elegant, although the priority system has me slightly confused - in particular, using different priorities for enabling/disabling flight. Think I'm going to need to write a testbed for this and see it in action

 

I use a lower priority to yield control to a different plugin when I'm done with my modification. The fact that I disable it, is only actually relevant if there are no other plugins using FlightController.

Perhaps this should be a separate method, to encourage its use?

Code:java

/**
* Yield control over the flight value to the next change request in line.
* <p>
* The fallback value will only be used if there are no other change requests.
* @param player - the player we no longer wishes to modify.
* @param fallbackValue - flight value to set if there are no other change requests.
*/
public void yieldControl(Player player, boolean fallbackValue) {
    player.removeMetadata(METADATA_CHANGE, plugin);
 
    if (updateFlight(player) == null) {
        player.setAllowFlight(fallbackValue);
    }
}

Of course, the biggest problem here is probably not conceptional. It's the practical matter of convincing plugin authors to use this method ...

 

Comphenix
Your download broke

 

Wups, sorry. I forgot to add the link.

 

This logic seems like it could be applied to a lot of other things in the API, one that comes to mind being the showPlayer() and hidePlayer() methods.

 

Sure, it's not difficult to extract the main logic of FlightController into an abstract class, and go from there.

I should also mention that it would be possible to add an event listener to the controller, by adding another METADATA field that maps over Runnables. But it must be done now, before people start copying the class into their project.

That's unfortunately the main issue with such a design, in contrast to a shared library. It's very difficult to add new features (like an event system), once it's widely in use. It's best to write a new replacement in that case, like IPv6.

 

Comphenix
While writing an entire plugin for just FlightController isn't particularly justifiable, perhaps if there was a plugin that contained many features such as this it would make more sense? It would solve any issues with updating.

 

Comphenix

It would also be useful to have some method inside the FlightController class to check if the player could also fly without the plugins "permission", so that for example a jetpack plugin can check if it has to remove "fuel" from the player while he is flying through the ChessCraft region which allows flying even without using a jetpack. So instead of only constantly checking if player.isFlying() the jetpack plugin would also check something like FlightController.getFlightIgnore(thisJetpackPlugin) (or similar) which would return true if another plugin, besides the Jetpakc-Plugin, is allowing the player to fly.. in which case it doesn't have to remove "fuel" from the player

Basicly like the result of FlightController.updateFlight() but ignoring the given (Jetpack)-plugin there, if it allows flying currently, I think

 

Interesting.

Sure, I suppose I could add that. I think it's also a good match for a possible "event system" as well, as you'd probably want to be informed when the "controller" switches to a different plugin, so that you can immediately stop spending fuel.

I've updated the Gist with this new feature.

It would, but some people are very reluctant to depend on other plugins, even when it can solve their specific problem better than the alternative (and not muck things up for everybody else). I've seen this a lot with ProtocolLib, and it is the reason I wrote both the NBT library and TinyProtocol, among other things.

I think the best option is some sort of Bukkit API, but generalized to all kinds of game state. But that's easier said than done, especially since it has to both be simple yet powerful, and also be backwards compatible with older plugins ...

I suppose you could try to implement this through Bukkit events, but then you can no longer guarantee that certain method calls do what the claim they do. A plugin that must disable flight no matter what can no longer assume setAllowFlight() will as its told - it must now register a Bukkit event on HIGHEST, and force the value. Not ideal.

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

Yeah, a way to check if other plugins allow flying is desirable. Another use case: ChessCraft can (configurably) either contain players within the flight region while flying (bouncing them back toward the board centre if they try to leave), or allow them to leave but disable flight when they do. If the "captive" mode is active, I wouldn't want to bounce them back if some other plugin would allow flight to continue outside the board's boundary.

Very true. On the plus side, there's already interest from the developers of Magic and FlightPack, plus my two plugins (ChessCraft/Checkers). Would be good to hear comments from the Essentials & MagicSpells authors too - so I'm tagging KHobbits and nisovin ...

If/when this system actually starts seeing some use, then authors of plugins who do use it can always start directing users who experience flight conflicts to nag plugin authors who don't use it yet

Hmm, how will this work if the Jetpack plugin has registered control with a HIGH priority, but then ChessCraft registers control with NORMAL priority? Since ChessCraft has a lower priority, will the onChanged() callback get called when a player flies into a chessboard zone?

Update: yes, if some other plugin registers with priority HIGH and I fly or walk into a chessboard (ChessCraft registers with priority NORMAL), controller change notification doesn't occur. Which poses a problem, since the high-priority plugin doesn't get to know that ChessCraft can allow flight and e.g. stop consuming fuel.

How necessary is the whole priority system, anyway? Or putting it another way, what are the potential problems if every plugin just uses the same priority (I've tested with both plugins at priority NORMAL and everything works smoothly - control is handed over to ChessCraft when I fly into a board, and back to my test plugin when I fly out) ?

Update #2: thinking about it some more, priorities are useful, but perhaps the reverse of what you originally suggested: ChessCraft should register with HIGH priority, since it a) applies to a more specific region, and b) allows "free" flight within that region. If a jetpack plugin registers as NORMAL, then activating the jetpack while in a chessboard will leave ChessCraft as the controlling plugin. Once the player leaves the board, the jetpack plugin will become the controller, and fuel can start being consumed, until another board is entered, when ChessCraft becomes the controller again.

Might be good if notifyController() was always called, even when state (and thus bestPlugin) is null? Plugins may want to know when there's no controller any more, i.e. a null value for the plugin is passed to the callback.

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

Good point, I'll add that now.

You're right - that would make much more sense. Perhaps we should discuss and document the semantics of the different priorities now, before it is adopted by plugins.

I suppose one priority, such as normal NORMAL, could be reserved for flying abilities that consume or cost resources. If the price is really exorbitant, the plugin could use LOW instead. And then anything that is arena-specific and free (such as ChessCraft) could be classified as HIGH, preventing plugins from unnecessarily charging resources. This also makes it easy to disable flying on the server generally, without interfering with ChessCraft or Checkers.

 

Yeah, that's pretty much how I see it. Keeping the current list of priorities would probably be fine, though it's hard to visualise the need for more the LOW/NORMAL/HIGH. Still, better to have extra priorities than not enough... e.g. HIGHEST might be useful to grant admin-level free flight without actually going into creative mode (e.g. the Essentials /fly command).