Implement Sponge API

[Aaron1011]

@SpaceManiac has said that Glowstone will (eventually) implement the Sponge API.

Now that the Sponge API is scheduled for a release this month, I think we should start discussing how we plan to implement it.

However it's done, I think it's going to require a lot of rewriting, even if we were to drop Bukkit support entirely. When we decide to start actually implementing it, we might want to consider temporarily freezing the codebase, as this is going to touch almost every file.

[ZephireNZ]

IMO, we should split Glowstone up into 3 bits. The core Glowstone with all the mechanics would be API agnostic, but be influenced by the needs of both. And then have two API layers (perhaps exclusive, or both) that run on top of the core.

[dequis]

we might want to consider temporarily freezing the codebase

Please no. Branches exist. We still need to develop a lot of vanilla features and IMO that's a higher priority than sponge.

The core Glowstone with all the mechanics would be API agnostic

Some independence from glowkit would be cool, but... I can't help but think this is going to end up with copy-pasting half of glowkit into our own half-assed abstraction layer "API".

[turt2live]

I personally think it would be best to let @SpaceManiac continue playing with it before deciding what should happen.

As for more of my personal opinions:

...drop Bukkit support...

No. Just no. I shouldn't need to supply reasoning as common sense says no.

..two API layers...

This is not how a well designed application would work. You would have one API layer that maps to one implementation. Unless you are considering Sponge and Glowkit as API layers, there should only be one logical API layer.

...vanilla features ... a higher priority than sponge

Getting the game working is top priority. Of course some consideration is/has been made for supporting other API interfaces, but the implementation of additional API interfaces should be put off until the game is complete.

[SpaceManiac]

Probably I will want to have a better baseline for Sponge API support through ShinySponge or an investigation like it before anything substantial happens. Dropping Bukkit API support is not something I think is necessary but it may be tricky to support Bukkit and Sponge simultaneously.

I agree that - at least for now - features and bugfixes in the implementation take priority over Sponge API support. I will keep an eye on things in this regard.

As an alternative to a big freeze, here's what I think is a reasonable migration plan for any big rewrite:

• Get any important big, complex changes or additions tested and merged before beginning.
• Start the rewrite on a branch.
• Continue development and PR processes as normal on master.
• Work on the rewrite until it is up to speed with master as it was before the branch.
• Freeze development on master.
• Merge changes on master since the rewrite started into the rewrite branch.
• Merge the rewrite into master.
• Proceed as normal.

[mastercoms]

Perhaps we could have something like this:

net.glowstone.GlowServer
// ...
import net.glowbridge.plugin.*;
enablePlugins(PluginLoadOrder.STARTUP);
// ...
net.glowbridge.plugin.SimplePluginManager
// ...
public void enablePlugins(plugins) {
    if (usesBukkit) {
          enableBukkitPlugins(plugins);
     } else if (usesSponge) {
          enableSpongePlugins(plugins);
     } else {
          enableBukkitPlugins(plugins);
          enableSpongePlugins(plugins);
     }
}
// ...

It is messy, sorry. And I am pretty sure there is a better way of doing this but I guess it could work.

[turt2live]

@mastercoms I edited your comment to be readable.

[mastercoms]

@turt2live thanks

[Tonodus]

Perhaps we could have something like this:

I'm not certain I understood you right, but some thoughts:

• IMO the PluginManager / EventManager isn't the big problem. The problem is, that all our files implemts Bukkit interfaces and letting them implement the relevated Sponge ones will cause errors:

class GlowWorld implements org.bukkit.World, org.spongepowered.api.world.World {
    List<org.bukkit.entity.Entity> getEntities() {...}
    Collection<org.spongepowered.api.entity.Entity> getEntities() {...}
    //...
}

• What you wrote suggest that the server can load bukkit plugins or sponge plugins. Why not supporting both at the same time? Imo it'd be a big benefit, if Glowstone supports multiple apis at the same time.
• It'd be really nice, if GS could be easily extended to support other APIs as well.
• Btw, iirc Sponge wanted to create a "glowstone sponge bridge" themselves, so no actual need to implement that on our side, or did I miss something?
• Lastly, was decided, whether GS supports other APIs but bukkit directly or through gs-plugin/bukkit-plugin?

[PaulBGD]

Well, there's a few options. 1) Do what Bukkit did to the Minecraft Server, and simply wrap Glowstone 2) Implement both Bukkit and Sponge (possible) 3) Option 1, however we wrap both Bukkit & Sponge and ditch implementing Bukkit with Glowstone

Honestly, my personal option would be 3. However what I think we'll do is 1. It allows us to somewhat support Sponge, while keeping native support for Bukkit.

[mastercoms]

@Tonodus Well the plugin manager was just one example of any method for sponge and bukkit. Basically, I was trying to show the idea of GlowBridge, which would be called by Glowstone instead of bukkit, like we do now, and then GlowBridge calls either bukkit or sponge or both.

[turt2live]

I personally don't think it's fair to consider our "options" at this stage. Glowstone has many unimplemented vanilla features (let alone Bukkit API functionality) that should at least be present before something like Sponge can be considered more heavily.

I think it would be best to keep in mind that Sponge support is ideal for Glowstone when writing current and future code, but the active support of it should be put off until more of the game is complete.

[deathcap]

Just a point on this:

IMO the PluginManager / EventManager isn't the big problem. The problem is, that all our files implements Bukkit interfaces and letting them implement the relevated Sponge ones will cause errors:

class GlowWorld implements org.bukkit.World, org.spongepowered.api.world.World {
    List<org.bukkit.entity.Entity> getEntities() {...}
    Collection<org.spongepowered.api.entity.Entity> getEntities() {...}
    //...
}

This may or may not be a problem, since even though Java-the-language does not allow overloading methods based on return type, the JVM has no problem with it. As long as the Sponge API and Bukkit API interface methods have distinct type signatures it should be possible to implement both on the same class.

As for how to do this, the Sponge team has developed a Mixin library supporting it: https://github.com/SpongePowered/Mixin/wiki/Introduction-to-Mixins---Understanding-Mixin-Architecture - and of course the method mapper already in Glowstone for the Bukkit API overloaded return values (_INVALID_damage, etc.).

I'm not sure how this overloading will interact with generics type erasure, though. List<org.bukkit.entity.Entity> erased to List and Collection<org.spongepowered.api.entity.Entity> to Collection, signatures would be ()Ljava/lang/List; and ()Ljava/util/Collection; (is this a problem since List is a Collection?). If the types only differ by a generic type variable, unsure what will happen.

But for this particular example, I can't find getEntities() in https://github.com/SpongePowered/SpongeAPI/blob/master/src/main/java/org/spongepowered/api/world/World.java - maybe the API has changed, or was this collision hypothetical? If not, and overloading is not possible, I suppose separate classes could be used as needed.

[gdude2002]

I think there's definitely going to be huge considerations, naming/typing conflicts aside, since Glowstone is very much designed around the Bukkit API and the way the Bukkit API does things. I'm not really sure how different Sponge is from Bukkit implementation-wise, but maybe a way to do this is to have base Glowstone classes, and then specific subclasses or wrapper classes for each type of plugin?

[ZephireNZ]

As for how to do this, the Sponge team has developed a Mixin library supporting it: https://github.com/SpongePowered/Mixin/wiki/Introduction-to-Mixins---Understanding-Mixin-Architecture - and of course the method mapper already in Glowstone for the Bukkit API overloaded return values (_INVALID_damage, etc.).

As far as I know, this would be very difficult to use. It uses LaunchWrapper to initate and perform the Mixins. We might be able to emulate the starting of LaunchWrapper, but we won't just be able to use the library off the bat.

[kamcio96]

Wrapping classes is good option, but what about events? What about their classes? SpongeAPI has interfaces, should we implement they in Glowkit? And what about custom events between Bukkit and SpongeAPI?

We can't use Mixin. It works in runtime, so we don't need it. We have other tools eg. glowremapper

[Tonodus]

Just a point on this:

IMO the PluginManager / EventManager isn't the big problem. The problem is, that all our files implements Bukkit interfaces and letting them implement the relevated Sponge ones will cause errors:

class GlowWorld implements org.bukkit.World, org.spongepowered.api.world.World {
    List<org.bukkit.entity.Entity> getEntities() {...}
    Collection<org.spongepowered.api.entity.Entity> getEntities() {...}
    //...
}

This may or may not be a problem, since even though Java-the-language does not allow overloading methods based on return type, the JVM has no problem with it. As long as the Sponge API and Bukkit API interface methods have distinct type signatures it should be possible to implement both on the same class.

Thats true, however it's kind of hacky and should be avoided if possible IMO, all the nice things like overriding interface's methods can't be done then...

I'm not sure how this overloading will interact with generics type erasure, though. List erased to List and Collection to Collection, signatures would be ()Ljava/lang/List; and ()Ljava/util/Collection; (is this a problem since List is a Collection?). If the types only differ by a generic type variable, unsure what will happen.

I'm sure List and Collection wouldn't impair each other, however there moght be cases where the same method name needs to return the same object (f.e. two times List<?> which would be seen as java/util/List both times). Imagine sponge change the API to return a List rather than a Collection? We couldn't use one class for the two implementations then.

But for this particular example, I can't find getEntities() in https://github.com/SpongePowered/SpongeAPI/blob/master/src/main/java/org/spongepowered/api/world/World.java - maybe the API has changed, or was this collision hypothetical? If not, and overloading is not possible, I suppose separate classes could be used as needed.

Seperated classes would be needed, but are still a pain, as they have to interact with the core Glow*-classes.

I think there's definitely going to be huge considerations, naming/typing conflicts aside, since Glowstone is very much designed around the Bukkit API and the way the Bukkit API does things. I'm not really sure how different Sponge is from Bukkit implementation-wise, but maybe a way to do this is to have base Glowstone classes, and then specific subclasses or wrapper classes for each type of plugin?

Subclasses aren't possible, if the interfaces can't be implemented in the same class. The only advantage of subclasses would be that bukkit and sponge are seperated, however most things must simple be done in the base class, leading to many functions doing nothing but calling super.something();

Wrapping classes is good option, but what about events? What about their classes? SpongeAPI has interfaces, should we implement they in Glowkit? And what about custom events between Bukkit and SpongeAPI?

Glowkit should be the continued bukkit api, IMO, no need for implementing SpongeAPIs there (I' ve no idea how you would do that anyway...). As for the events, it shouldn't be that problem... Best option would be to call all sponge listeners with their event class implementations and if a vanilla callback is cancelled by a sponge plugin, the bukkit event is cancelled by default too. However, detailed thoughts on this can be considered if we actually have or are near a sort of a sponge implementation.

Wrapper classes seems the best option we have actually (if a class can't implement both sides), however they still require A LOT of rewrite of glowstone's core code. We also have to decide whether we still want to be bukkit implementation allowing sponge plugins to run through a sponge layer, or whether we want to be a sponge implementation allowing bukkit plugins to run through a bukkit layer. It might make more sense using wrapper classes for bukkit compatibility than the other way round.

[kamcio96]

@Tonodus, your last sentence means SpongeAPI should be main implementation and then wrap it into bukkit?

[gdude2002]

@kamcio96 Pretty much, although that was more of an open-ended opinion, I think :P

[ZephireNZ]

I know that it's still quite new, but what about converting Glowstone to a full Sponge implementation, and then rely on a project such as Pore?

That abstracts out the Bukkit compatibility to them, while we focus on converting to Sponge.

[PaulBGD]

@ZephireNZ Glowstone at the moment doesn't exactly have a ton of active development.. completely switching what we're doing would take a lot of time, and development.

[kamcio96]

@PaulBGD so when we should change api to SpongeAPI? Now we have a lot of code to rewrite, but in future we will have it more

[gdude2002]

Glowstone was always intended and designed to be a Bukkit replacement. I doubt @SpaceManiac is going to want to switch tack all that suddenly.

[Tonodus]

@kamcio96

@Tonodus, your last sentence means SpongeAPI should be main implementation and then wrap it into bukkit?

Glowstone will have an API it is designed for and which it supports 100% without hacky things, wrapper classes, etc. pp. Currently it's bukkit, I just said it is technically possible to change this.

[Ribesg]

It is technically possible to change everything. That's not the question.

[kamcio96]

So when can we start changing it? :smile:

[turt2live]

I still don't think it's fair to say that we need to "switch" or turn our direction towards another goal. Instead, we should be working on a continuous path of implementing missing features (as well as resolving bugs) with a secondary path of working towards a secondary goal, such as investigating how the Sponge API could be implemented.

This kind of workflow means that we don't have to throw everything away to get something done. This also means that community members can, if they please, take different approaches and propose them through the pull request system.

[deathcap]

Of interest, just posted this: https://forums.spongepowered.org/t/bukkit2sponge-an-implementation-of-spongeapi-for-bukkit-servers/6747/3 - a Bukkit plugin to load SpongeAPI plugins, based on an updated version of @SpaceManiac's https://github.com/GlowstoneMC/ShinySponge. Very barebones at the moment, but it does work with Glowstone.

The strategy of bridging Bukkit to Sponge is looking to be more feasible than I expected. The Bukkit->Sponge (or Sponge->Bukkit depending on your perspective) bridge could either be a standalone plugin, or part of the server implementation, either works.

Only catch is that the Bukkit API layer has to be implemented comprehensively enough to let SpongeAPI fully hook into it. There is more active development on SpongeAPI and it supports newer game features not necessarily in Bukkit (either in the last official 1.7 Bukkit, Glowkit 1.8, or Spigot-Bukkit 1.8, currently) so it would have to be enhanced accordingly but that should be possible to do in Glowkit as needed.