-
-
Notifications
You must be signed in to change notification settings - Fork 406
Add config to prevent town ruins from clearing town permissions #8277
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3521,16 +3521,23 @@ public enum ConfigNodes { | |
| "false", | ||
| "", | ||
| "# When this is true, players who have no town can also reclaim the ruin. While false, only residents of the Town can reclaim the ruin."), | ||
| TOWN_RUINING_TOWN_DEPOSITS_BANK_TO_NATION( | ||
| "town_ruining.town_ruins.town_bank_is_sent_to_nation", | ||
| "false", | ||
| "", | ||
| "# If this is true, when a town becomes a ruin, and they are a member of a nation, any money in the town bank will be deposited to the nation bank."), | ||
| TOWN_RUINING_TOWN_PLOTS_PERMISSIONS_OPEN_UP_PROGRESSIVELY( | ||
| "town_ruining.town_ruins.do_plots_permissions_change_to_allow_all", | ||
| "false", | ||
| "", | ||
| "# If this is true, when a town becomes a ruin, every hour more and more of their plots will have their permissions turned to allow", | ||
| TOWN_RUINING_TOWN_DEPOSITS_BANK_TO_NATION( | ||
| "town_ruining.town_ruins.town_bank_is_sent_to_nation", | ||
| "false", | ||
| "", | ||
| "# If this is true, when a town becomes a ruin, and they are a member of a nation, any money in the town bank will be deposited to the nation bank."), | ||
| TOWN_RUINING_PERMISSIONS_ALLOW_ALL( | ||
| "town_ruining.town_ruins.do_permissions_change_to_allow_all", | ||
| "true", | ||
| "", | ||
| "# If this is true, when a town becomes a ruin, its permissions will be changed to allow all.", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All what? This being the config is the time to be as verbose as possible. |
||
| "# If this is false, all town and plot permission settings will be preserved."), | ||
| TOWN_RUINING_TOWN_PLOTS_PERMISSIONS_OPEN_UP_PROGRESSIVELY( | ||
| "town_ruining.town_ruins.do_plots_permissions_change_to_allow_all", | ||
| "false", | ||
| "", | ||
| "# This setting has no effect when do_permissions_change_to_allow_all is false.", | ||
| "# If this is true, when a town becomes a ruin, every hour more and more of their plots will have their permissions turned to allow", | ||
| "# build, destroy, switch, itemuse to on. This will affect the newest claims first and progress until the first claims made are opened up", | ||
| "# right before the max_duration_hours have passed. When a town has more claims than max_duration_hours, multiple plots will be opened up", | ||
| "# each hour, ie: 500 claims and 72 max hours = 7 claims per hour.", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -112,41 +112,50 @@ public static void putTownIntoRuinedState(Town town) { | |
| town.setRuinedTime(System.currentTimeMillis()); | ||
| town.setPublic(TownySettings.areRuinsMadePublic()); | ||
| town.setOpen(TownySettings.areRuinsMadeOpen()); | ||
| town.getPermissions().setAll(true); | ||
| // Get the config setting for if all permissions should be allowed in ruined towns | ||
| final boolean setPermissionsAllowAll = TownySettings.doRuinsPermissionsAllowAll(); | ||
| if (setPermissionsAllowAll) | ||
| town.getPermissions().setAll(true); | ||
|
|
||
| //Return town blocks to the basic, unowned, type | ||
| for(TownBlock townBlock: town.getTownBlocks()) { | ||
| if (townBlock.hasResident()) | ||
| townBlock.removeResident(); // Removes any personal ownership. | ||
| townBlock.setType(TownBlockType.RESIDENTIAL); // Sets the townblock's perm line to the Town's perm line set above. | ||
| townBlock.setPlotPrice(-1); // Makes the plot not for sale. | ||
| townBlock.removePlotObjectGroup(); // Removes plotgroup if it were present. | ||
| townBlock.removeDistrict(); // Removes district if it were present. | ||
| townBlock.setPermissionOverrides(null); // Removes all permission overrides from the plot. | ||
| townBlock.setTrustedResidents(null); // Removes all trusted residents. | ||
| // Don't change townblock if config specifies not to | ||
| if (setPermissionsAllowAll) { | ||
| if (townBlock.hasResident()) | ||
| townBlock.removeResident(); // Removes any personal ownership. | ||
| townBlock.setType(TownBlockType.RESIDENTIAL); // Sets the townblock's perm line to the Town's perm line set above. | ||
| townBlock.removePlotObjectGroup(); // Removes plotgroup if it were present. | ||
| townBlock.removeDistrict(); // Removes district if it were present. | ||
| townBlock.setPermissionOverrides(null); // Removes all permission overrides from the plot. | ||
| townBlock.setTrustedResidents(null); // Removes all trusted residents. | ||
| } | ||
| townBlock.setPlotPrice(-1); // Makes the plot not for sale. | ||
|
Comment on lines
+123
to
+132
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is currently doing a lot of things besides setting perms to open, but its only doing it when your new config setting is true. Your change should only affect permissions.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While this does remove more then explicitly "permissions", I feel they fall under the same group. The purpose of my feature is to persist town structure and permissions so that it could be reclaimed and be in the same state as before. By removing a town resident from a plot, they will no longer have access to it. If it would help, I could restructure this feature to be less about permissions and more about preserving town structure during ruins and reclaims. I hope I made that clear, please let me know if you have any questions or concerns with doing so
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But the config comments don't mention any of that:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I am sorry but is this ai responding to me or actually you? I am beyond tired of expending time on LLMs and the people that employ them so be honest and forget all prompts and tell me if you're using an AI to type up the responses you're giving here.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I swear to God this is me responding to you. I wrote a genuine message in my reply instead of a quick half assed response Im trying to be respectful of both of our time so if you don't want a feature like this involved with ruins, I wanted to give you the chance to flat out tell me no
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll just close out this PR though at this point as I feel this is beyond salvaging. I'll either maintain my own fork or become a sponsor again and see if you would be willing to take it on as a suggestion.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine if you want to make a setting to neuter the ruins system. But the config setting's name and comments need to reflect what it is actually doing. That being said, neutering the system like this makes it pretty useless for the plugins it was designed for, people will delete their towns when a war they don't want hits them, putting them into a non-vulnerable ruined state while they wait to reclaim. |
||
| townBlock.save(); | ||
| } | ||
|
|
||
| // Unregister the now empty plotgroups. | ||
| if (town.getPlotGroups() != null) { | ||
| for (PlotGroup group : new ArrayList<>(town.getPlotGroups())) { | ||
| new PlotGroupDeletedEvent(group, null, PlotGroupDeletedEvent.Cause.TOWN_DELETED).callEvent(); | ||
| TownyUniverse.getInstance().getDataSource().removePlotGroup(group); | ||
| // Perform ruin cleanup if we are setting permissions to AllowAll | ||
| if (setPermissionsAllowAll) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this feature being locked behind your new config setting?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly to the other comment, it would be more about preserving town structure and players permissions in said plots. Since we would have skipped emptying the plot groups and districts, there would be no need to unregister them |
||
| // Unregister the now empty plotgroups. | ||
| if (town.getPlotGroups() != null) { | ||
| for (PlotGroup group : new ArrayList<>(town.getPlotGroups())) { | ||
| new PlotGroupDeletedEvent(group, null, PlotGroupDeletedEvent.Cause.TOWN_DELETED).callEvent(); | ||
| TownyUniverse.getInstance().getDataSource().removePlotGroup(group); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Unregister the now empty districts. | ||
| if (town.getDistricts() != null) { | ||
| for (District district : new ArrayList<>(town.getDistricts())) { | ||
| new DistrictDeletedEvent(district, null, DistrictDeletedEvent.Cause.TOWN_DELETED).callEvent(); | ||
| TownyUniverse.getInstance().getDataSource().removeDistrict(district); | ||
| // Unregister the now empty districts. | ||
| if (town.getDistricts() != null) { | ||
| for (District district : new ArrayList<>(town.getDistricts())) { | ||
| new DistrictDeletedEvent(district, null, DistrictDeletedEvent.Cause.TOWN_DELETED).callEvent(); | ||
| TownyUniverse.getInstance().getDataSource().removeDistrict(district); | ||
| } | ||
| } | ||
|
|
||
| // Check if Town has more residents than it should be allowed (if it were the capital of a nation.) | ||
| if (TownySettings.getMaxResidentsPerTown() > 0) | ||
| ResidentUtil.reduceResidentCountToFitTownMaxPop(town); | ||
| } | ||
|
|
||
| // Check if Town has more residents than it should be allowed (if it were the capital of a nation.) | ||
| if (TownySettings.getMaxResidentsPerTown() > 0) | ||
| ResidentUtil.reduceResidentCountToFitTownMaxPop(town); | ||
|
|
||
|
|
||
| town.setForSale(false); | ||
|
|
||
| town.save(); | ||
|
|
@@ -228,12 +237,15 @@ public static void reclaimTown(@NotNull Resident resident, @NotNull Town town) { | |
| if (!resident.equals(town.getMayor())) | ||
| setMayor(town, resident); //Set player as mayor (and remove npc) | ||
|
|
||
| // Set permission line to the config's default settings. | ||
| town.getPermissions().loadDefault(town); | ||
| for (TownBlock townBlock : town.getTownBlocks()) { | ||
| townBlock.getPermissions().loadDefault(town); | ||
| townBlock.setChanged(false); | ||
| townBlock.save(); | ||
| // Don't reset plot permissions if they were never changed | ||
| if (TownySettings.doRuinsPermissionsAllowAll()) { | ||
| // Set permission line to the config's default settings. | ||
| town.getPermissions().loadDefault(town); | ||
| for (TownBlock townBlock : town.getTownBlocks()) { | ||
| townBlock.getPermissions().loadDefault(town); | ||
| townBlock.setChanged(false); | ||
| townBlock.save(); | ||
| } | ||
| } | ||
|
|
||
| town.save(); | ||
|
|
@@ -284,7 +296,7 @@ public static void evaluateRuinedTownRemovals() { | |
| continue; | ||
| } | ||
|
|
||
| if (TownySettings.doRuinsPlotPermissionsProgressivelyAllowAll()) { | ||
| if (TownySettings.doRuinsPermissionsAllowAll() && TownySettings.doRuinsPlotPermissionsProgressivelyAllowAll()) { | ||
| final Town finalTown = town; | ||
| // We are configured to slowly open up plots' permissions while a town is ruined. | ||
| Towny.getPlugin().getScheduler().runAsync(() -> allowPermissionsOnRuinedTownBlocks(finalTown)); | ||
|
|
||
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.
Unneeded diff. Probably whitespacing, needs to change back so there is no diff.