SpongePowered / SpongeAPI

A Minecraft plugin API
http://www.spongepowered.org/
MIT License
1.14k stars 342 forks source link

Check if a block is modifiable by a player #1169

Open randombyte-developer opened 8 years ago

randombyte-developer commented 8 years ago

Edit: Now I'd just go for checking if a block is modifiable by a player.

Hello!

In this thread some people had discussed about having a Region API. The Region API should be a thing that a plugin can ask for example:

I am not sure about the ownership thing(creator, owner, member, ...) and as @gravityfox stated here about multiple 'region' provider.

Aaron1011 commented 8 years ago

I'm not really sure that this should be in the API.

The current 'creator and 'notifier' concepts in the API don't really have to do with ownership - they're purely a tracking concept. The 'creator' represents the player that originally spawned an entity/set a BlockSnapshot, while the 'notifier' represents the last player to trigger a block update (it's currently not set for entities).

What you're describing could be exposed by an individual protection plugin, but doesn't really belong in the API. Any such regions should be completely transparent to a plugin. Instead, a plugin can provide the Player it's performing an action on behalf of. This could be in the form of a Context when checking permissions, or a Cause object when setting a block.

The advantage to this approach is that it doesn't require the plugin to be aware of the underlying regions, or if there are regions at all. A protection plugin can simply cancel an event as necessary, based on the information (usually a Player) provided by the calling plugin.

randombyte-developer commented 8 years ago

My problem was that my plugin had to check before registering a block to its system if the player has build permission on this block. My only idea was to provide a command to go in edit mode in which the player has to switch before breaking the block. If the player break wasn't undone by any other plugin I assumed that the player has build permissions there. It would definitely a better user experience to just check via an unified API if the player has permissions. Otherwise I would have to rely on the different APIs by WorldGuard, FoxGuard, RedProtect and others.

Edit: Wait, you mean add another key to this like INTERACT_POSITION and then check for something unified like blocks.change - permission with that Context?!

ryantheleach commented 8 years ago

I think the biggest problem occurs, is when you are trying to work out if a player has permission to perform an action remotely at a given location.

E.g. Player wants to create a dangerous plugin provided thing. plugin wants to check explicitly for whether the player is trying to perform this action in an area where they would be allowed to place / break blocks.

Fire a fake block place event? Sure it works, but other plugins assume a block was placed.

Check to see if a player has permission? The contextCalculators for the current player, are basing their checks based on the players current position, not at the position that the player is currently acting on. This means that you could stand outside of spawn, and create a dangerous thing just inside the boundary of spawn...

Provide a contextCalculator that evaluates to see whether the player is currently performing an action in a different context? Sounds like it's going to get messy fast.

Possible Solutions?

  1. Have explicit support for acting at a remote distance with permissions / contexts.
  2. Have some way of querying whether a player is an owner/member of an area.
  3. Somehow have a context calculator that is action aware, and can tell where a player is interacting with.
  4. Something else?
randombyte-developer commented 8 years ago

I would vote for 3. because Contexts are already there. So we have one thing, the Context, for direct and remote things.

eitanmk commented 8 years ago

I've been working on a region service for my modular Sponge plugin, and have been researching a few Bukkit region plugins for inspiration. @randombyte-developer, I think you are being a bit specific about what you want from regions, which is why there is pushback on putting it in the API.

It seems that all a service provider for regions needs to implement is a Region getRegion(Location) method. That way, an implementer can choose how to create/modify/delete regions (align with chunks? polygon shapes? have a limited height? can they overlap?) and other features (ownership? protections? notifications when entering/leaving? prevent physics?). Any other plugin would need to simply get a Region object by Location and determine how to act.

Am I totally off base here?

gravityfox commented 8 years ago

If anything, I think that's really what a region is. It's just a bounded area.

Foxguard already has regions defined that way. The IRegion class just has a few contains() methods for determining whether an entity or block is inside or outside the region.

There are then utility methods for getting all of the regions at a certain location.

kashike commented 8 years ago

As @Aaron1011 said, I'm not really sure that this should be in the API - this is something for plugins to do.

gravityfox commented 8 years ago

Actually. If a plugin is going to be modifying blocks or something, just fire your own dummy BlockChangeEvent.Modify or something, and see if it gets canceled. The event can be a stub with nothing but the very basics.

eitanmk commented 8 years ago

First, it seems that all of this back and forth is because it isn't clear what the feature requests in the OP have to do with a Region API. I think Regions could possibly help for those features, but I agree that you can build these features without Regions. However, given that the issue title is about Regions, can we either shift the conversation back to Regions or can we change the title of this issue? If we're only going to give feedback on the specific features in the description, then the title doesn't reflect the discussion and I will gladly open a more relevant conversation in a different issue. I don't want to doom the possibility of a fruitful discussion of a Region API b/c of a specific list of feature requests.

@gravityfox - I agree with you that region implementations belong in plugins. I'm glad FoxGuard has it as a feature. I'm not understanding the argument that because FoxGuard does it, how does it follow that Regions doesn't belong in the Sponge API? How is it any different from the Economy API? I would argue that having a common way to register regions across plugins would allow for some interesting features. And 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.

gravityfox commented 8 years ago

I don't think I ever said that A Region API shouldn't be part of Sponge. It's just that @randombyte-developer is selling a specific use case that actually doesn't need a Region API at all. In fact, after a few conversations with @Aaron1011, we found several cases in which a Region API would be counterproductive for this purpose. A few things came out of that conversation which are worthy of note:

There were some other points made, but those are the highlights I think.

Now, a Region API could be used for a lot of other things, and in that case, that could actually be great. However. an example use case does not immediately jump to mind, so if anyone has any ideas as to what a Region API could be useful for, that would help this conversation a great deal.

randombyte-developer commented 8 years ago

Meanwhile, I worked around this issue. But it would be cool if a plugin could check if a block is modifiable by a player. If the plugin would just register a block and some time later the plugin would actually modify the block the first time, the plugin could just modify it, without being cancelled because the cancelling plugin has no context who indirectly modified the block by registering it to the plugin. If the cancelling would know who indirectly modified the block the message "You don't have permission here" would be confusing to the player because he doesn't know that the plugin changed blocks in his context. Apart from that, the plugin could do other things like cancelling spawn events. Then the plugin couldn't even know if this action gets uncancelled. So I'd now go for just checking if a player can modify a block.

eitanmk commented 8 years ago

@randombyte-developer - thanks for clearing up the issue title

@gravityfox - I apologize. You are correct. I've been very confused by people replying to the OP's use case and not talking about regions. Looks like we may need a separate issue after all.