From e1ab389449a553d5c9dda804ee8ce3f10e97e383 Mon Sep 17 00:00:00 2001 From: Goosius1 Date: Sat, 24 Jun 2023 19:39:40 +0100 Subject: [PATCH 1/3] Fix for LV job --- pom.xml | 2 +- .../townyprovinces/TownyProvinces.java | 1 + .../LandValidationTaskController.java | 43 ++--------------- .../land_validation/LandvalidationTask.java | 47 +++++++++++++++++-- .../RegenerateRegionTask.java | 7 ++- 5 files changed, 55 insertions(+), 45 deletions(-) diff --git a/pom.xml b/pom.xml index cb6e79d..8822d25 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ 4.0.0 io.github.townyadvanced TownyProvinces - 1.4.1 + 1.4.2 townyprovinces diff --git a/src/main/java/io/github/townyadvanced/townyprovinces/TownyProvinces.java b/src/main/java/io/github/townyadvanced/townyprovinces/TownyProvinces.java index 61c4148..44565e1 100644 --- a/src/main/java/io/github/townyadvanced/townyprovinces/TownyProvinces.java +++ b/src/main/java/io/github/townyadvanced/townyprovinces/TownyProvinces.java @@ -30,6 +30,7 @@ public class TownyProvinces extends JavaPlugin { * to avoid concurrent modification problems */ public static final Object DYNMAP_DISPLAY_LOCK = new Object(); + public static final Object LAND_VALIDATION_LOCK = new Object(); private static TownyProvinces plugin; private static final Version requiredTownyVersion = Version.fromString("0.99.1.0"); diff --git a/src/main/java/io/github/townyadvanced/townyprovinces/jobs/land_validation/LandValidationTaskController.java b/src/main/java/io/github/townyadvanced/townyprovinces/jobs/land_validation/LandValidationTaskController.java index ce06819..f1502ba 100644 --- a/src/main/java/io/github/townyadvanced/townyprovinces/jobs/land_validation/LandValidationTaskController.java +++ b/src/main/java/io/github/townyadvanced/townyprovinces/jobs/land_validation/LandValidationTaskController.java @@ -2,46 +2,30 @@ import com.palmergames.bukkit.towny.object.Translatable; import io.github.townyadvanced.townyprovinces.TownyProvinces; -import io.github.townyadvanced.townyprovinces.data.DataHandlerUtil; -import io.github.townyadvanced.townyprovinces.data.TownyProvincesDataHolder; import io.github.townyadvanced.townyprovinces.messaging.Messaging; -import io.github.townyadvanced.townyprovinces.objects.Province; public class LandValidationTaskController { private static LandValidationJobStatus landValidationJobStatus; static { - if(areAnyValidationsPending()) { - landValidationJobStatus = LandValidationJobStatus.PAUSED; - } else { - landValidationJobStatus = LandValidationJobStatus.STOPPED; - } + //Could actually be paused, so this could be misleading + //Todo - Maybe improve in future. You would need to do a read in a separate thread to determine if paused + landValidationJobStatus = LandValidationJobStatus.STOPPED; } private static LandvalidationTask landValidationTask = null; - public static boolean startTask() { - /* - * If there are any requests pending, just start the job - * otherwise request all provinces - */ - if(!areAnyValidationsPending()) { - setLandValidationRequestsForAllProvinces(true); - } + public static void startTask() { landValidationTask = new LandvalidationTask(); landValidationTask.runTaskAsynchronously(TownyProvinces.getPlugin()); landValidationJobStatus = LandValidationJobStatus.STARTED; Messaging.sendGlobalMessage(Translatable.of("msg_land_validation_job_started")); - return true; } public static void stopTask() { if(landValidationTask != null) { landValidationTask.cancel(); landValidationTask = null; - setLandValidationRequestsForAllProvinces(false); //Clear all requests landValidationJobStatus = LandValidationJobStatus.STOPPED; - TownyProvinces.info("Land Validation Task: Saving data"); - DataHandlerUtil.saveAllData(); Messaging.sendGlobalMessage(Translatable.of("msg_land_validation_job_stopped")); } } @@ -51,8 +35,6 @@ public static void pauseTask() { landValidationTask.cancel(); landValidationTask = null; landValidationJobStatus = LandValidationJobStatus.PAUSED; - TownyProvinces.info("Land Validation Task: Saving data"); - DataHandlerUtil.saveAllData(); Messaging.sendGlobalMessage(Translatable.of("msg_land_validation_job_paused")); } } @@ -62,15 +44,6 @@ public static void restartTask() { startTask(); } - private static void setLandValidationRequestsForAllProvinces(boolean value) { - for(Province province: TownyProvincesDataHolder.getInstance().getProvincesSet()) { - if(province.isLandValidationRequested() != value) { - province.setLandValidationRequested(value); - province.saveData(); - } - } - } - public static LandValidationJobStatus getLandValidationJobStatus() { return landValidationJobStatus; } @@ -79,14 +52,6 @@ public static void setLandValidationJobStatus(LandValidationJobStatus status) { landValidationJobStatus = status; } - private static boolean areAnyValidationsPending() { - for(Province province: TownyProvincesDataHolder.getInstance().getProvincesSet()) { - if(province.isLandValidationRequested()) { - return true; - } - } - return false; - } } \ No newline at end of file diff --git a/src/main/java/io/github/townyadvanced/townyprovinces/jobs/land_validation/LandvalidationTask.java b/src/main/java/io/github/townyadvanced/townyprovinces/jobs/land_validation/LandvalidationTask.java index 720fed7..76f8ed3 100644 --- a/src/main/java/io/github/townyadvanced/townyprovinces/jobs/land_validation/LandvalidationTask.java +++ b/src/main/java/io/github/townyadvanced/townyprovinces/jobs/land_validation/LandvalidationTask.java @@ -1,7 +1,10 @@ package io.github.townyadvanced.townyprovinces.jobs.land_validation; +import com.palmergames.bukkit.towny.object.Translatable; import io.github.townyadvanced.townyprovinces.TownyProvinces; +import io.github.townyadvanced.townyprovinces.data.DataHandlerUtil; import io.github.townyadvanced.townyprovinces.data.TownyProvincesDataHolder; +import io.github.townyadvanced.townyprovinces.messaging.Messaging; import io.github.townyadvanced.townyprovinces.objects.Province; import io.github.townyadvanced.townyprovinces.objects.TPCoord; import io.github.townyadvanced.townyprovinces.settings.TownyProvincesSettings; @@ -18,9 +21,37 @@ public class LandvalidationTask extends BukkitRunnable { @Override public void run() { - //Execute the land validation job - TownyProvinces.info("Land Validation Job Starting."); - executeLandValidation(); + TownyProvinces.info("Acquiring land validation lock."); + synchronized (TownyProvinces.LAND_VALIDATION_LOCK) { + TownyProvinces.info("Land validation lock acquired."); + TownyProvinces.info("Land Validation Job Starting."); + /* + * If there are no requests pending, + * this is a fresh start, so request all provinces + */ + if(!areAnyValidationsPending()) { + setLandValidationRequestsForAllProvinces(true); + } + executeLandValidation(); + } + } + + private boolean areAnyValidationsPending() { + for(Province province: TownyProvincesDataHolder.getInstance().getProvincesSet()) { + if(province.isLandValidationRequested()) { + return true; + } + } + return false; + } + + private void setLandValidationRequestsForAllProvinces(boolean value) { + for(Province province: TownyProvincesDataHolder.getInstance().getProvincesSet()) { + if(province.isLandValidationRequested() != value) { + province.setLandValidationRequested(value); + province.saveData(); + } + } } /** @@ -59,12 +90,22 @@ private void executeLandValidation() { LandValidationJobStatus landValidationJobStatus = LandValidationTaskController.getLandValidationJobStatus(); switch (landValidationJobStatus) { case STOP_REQUESTED: + TownyProvinces.info("Land Validation Task: Clearing all validation requests"); + setLandValidationRequestsForAllProvinces(false); //Clear all requests + TownyProvinces.info("Land Validation Task: Saving data"); + DataHandlerUtil.saveAllData(); LandValidationTaskController.stopTask(); return; case PAUSE_REQUESTED: + TownyProvinces.info("Land Validation Task: Saving data"); + DataHandlerUtil.saveAllData(); LandValidationTaskController.pauseTask(); return; case RESTART_REQUESTED: + TownyProvinces.info("Land Validation Task: Clearing all validation requests"); + setLandValidationRequestsForAllProvinces(false); //Clear all requests + TownyProvinces.info("Land Validation Task: Saving data"); + DataHandlerUtil.saveAllData(); LandValidationTaskController.restartTask(); return; } diff --git a/src/main/java/io/github/townyadvanced/townyprovinces/jobs/province_generation/RegenerateRegionTask.java b/src/main/java/io/github/townyadvanced/townyprovinces/jobs/province_generation/RegenerateRegionTask.java index 2616df4..be6b500 100644 --- a/src/main/java/io/github/townyadvanced/townyprovinces/jobs/province_generation/RegenerateRegionTask.java +++ b/src/main/java/io/github/townyadvanced/townyprovinces/jobs/province_generation/RegenerateRegionTask.java @@ -42,10 +42,13 @@ public RegenerateRegionTask(String regionName) { public void run() { try { TownyProvinces.info("Regeneration Job Started"); - TownyProvinces.info("Regeneration Job: Acquiring dynmap display lock"); + TownyProvinces.info("Regeneration Job: Acquiring synchronization locks"); synchronized (TownyProvinces.DYNMAP_DISPLAY_LOCK) { TownyProvinces.info("Regeneration Job: Dynmap display lock acquired"); - executeRegionRegenerationJob(); + synchronized (TownyProvinces.LAND_VALIDATION_LOCK) { + TownyProvinces.info("Regeneration Job: Land Validation lock acquired"); + executeRegionRegenerationJob(); + } } } finally { TownyProvinces.info("Regeneration Job: Dynmap display lock released"); From 1a08d3c301fed14d0cf1f052837eb1c8ae843e49 Mon Sep 17 00:00:00 2001 From: Goosius1 Date: Sat, 24 Jun 2023 19:43:33 +0100 Subject: [PATCH 2/3] Improved feedback --- .../jobs/province_generation/RegenerateRegionTask.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/github/townyadvanced/townyprovinces/jobs/province_generation/RegenerateRegionTask.java b/src/main/java/io/github/townyadvanced/townyprovinces/jobs/province_generation/RegenerateRegionTask.java index be6b500..a89a9ed 100644 --- a/src/main/java/io/github/townyadvanced/townyprovinces/jobs/province_generation/RegenerateRegionTask.java +++ b/src/main/java/io/github/townyadvanced/townyprovinces/jobs/province_generation/RegenerateRegionTask.java @@ -42,9 +42,10 @@ public RegenerateRegionTask(String regionName) { public void run() { try { TownyProvinces.info("Regeneration Job Started"); - TownyProvinces.info("Regeneration Job: Acquiring synchronization locks"); + TownyProvinces.info("Regeneration Job: Acquiring dynmap display lock"); synchronized (TownyProvinces.DYNMAP_DISPLAY_LOCK) { TownyProvinces.info("Regeneration Job: Dynmap display lock acquired"); + TownyProvinces.info("Regeneration Job: Acquiring land validation lock"); synchronized (TownyProvinces.LAND_VALIDATION_LOCK) { TownyProvinces.info("Regeneration Job: Land Validation lock acquired"); executeRegionRegenerationJob(); From ae20efa0eed65f178fa25bea2930a4afc132249b Mon Sep 17 00:00:00 2001 From: Goosius1 Date: Sat, 24 Jun 2023 19:48:19 +0100 Subject: [PATCH 3/3] small usabliity improveent --- .../province_generation/RegenerateRegionTask.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/github/townyadvanced/townyprovinces/jobs/province_generation/RegenerateRegionTask.java b/src/main/java/io/github/townyadvanced/townyprovinces/jobs/province_generation/RegenerateRegionTask.java index a89a9ed..f482498 100644 --- a/src/main/java/io/github/townyadvanced/townyprovinces/jobs/province_generation/RegenerateRegionTask.java +++ b/src/main/java/io/github/townyadvanced/townyprovinces/jobs/province_generation/RegenerateRegionTask.java @@ -42,12 +42,12 @@ public RegenerateRegionTask(String regionName) { public void run() { try { TownyProvinces.info("Regeneration Job Started"); - TownyProvinces.info("Regeneration Job: Acquiring dynmap display lock"); - synchronized (TownyProvinces.DYNMAP_DISPLAY_LOCK) { - TownyProvinces.info("Regeneration Job: Dynmap display lock acquired"); - TownyProvinces.info("Regeneration Job: Acquiring land validation lock"); - synchronized (TownyProvinces.LAND_VALIDATION_LOCK) { - TownyProvinces.info("Regeneration Job: Land Validation lock acquired"); + TownyProvinces.info("Regeneration Job: Acquiring land validation lock"); + synchronized (TownyProvinces.LAND_VALIDATION_LOCK) { + TownyProvinces.info("Regeneration Job: Land Validation lock acquired"); + TownyProvinces.info("Regeneration Job: Acquiring dynmap display lock"); + synchronized (TownyProvinces.DYNMAP_DISPLAY_LOCK) { + TownyProvinces.info("Regeneration Job: Dynmap display lock acquired"); executeRegionRegenerationJob(); } }