-
-
Notifications
You must be signed in to change notification settings - Fork 356
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
Various Improvements to Jail Functionality #6031
Conversation
My merge from the master seems to have gotten dragged into this pull-request, let me know if a new pull-request is wanted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done an initial review, looks quite good so far.
On top of the comments left behind, I'd appreciate it if your ifs were changed around to match the norm in towny:
if {
} else {
}
instead of
if
{
}
else {
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Realized I had done a review which I thought I had submitted but it was sitting here in Pending. So there's some older comments and some newer comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On top of the changes requested I think there's still one somewhat major thing to cover.
Currently bail is not going to be an option on every server, but the command structure has bail required.
The command is going to have to handle bail not being enabled and the command syntax being shorter (and not including a double.)
src/com/palmergames/bukkit/towny/listeners/TownyEntityMonitorListener.java
Outdated
Show resolved
Hide resolved
I've made some extensive changes in latest commit especially with regards to the command parsing. I'm unsure if my new chosen method for choosing how to provide feedback for if bail is enabled vs disabled is efficient so feedback regarding that would be especially appreciated if it's off at all. Particularly you mentioned that it should not involve a double, is it better for stability to parse it as an int then convert to a double later? |
@Kali0033 sorry it has been so long since I have looked into this. Due to changes in a couple of classes the PR needs to be rebased. |
- Settable max limit for players jailed per town - Three modes for dealing with attempted new prisoners if at capacity (rejected/prisoner with lowest remaining sentence/prisoner with lowest custom set bail) - Initial imprisonment fee with a further option to charge towns an hourly upkeep - Custom bails set either at time of jailing - Custom maximum bail in configuration - Maximum custom sentence in configuration JailReason has been largely deprecated to allow for configurable hours/bail, provisionally Outlaw and POW jailings are being loaded from config directly. Further improvements are to follow regarding this.
- Formatting cleaned up. - All economy interactions check for TownyEconomyHandler.isActive() first now. - Code for checking jail behaviour activity simplified. - Erroneous use of term deprecated from JailReason.java removed and replaced with more useful comments. - ConfigNodes.java improved to clarify money is withdrawn from Town bank. - Unnecessary differences removed. - Resident name for Jailee upkeep added for bankhistory books clarity. - Overhauled getJailed() method in Town to pull from universe.getJailedResidentMap() thus fixing issue relating to it not counting outlaws or prisoners of war.
Master branch method readded pointing to new method with bail amount supplied. Check if server has bail configuration option enabled and has economy enabled, send appropriate messaging.
Changed check in TownCommand to check for greater than or equal to 1 as opposed to 0. Hopefully finally fixed the unness difference on JailUtil.java Line 160.
Check for greater then 0 instead of greater then or equals.
Change jailResident method to send bail from settings as default if isAllowingBail and TownyEconomyHandler.isActive otherwise send 0.0 as a filler amount.
- Update comments to reflect changes to getJailed() method Changes made to TownCommand.java - Unneeded diff removed on lines 1905, 1907 - Only check if initialJailFee is greater than 0 Changes made to JailUtil.java - Suggested change implemented, unneeded use of else removed and bail only shows if greater than 0
- Added new boolean for configuration isJailBookEnabled() to address issue #6013 Changes to TownyEntityMonitorListener.java - Change method to use jailResident instead of jailResidentWithBail - Change hours provided for outlaw imprisonment to those of TownySettings.getJailedOutlawJailHours() Changes to TownCommand.java - Parse inputs differently and provide different feedback depending on if Bail is enabled or not. - Fixed unnecessary bloat for max potential jailed check Changes to Town.java - Implemented recommended changes for getJailedPlayerCount() and getJailedResidents() Changes to JailUtil.java - Add check for if isJailBookedEnabled() in configuration before sending book. - Feedback messages separated from switch and made to account for towny settings regarding bail and economy. - Formatting mistakes in sendJailedBookToResident fixed - Change to new method for Town.getJailedResidents Changes to HourlyTimerTask: - Variables declared and moved outside of loop - Merge the if block into the for loop, in relation to comment made regarding potential 20 tick delay. Changes to HelpMenu.java: - Restore TOWN_JAIL from master, add new TOWN_JAILWITHBAIL to replace functionality. Changes to en-US.yml: - Added msg_player_has_been_sent_to_jail_into_cell_number_x_for_x_hours_by_x - Readded msg_you_have_been_jailed from master Changes to ConfigNodes.java: - Added JAIL_IS_JAILBOOK_ENABLED
- Fix up spacing in ConfigNodes - Rename bail cost methods in Resident. - Redo jailing in TownCommand - To reduce duplicate code, use new features in non-bail jailing. - Fix incorrect helpmenu being used. - Reduce nesting. - Small changes to HourlyTimerTask to reduce duplicate code. - Small fixes to JailUtil.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point I think it's been cleaned up enough for merging.
- Includes the ability for mayors to set bail amounts. - Admins can limit the upper limit on bail costs. - Includes the optional ability for admins to set a jail-upkeep. - Towns can be made to pay an initial fee to jail someone. - Towns can be made to pay hourly to house prisoners. - Includes the ability to limit how many players can be jailed in one town. - There is also a configurable system that decides when happens when a town has reached their jailed-residents limit. - Includes the ability to give or not give the jail book received by newly jailed players. - New Config Options: - jail.pow_jail_hours - Default: 5 - How many hours an attacking enemy will be jailed for. - jail.maximum_jail_hours - Default: 5 - The maximum hours that a mayor can set when jailing someone, full number expected. - jail.fee_initial_amount - Default: -1 - Amount that it costs per player jailed for a town, this is withdrawn from the Town bank. Set to -1 to disable. - jail.fee_hourly_amount - Default: -1 - Amount that it costs per player jailed per hour for a town, this is withdrawn from the Town bank. Set to -1 to disable. - jail.bail.bailmax_amount - Default: 100 - Max bail cost that a mayor can set. - jail.max_jailed_count - Default: -1 - Amount of potential jailed players per town, set to -1 to disable. - jail.max_jailed_newjail_behavior - Default: 0 - Behaviour for new jail attempts if max jailed count is reached. - 0 = Unable to jail new players until a current prisoner is released. - 1 = A prisoner slot will be made by automatically releasing a prisoner based on remaining time. - 2 = A prisoner slot will be made by automatically releasing a prisoner based on lowest custom bail. - jail.is_jailbook_enabled - Default: true - If false players will not be provided with a book upon being jailed. - Changed command: /t jail. - If your server has bail allowed: jail.bail.is_allowing_bail, the jail command will now accept a bail amount: - /t jail [playername] {hours} {bail} {jail} {cell} - If bail is not allowed the old command is used: /t jail [playername] {hours} {jail} {cell}
Description:
This pull request is an improvement to the jail system in multiple ways:
New Nodes/Commands/ConfigOptions:
Relevant Towny Issue ticket:
Closes #6029 Resolves this by setting default sentence to two hours.
Closes #6008 Resolves this by allowing for a relevant configuration option.
Closes #4940 Resolves this by allowing for mayors to set bails, ticket should be kept open as further improvement via percentage opposed to flat rate possible.
Closes #6013 by making the jail book an option.
Testing Environment: Paper version git-Paper-387 (MC: 1.18.2) (Implementing API version 1.18.2-R0.1-SNAPSHOT) (Git: df630a2 on ver/1.18.2)
By making this pull request, I represent that I have the right to waive copyright and related rights to my contribution, and agree that all copyright and related rights in my contributions are waived, and I acknowledge that the TownyAdvanced organization has the copyright to use and modify my contribution under the Towny License for perpetuity.