search

Civcraft / JukeAlert

Do not open issues here; open them on the maintained fork @ DevotedMC
https://github.com/DevotedMC/JukeAlert
BSD 3-Clause "New" or "Revised" License
5 stars 15 forks source link

Feature Addition: Juke Lever Triggering #7

Closed Pentom closed 9 years ago

Pentom commented 9 years ago

Included is new code to allow new feature "Juke Lever Triggering" to exist. Specifically, this code will allow jukeboxes (not noteblocks) that have opted in to lever triggering to cause a lever to momentarily fire (~0.75 second ish) such that cardinal north when a player enters/logs in a snitch radius, cardinal south when a player opens a chest, cardinal east when a player places a block, and cardinal west when a player breaks a block. This new feature is safeguarded by new configuration setting (default false) "allowTriggeringLevers". If false, which it is by default, then there should be no server-affect of this change except a new column added to the main snitches table. If true, then it will allow users (owners/moderators of a snitch) to call new command /jaToggleLevers that will allow them to opt-in a juke box into this new functionality. It is possible to remove this functionality after it has gone live by modifying the configuration option with no harm caused. While levers were chosen for this functionality, it could easily have been buttons as they offer the same benefit (redstone pulse) and are part of the bukkit supported mechanism for said functionality - levers were kept simply because the client visible affect looks more interesting and novel. Churned plugin.yml version.

Pentom commented 9 years ago

Specification can be found via http://www.reddit.com/r/Civcraft/comments/2m2ipo/civcraft_development_juke_alert_modification/

ttk2 commented 9 years ago

very cool, I don't see any reason why I should not merge this now, but I will run it by the changelog and such, next up is testing

On Thu, Nov 13, 2014 at 12:48 AM, Pentom notifications@github.com wrote:

Specification can be found via http://www.reddit.com/r/Civcraft/comments/2m2ipo/civcraft_development_juke_alert_modification/

— Reply to this email directly or view it on GitHub https://github.com/Civcraft/JukeAlert/pull/7#issuecomment-62849064.

rourke750 commented 9 years ago

Looks all good code wise. Although for getting the south, west, north, east look into block faces. It might be easier to do.

WildWeazel commented 9 years ago

second the block faces, looks pretty good to me otherwise

Pentom commented 9 years ago

Confirmed - in retrospect, it may be easier next time to use block faces. Not enough value to fix at this point but if I am in the code here again will revisit. Thanks for the review and will watch that next time.

erocs commented 9 years ago

http://192.99.169.83:8080/job/JukeAlert-master/29/

Pentom commented 9 years ago

Good deal. Thanks.

Will add some logging functionality into the next fix I publish (for the database changes / errors) - per request. If rokurue's citadel change does not halt the scrolling screen on the other open issue, will review that instead as priority.

rourke750 commented 9 years ago

rokurue's

Way to just butcher the shit out of that.

erocs commented 9 years ago

It's ok Roro. On Nov 14, 2014 7:10 AM, "rourke750" notifications@github.com wrote:

rokurue's

Way to just butcher the shit out of that.

— Reply to this email directly or view it on GitHub https://github.com/Civcraft/JukeAlert/pull/7#issuecomment-63077167.

ttk2 commented 9 years ago

so should I be putting builds on Civtest or are things going to be changed?

On Fri, Nov 14, 2014 at 9:13 AM, erocs notifications@github.com wrote:

It's ok Roro. On Nov 14, 2014 7:10 AM, "rourke750" notifications@github.com wrote:

rokurue's

Way to just butcher the shit out of that.

— Reply to this email directly or view it on GitHub https://github.com/Civcraft/JukeAlert/pull/7#issuecomment-63077167.

— Reply to this email directly or view it on GitHub https://github.com/Civcraft/JukeAlert/pull/7#issuecomment-63077644.

rourke750 commented 9 years ago

You can put it on. You just need to modify the config to enable the changes. On Nov 14, 2014 11:31 AM, "ttk2" notifications@github.com wrote:

so should I be putting builds on Civtest or are things going to be changed?

On Fri, Nov 14, 2014 at 9:13 AM, erocs notifications@github.com wrote:

It's ok Roro. On Nov 14, 2014 7:10 AM, "rourke750" notifications@github.com wrote:

rokurue's

Way to just butcher the shit out of that.

— Reply to this email directly or view it on GitHub https://github.com/Civcraft/JukeAlert/pull/7#issuecomment-63077167.

— Reply to this email directly or view it on GitHub https://github.com/Civcraft/JukeAlert/pull/7#issuecomment-63077644.

— Reply to this email directly or view it on GitHub https://github.com/Civcraft/JukeAlert/pull/7#issuecomment-63090558.