Civcraft / PrisonPearl

Minecraft plugin for civcraft which allows players to imprison other players inside ender pearls
BSD 3-Clause "New" or "Revised" License
4 stars 16 forks source link

World border whitelist #47

Closed idoash4 closed 9 years ago

idoash4 commented 9 years ago

So this is my first big change I hope this is right. I didn't test it so there are probably a few bugs.

ttk2 commented 9 years ago

so how does this change work? do we exclude specific chests from coal costs?

On Sun, May 24, 2015 at 9:13 AM BlackXnt notifications@github.com wrote:

So this is my first big change I hope this is right. I didn't test it so

there are probably a few bugs.

You can view, comment on, or merge this pull request online at:

https://github.com/Civcraft/PrisonPearl/pull/47 Commit Summary

  • Adding World Border Manager
  • Commands
  • Changing file name

File Changes

Patch Links:

— Reply to this email directly or view it on GitHub https://github.com/Civcraft/PrisonPearl/pull/47.

idoash4 commented 9 years ago

You have a file with a list of all the locations(cords) that the plugin will not release the pearls in them if they are outside of world border. Pearls can still be freed if there isn't coal in the chest. I even added a command to easily add a new location to the file: /ppwb add/remove x y z

ttk2 commented 9 years ago

That sounds about right. I will let others review. Thanks!

On Sun, May 24, 2015, 10:19 AM BlackXnt notifications@github.com wrote:

You have a file with a list of all the locations(cords) that the plugin will not release the pearls in them if they are outside of world border. Pearls can still be freed if there isn't coal in the chest. I even added a command to easily add new location to the file: /ppwb add/remove x y z

— Reply to this email directly or view it on GitHub https://github.com/Civcraft/PrisonPearl/pull/47#issuecomment-105026242.

ttk2 commented 9 years ago

bump for review.

On Sun, May 24, 2015 at 2:36 PM justin kilpatrick < kilpatrickjustin@gmail.com> wrote:

That sounds about right. I will let others review. Thanks!

On Sun, May 24, 2015, 10:19 AM BlackXnt notifications@github.com wrote:

You have a file with a list of all the locations(cords) that the plugin will not release the pearls in them if they are outside of world border. Pearls can still be freed if there isn't coal in the chest. I even added a command to easily add new location to the file: /ppwb add/remove x y z

— Reply to this email directly or view it on GitHub https://github.com/Civcraft/PrisonPearl/pull/47#issuecomment-105026242.

CivcraftBot commented 9 years ago

Can one of the admins verify this patch? Type 'ok to test' to test.

ProgrammerDan commented 9 years ago

Did a bit of a code review. I don't think this will compile as written, but in general the thrust of the code appears consistent with intent. Lots of cleanup, code style, and best practice changes; I tried to comment on each place I noted.

idoash4 commented 9 years ago

Thanks! I will go over everything and commit all the changes.

idoash4 commented 9 years ago

This should be good now. I managed to build the plugin on my pc and to run a few tests. Best way to add a new chest to the whitelist is to stand on it and type /ppwb add. Thank you @ProgrammerDan for helping me out on this.

ttk2 commented 9 years ago

merged, will this update the config?

ProgrammerDan commented 9 years ago

The config Xnt included has the new options with defaults specified, if that's what you mean?

ttk2 commented 9 years ago

Yes thats fine. But why would there be any default excluded chests? I just wanted an example.

On Wed, May 27, 2015, 12:43 AM Daniel Boston notifications@github.com wrote:

The config Xnt included has the new options with defaults specified, if that's what you mean?

— Reply to this email directly or view it on GitHub https://github.com/Civcraft/PrisonPearl/pull/47#issuecomment-105763822.

idoash4 commented 9 years ago

Not sure what are you both talking about. When you run the plugin it will automatically generate a new empty text file that will save all the cords for the whitelisted chests. If you are talking about a config option to enable the feature I didn't add a new one. The feature will work if freeOutsideWorldBorder set to true.

ProgrammerDan commented 9 years ago

Yep, my bad Xnt; you have added new configuration options but they are specifically and only related to permission to use the new Commands. I'm not sure what TTK is talking about either.

ttk2 commented 9 years ago

well I was just looking for if the new file generated or not.

On Wed, May 27, 2015 at 6:03 PM Daniel Boston notifications@github.com wrote:

Yep, my bad Xnt; you have added new configuration options but they are specifically and only related to permission to use the new Commands. I'm not sure what TTK is talking about either.

— Reply to this email directly or view it on GitHub https://github.com/Civcraft/PrisonPearl/pull/47#issuecomment-106104376.