SpongeAPI icon indicating copy to clipboard operation
SpongeAPI copied to clipboard

Proposal: Region API

Open eitanmk opened this issue 8 years ago • 37 comments

(Separating out the relevant parts of the discussion from #1169 )

Without an abstract API definition, each plugin has to maintain its own regions and server operators have to copy/redefine/recreate regions in each plugin that they want to use. It would be nice to be able to choose your Region system and be able to have plugins access your region list and build features around it.

Some use cases - just brainstorming off the top of my head here:

  • MMO style "Entering " messages - can be floors within a structure or for a whole kingdom
  • Have different Economies in different regions (mimics the real world)
  • Owned/Claimable plots of land (with grief protection)
  • Arenas (physics can be different, auto health regen if you're inside, etc)

Yes, I'm sure there are ways of building these things in plugins without a Region API, and I'm not looking for suggestions on how to build them without Regions. The point is that the benefit of centralizing regions in a service for use in multiple plugins on a server. The actual details on how to create/modify/delete/save regions depends on the regions service implementation, where the server owner can choose the implementation that fits her requirements.

eitanmk avatar Apr 06 '16 12:04 eitanmk

I'm quite against having a region API in the SpongeAPI. In existing platforms (Bukkit, etc), region plugins have done things differently - some do regioning completely different than others, and it's better to allow the same here with SpongeAPI.

kashike avatar Apr 06 '16 14:04 kashike

I honestly could see this being implemented as a very elementary service at the very most which could allow you to retrieve a Region for given coordinates and have some common methods in it. Other than that, I agree this probably does not belong in the API.

A region could potentially extend Extent and be kind of a middleware class between World and Chunk.

windy1 avatar Apr 06 '16 14:04 windy1

The problem with doing even that, is that we're controlling how you obtain a Region, and how it has to be queried. I'd also say that it shouldn't extend Extend, a Region shouldn't have every single method a World/Chunk does in my opinion.

kashike avatar Apr 06 '16 14:04 kashike

@kashike You would just get the Region through a service that a plugin could provide or there could be some kind of RegionProvider default implementation for registering Regions. What methods of Extent don't you think it should have? In most cases a Region would probably just serve as a proxy for a subset of coordinates that convert to World coordinates and call the method on the World.

windy1 avatar Apr 06 '16 15:04 windy1

The biggest issue I see with the Region API is we limit the way we recognise/define "regions".

kinggoesgaming avatar Apr 06 '16 15:04 kinggoesgaming

@windy1 The fact that SpongeAPI would dictate what methods a Region should have. That's something that should be left to the plugin actually handling regioning. Why should we say that Region accepts X when PluginA wants it to accept Y?

kashike avatar Apr 06 '16 15:04 kashike

Well all I'm saying is that we add some common methods to a region object that people can use without adding plugin dependencies. If they want to get more specific they can add a dependency and then cast the region to FoxguardRegion or whatever.

windy1 avatar Apr 06 '16 16:04 windy1

And what I'm saying is that the region plugin might not want the methods SpongeAPI has - they may do things very differently, and it would result in them not using what SpongeAPI provides anyways.

kashike avatar Apr 06 '16 16:04 kashike

The only thing it would represent would be a subset of coordinates within a World? What possible implementation of Region would not fit that definition?

windy1 avatar Apr 06 '16 16:04 windy1

What I'd like to understand is, what qualities do the Permission and Economy APIs have that merit inclusion in Sponge, that Regions don't have. Limiting what can be done in either Permission or Economy APIs doesn't seem to be an issue, so why is it for Regions? The same arguments I'm seeing here against this idea (limiting how we define x, controlling how to obtain x, x has been done completely differently in the past) seem arbitrary to me, and I can levy the same criticisms for Permission and Economy APIs.

So can you please spell it out for me? What are the fundamental differences between the Permission and Economy APIs, versus a region API?

Is there an objective rubric for API inclusion? There are some really smart devs on this project. Surely something extensible could be created...

eitanmk avatar Apr 06 '16 17:04 eitanmk

I started sort-of beginning work on one here: https://github.com/kashike/SpongeAPI/commit/7ffe0c666054fb59330bc42483dc523467f53e06

I'll go ahead and continue on it and open a PR later, but input on what can be regioned, how to obtain regions, what can be tested against one, etc would be nice.

kashike avatar Apr 06 '16 17:04 kashike

@eitanmk: Here's some of the rationale behind the Permissions and Economy API:

For permissions, a basic permissions service already exists in Minecraft - operator levels. This is what the default PermissionService implementation uses to perform permission checks. However, most people will replace this with a full permissions plugin, as oping people to control permissions is a very coarse-grained way of managing permissions.

Economy, on the other hand, doesn't exist at all in Vanilla. Unlike Permissions, the purpose of the Economy API is solely to allow compatibility between plugins which want to use economy in some way. Instead of telling plugin developers to use something like Vault (but for sponge), we opted to standardize and include it in the API itself.

My main objection to a Region API is that it essentially hides the underlying form of protection, that is, a plugin cancelling an event. By exposing a Region API, we would be encouraging plugins to check a region instead of doing the correct thing - firing an event/calling a method, and checking if it was canceled.

The key thing to consider is that while a protection plugin may use some form of a region, many other types of plugins can and will cancel ChangeBlockEvent. To ensure maximum compatibility, a plugin shouldn't rely on any one particular form of protection (such as a region), but instead simply check the final result of an action such as setting a block.

Aaron1011 avatar Apr 06 '16 19:04 Aaron1011

@Aaron1011 - Thanks for the explanation. I think I follow. It would essentially require a conditional in event handlers to check region containment in addition to checking event cancellation status? And the former can never be guaranteed, thereby opening up a slew of logical errors that will likely get reported on the forums?

I guess I'm grasping at straws here, but is there some other way to expose region data in event handlers? Perhaps as part of the event cause/filter system? Or the location API somehow, like how @windy1 sort of suggested?

eitanmk avatar Apr 06 '16 19:04 eitanmk

One thing that has always bothered me about throwing ChangeBlockEvents is that plugins listening later assume the block got changed.

Is there not some easy way of throwing an event, and having it only be processed by the pre processor plugins that would cancel something for protection purposes?

I mean, you could throw your own event, but unless other plugins know what to listen for, they arn't going to deal with it correctly. Changing blocks is significantly easier in sponge, because of the multi-block changes which can cover a large usecase, But for stuff where you want to test to see if a different plugin can allow people to claim land for instance, how can you check to see if that is spawn protected by another plugin, or if a player has permission at that specific block (as opposed to the players current location) These are the sorts of questions that people normally fire "fake" events for as opposed to throwing the correct event.

ryantheleach avatar Apr 06 '16 19:04 ryantheleach

@ryantheleach: The issue is that, fundamentally, a plugin can do whatever they want to in an event handler. We can't really require plugins to separate out cancellation logic from everything else in the handler, so the only surefire way is to actually set the block.

Currently, you'll have to actually check the BlockState in the world after calling setBlock.

@gabizou: Would it make sense to change the return type of setBlock to indicate the result (e.g. returning the event itself, or something indicating what BlockSnapshot was ultimately used).

Aaron1011 avatar Apr 06 '16 19:04 Aaron1011

The only thing it would represent would be a subset of coordinates within a World? What possible implementation of Region would not fit that definition?

That's a one-way definition. It only means that given a Region object you can find out if a coordinate is contained in that Region object. What if I want overlapping regions? If I query the service for a coordinate, will it return a collection of Region objects? If so, Region objects cannot be defined by the space they contained, but something else entirely (like a string id), or else two Regions that overlap perfectly would be considered the same Region.

wizjany avatar Apr 06 '16 19:04 wizjany

IMO the region API is almost useless for all currently discussed topics or adds way to much overhead to be in the API.

All usecases more or less target ownership or general protection issues.

The following quedtions came to my mind while thinkinh about those. If some plugin discovers that a region is protected by some plugin what can it learn from that fakt? Nothing. It does not know if its just a region to give said region a name or whether some actions are prohibitioned or not. Are just users not allowed to do it or plugins as well? Can plugins forcibly bypass the protection or are the blocked at all if they dont do any checks? If we test all possible changes then we could do the same using events as well.

Does the absense of a region tells us anything? Global region rules? Region that is not registered accidentally?

Neither the presense nio the absense of a region tells us anything. If you have to know that some plugins region have special properties that you have to chrck for in specific impls than skip the region api atall and just ask for that plugins region directly anyway.

ST-DDT avatar Apr 06 '16 23:04 ST-DDT

@ST-DDT - Broad sweeping statements without specifics isn't helpful.

All usecases more or less target ownership or general protection issues.

That isn't even close to true. MMO region announcements has nothing to do with neither ownership nor protection. A plugin would use the region list to determine when a player has passed into a region and show a message. Regen health in an arena. Again, no connection to ownership or protection. Yes, some things could make use of ownership/protection features, but to sweep this all aside b/c of that alone isn't constructive.

If some plugin discovers that a region is protected by some plugin what can it learn from that fakt? Nothing.

You've entirely missed the point. If you are implementing a plugin that is trying to determine if something is allowed in a region, you're doing it wrong. Period. A region is simply a construct of location. A plugin uses a region service to provide functionality based on location in or out of regions. They would have to use the rest of the Sponge API to prevent conflicts like any other combination of plugins in a responsible ecosystem.

@wizjany

What if I want overlapping regions?

That is precisely the hardest question I'm trying to sort through on my own. I'd argue against them, but an easy way to prevent this from happening has eluded me.

eitanmk avatar Apr 07 '16 00:04 eitanmk

To announce join messages on region join you have to listen to move events. Doing the nessesary checks every tick for multiple players from multiple plugins drains a lot of performance. IMO its better to shove this off to a single plugin using a callback like system. However only the feature should be plugged in but any config should stay in the region plugin. Otherwise you will get multiple messages for a single region without knowing which plugin caused it. This also spreds the config making it hard to create proper backups of the regions. This could also cause inconsistencies due to manually edited files of the region plugin causing a region to be absent but still being referenced by the other plugin.

What if I want overlapping regions?

I will most definitly need overlapping regions. Mainly because them with different funtional contexts. Fighting stage (pvp+destructible), arena building (no pvp + indestructible + message), outer protection (indestructible) + minigame area (named region for /whereis player calls).

If you are implementing a plugin that is trying to determine if something is allowed in a region, you're doing it wrong. Period. A region is simply a construct of location.

Well IMO there is already Extend that offers all needed methods maybe we need some helpers for contains checks but thats all. But as mentioned already. If i want to add a feature to a plugin i should hook into said plugin and dont force overhead into the API.

ST-DDT avatar Apr 07 '16 07:04 ST-DDT

@ST-DDT - thanks for clarifying. I see what you're saying now. I guess that for someone like me, who is very stingy when using plugins on my server, I forget that other setups are way more complex and potentially confusing. My inclination is that performance drops as the number of plugins goes up and has nothing to do with regions specifically.

eitanmk avatar Apr 07 '16 11:04 eitanmk

If this was added it would be interesting to have the region API tied with events. Have specific event only get fired when x is in y.

ewized avatar Apr 07 '16 18:04 ewized

This is how it ends without a region api.

randombyte-developer avatar Apr 18 '16 05:04 randombyte-developer

phew, looks like we dodged a bullet there then.

wizjany avatar Apr 18 '16 13:04 wizjany

@randombyte-developer

Region providers register context providers, no different then

public boolean isPlayerIn(Player player, World world, String region) {

The contextCalculator can instanceOf check to see if the subject is a player, and if it is in the region.

Get a new usecase.

ryantheleach avatar Apr 18 '16 14:04 ryantheleach

There are about a billion and one use cases for performing actions when a player enters or leaves an area of space, or performing actions on events within a space (especially with event filtering).

As for whether it should be in the API, the rule of thumb I use is if we can do it more powerfully or more efficiently than a plugin ever could then it should be in the api.

Deamon5550 avatar Apr 18 '16 15:04 Deamon5550

@Deamon5550 I don't see how Sponge can handle regions in a more powerful manor. Efficiency gains in this department would not be possible.

The problem is a matter of variety, some protection systems need less complexity, and they can run faster. Other protection systems need more complexity, and thus more time. Trying to tune to one or the other would be disastrous, and I think it would be detrimental to the ecosystem.

It's also worth noting, there are a lot of ways to go about implementing various region features. For instance, some systems may prefer to do entry messaging based on a timed check so as not to impact the server checking every movement, others may want to (or even need to) check every movement.

Let's also consult an established region plugin's source: https://github.com/sk89q/WorldGuard/blob/master/worldguard-legacy/src/main/java/com/sk89q/worldguard/protection/regions/ProtectedRegion.java

This is the base class for a WorldGuard region. There is a lot of complexity, and to really get out any value, you're either going to have to tie yourself down to one methodology, or you're going to have to have to have implementation specific code (cast with dependencies on the region plugin you want to work with) anyways to get the information you need.

tdlr; This just isn't practical.

DarkArc avatar May 26 '16 00:05 DarkArc

@DarkArc I appreciate you taking the time to write some thoughts. I certainly appreciate the concerns about performance and the desire to avoid complexity. While I understand that, is there something I can do or say to change the conversation from how it's not practical to do regions, to how we can turn it the idea into something practical? Yes, it looks like this doesn't belong in the API, but when the reasoning is spelled out in abstract terms, I'm unable to comprehend the seriousness of the argument in shooting down this idea. When I see that, I see a list of parameters and potential pitfalls to avoid in order to help shape an eventual plan forward. Is this the wrong forum for discussing the details on how something like this could work? It has been my experience in Sponge so far that the API has taught me a better way of building plugins. Instead of worrying how plugin developers will do the wrong thing, is there really no way the API can guide them toward the proper patterns?

(Just to clarify, I'm perfectly able to accept that this idea may be rejected. I'm really fine with that. I would try to contribute more thought to this idea if I understood the "Sponge way", but the quality of devs on this project are on a higher level than myself, so I feel like I don't know what to say or do to advocate for this idea properly.)

eitanmk avatar May 31 '16 00:05 eitanmk

From the perspective of someone who's maintaining a plugin with a region system, I would really REALLY like an API that will allow people to use my regions without having to add my plugin as a dependency. It also will allow me to make use of region systems introduced by other plugins. I think this is a good idea.

A Region API doesn't need to be anything more than a way to expose the regions of different plugins under a common interface. I don't really care too much about having the Region API to be particularly complicated.

All I want to know out of a region is the name, the plugin it belongs to, whether particular points are inside or outside for both double and int coordinates, and maybe its bounding box, if it has one.

All I want is to be able to see regions of other plugins.

I think we should just start there, and work our way up.

gravityfox avatar Oct 13 '16 16:10 gravityfox

So the main usecases can be summarized as:

(Sorted by level of controversy)

Region Query API

  • A Region Query API would support the minimum that gravity fox suggests, A registry of regions that can be queried by location (maybe polygon/aabb ?), and return an identifier, the owning plugin, Maybe some way of fetching data/meta? Maybe some way of fetching Collisions / Bounding Boxes.

Region Event API

  • Event's which are thrown when an entities region changes (if known), when a region is added/loaded or deleted/removed.

Region Management API

  • Programmatically create Regions/Handlers, Attach data/flags.

@gravityfox You say that you want other plugins to be able to query your regions, What's the usecase?

ryantheleach avatar Oct 14 '16 00:10 ryantheleach

I'm all for this.

The only thing I think is vital is that it's relatively abstract. We shouldn't force AABBs, for example.

ZephireNZ avatar Oct 14 '16 02:10 ZephireNZ