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.

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

Feature Addition: Juke Lever Triggering #7

Merged
merged 1 commit into from
Nov 14, 2014
Merged

Conversation

Pentom
Copy link

@Pentom Pentom commented Nov 13, 2014

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.

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
Copy link
Author

Pentom commented Nov 13, 2014

@ttk2
Copy link

ttk2 commented Nov 13, 2014

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
#7 (comment).

@rourke750
Copy link

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

@WildWeazel
Copy link

second the block faces, looks pretty good to me otherwise

@Pentom
Copy link
Author

Pentom commented Nov 14, 2014

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 added a commit that referenced this pull request Nov 14, 2014
Feature Addition:  Juke Lever Triggering
@erocs erocs merged commit 1c82e61 into Civcraft:master Nov 14, 2014
@erocs
Copy link

erocs commented Nov 14, 2014

@Pentom
Copy link
Author

Pentom commented Nov 14, 2014

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
Copy link

rokurue's

Way to just butcher the shit out of that.

@erocs
Copy link

erocs commented Nov 14, 2014

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
#7 (comment).

@ttk2
Copy link

ttk2 commented Nov 14, 2014

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
#7 (comment).


Reply to this email directly or view it on GitHub
#7 (comment).

@rourke750
Copy link

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
#7 (comment).


Reply to this email directly or view it on GitHub
#7 (comment).


Reply to this email directly or view it on GitHub
#7 (comment).

Maxopoly pushed a commit to Maxopoly/JukeAlert that referenced this pull request Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants