Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Create a permission for region available #241

Merged
merged 2 commits into from
Nov 19, 2016
Merged

Create a permission for region available #241

merged 2 commits into from
Nov 19, 2016

Conversation

jewome62
Copy link
Contributor

@jewome62 jewome62 commented Nov 2, 2016 •
edited
Loading

I try write some code for my suggestion #240

Enjoy

@jewome62
Update GeneralRegion.java
b138f0e 
Add Available abstract function
Check Permission modification

Update BuyRegion.java
Add Available function for selled

Update RentRegion.java
Add Available function for rented

Update TeleportCommand.java
Check new permission

Finish feature teleport available region
@jewome62 jewome62 changed the title [WIP] Create a permission for region available Create a permission for region available Nov 3, 2016
@jewome62
Copy link
Contributor Author

jewome62 commented Nov 3, 2016 •
edited
Loading

Hi,

I have add 2 new permissions (I mark this permission as default, but i can change)

  • areashop.teleportavailable
  • areashop.teleportavailablesign

I have add 2 new translations:

  • teleport-noPermissionAvailable
  • teleport-noPermissionAvailableSign

When the region is available (not rented or not sell) and the player have the permission. He can be teleported to it.

I tested on my servers (spigot 1.10.2) with PermissionEx and test

  • without permission i'm teleported (beause it's default permission)
  • with -areashop.teleportavailable (negative permission) i'm teleported to the sign
  • with -areashop.teleportavailable and -areashop.teleportavailablesign i'm not teleported

I wait your reviews. @NLthijs48 🥇

@jewome62 jewome62 mentioned this pull request Nov 3, 2016
Closed
@NLthijs48
Copy link
Owner

Thanks for the pull request! Sorry for the late review, got caught up with other stuff.

I support your idea of allowing people to teleport to regions that are free, this already kind of possible for players by using /as find so adding this and giving it by default is fine. The only problem I see in the changes are the messages, at the end you say regions availables, but that should be available regions. (also note the dot at the end). If that is fixed I can merge the pull request.

Even before your pull request I was already thinking about redoing the teleport permissions, because the permissions are not exclusive currently (they overlap in parts). When I do that I of course keep the option you added.

Thanks again for your pull request, I'll hear from you.

@jewome62
Copy link
Contributor Author

Translation is fixed

Thanks for reviews

@NLthijs48
Copy link
Owner

Looks good now, merging it. Thank you for adding this!

@NLthijs48 NLthijs48 merged commit 1dfedea into NLthijs48:master Nov 19, 2016
@jewome62
Copy link
Contributor Author

Thanks, I wait next release :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jewome62 @NLthijs48