Version 1.7.4 (#109)

* Version 1.7.3

* Add ${argLine} to get jacoco coverage

* Updated Jacoco POM entry

* Address bugs reported by SonarCloud

* Updated ReadMe

* Add max mobs option #99

* Use updated Bucket event

* Added tests to cover #99

* Fixes help text for user command.

Fixes #105

* Fixed maxmobs typo instead of maxmob

* Remove unused imports

* Update to new Bukkit Loader

* Remove debug

* Create plugin.yml (#106)

* Create plugin.yml

* Update pom.xml

* Update GreenhousesPladdon.java

* Removed static getInstance usage

* Version 1.7.4

* Refactored to reduce complexity

* Update surefire plugin

* Refactored to reduce complexity

* Minor typos and grammar fixes

* Reduced complexity

* Refactor to reduce complexity

* Refactor to reduce complexity

* Update Github Action build script

* Added distribution required for Github Action

* Update pom.xml

* Fixes mob spawning when no maxmob value given.

Found while doing #108

* Code clean up.

---------

Co-authored-by: BONNe <bonne@bonne.id.lv>

.github/workflows/build.yml

@@ -11,21 +11,22 @@ jobs:
     name: Build
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
         with:
           fetch-depth: 0  # Shallow clones should be disabled for a better relevancy of analysis
       - name: Set up JDK 17
-        uses: actions/setup-java@v1
+        uses: actions/setup-java@v3
         with:
+          distribution: 'adopt'
           java-version: 17
       - name: Cache SonarCloud packages
-        uses: actions/cache@v1
+        uses: actions/cache@v3
         with:
           path: ~/.sonar/cache
           key: ${{ runner.os }}-sonar
           restore-keys: ${{ runner.os }}-sonar
       - name: Cache Maven packages
-        uses: actions/cache@v1
+        uses: actions/cache@v3
         with:
           path: ~/.m2
           key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}

README.md

@@ -15,7 +15,7 @@ Greenhouses are made out of glass and must contain the blocks found in the Biome
 
 * Craft your own self-contained biome greenhouse on an island (or elsewhere if you like)
 * Greenhouses can grow plants that cannot normally be grown, like sunflowers
-* Friendly mobs can spawn if your greenhouse is well designed - need slimes? Build a swamp greenhouse!
+* Friendly mobs can spawn if your greenhouse is well-designed - need slimes? Build a swamp greenhouse!
 * Blocks change in biomes over time - dirt becomes sand in a desert, dirt becomes clay in a river, for example.
 * Greenhouses can run in multiple worlds.
 * Easy to use GUI shows greenhouse recipes (e.g. **/is greenhouses**)
@@ -27,7 +27,7 @@ This example is for when you are in the BSkyBlock world. For AcidIsland, just us
 
 1. Make glass blocks and build a rectangular set of walls with a flat roof.
 2. Put a hopper in the wall or roof.
-3. Put a door in the wall so you can get in and out.
+3. Put a door in the wall, so you can get in and out.
 4. Type **/island greenhouses** and read the rules for the greenhouse you want.
 5. Exit the GUI and place blocks, water, lava, and ice so that you make your desired biome.
 6. Type **/island greenhouses** again and click on the biome to make it.
@@ -42,7 +42,7 @@ This example is for when you are in the BSkyBlock world. For AcidIsland, just us
 
 ## FAQ
 
-* Can I use stained glass? Yes, you can. It's pretty.
+* Can I use stained-glass? Yes, you can. It's pretty.
 * Can I fill my greenhouse full of water? Yes. That's an ocean.
 * Will a squid spawn there? Maybe... okay, yes it will if it's a big enough ocean.
 * How do I place a door high up in the wall if the wall is all glass? Place it on a hopper.

pom.xml

@@ -51,7 +51,7 @@
         <!-- Revision variable removes warning about dynamic version -->
         <revision>${build.version}-SNAPSHOT</revision>
         <!-- This allows to change between versions and snapshots. -->
-        <build.version>1.7.3</build.version>
+        <build.version>1.7.4</build.version>
         <build.number>-LOCAL</build.number>
         <sonar.projectKey>BentoBoxWorld_Greenhouses</sonar.projectKey>
         <sonar.organization>bentobox-world</sonar.organization>
@@ -191,7 +191,7 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-surefire-plugin</artifactId>
-                <version>3.0.0-M5</version>
+                <version>3.1.0</version>
                 <configuration>
                     <argLine>
                         ${argLine}
@@ -252,7 +252,7 @@
             <plugin>
                 <groupId>org.jacoco</groupId>
                 <artifactId>jacoco-maven-plugin</artifactId>
-                <version>0.8.7</version>
+                <version>0.8.10</version>
                 <configuration>
                     <append>true</append>
                     <excludes>

src/main/java/world/bentobox/greenhouses/Greenhouses.java

@@ -11,6 +11,7 @@
 import world.bentobox.bentobox.api.flags.Flag;
 import world.bentobox.bentobox.api.flags.Flag.Mode;
 import world.bentobox.bentobox.api.flags.Flag.Type;
+import world.bentobox.greenhouses.greenhouse.Walls;
 import world.bentobox.greenhouses.managers.GreenhouseManager;
 import world.bentobox.greenhouses.managers.RecipeManager;
 import world.bentobox.greenhouses.ui.user.UserCommand;
@@ -28,19 +29,13 @@ public class Greenhouses extends Addon {
     public static final Flag GREENHOUSES = new Flag.Builder("GREENHOUSE", Material.GREEN_STAINED_GLASS)
             .mode(Mode.BASIC)
             .type(Type.PROTECTION).build();
-    private static Greenhouses instance;
     private final Config<Settings> config;
 
-    public static Greenhouses getInstance() {
-        return instance;
-    }
-
     /**
      * Constructor
      */
     public Greenhouses() {
         super();
-        instance = this;
         config = new Config<>(this, Settings.class);
     }
 
@@ -122,4 +117,15 @@ public List<World> getActiveWorlds() {
         return activeWorlds;
     }
 
+    /**
+     * Check if material is a wall material
+     * @param m - material
+     * @return true if wall material
+     */
+    public boolean wallBlocks(Material m) {
+        return Walls.WALL_BLOCKS.contains(m)
+                || (m.equals(Material.GLOWSTONE) && getSettings().isAllowGlowstone())
+                || (m.name().endsWith("GLASS_PANE") && getSettings().isAllowPanes());
+    }
+
 }

src/main/java/world/bentobox/greenhouses/greenhouse/BiomeRecipe.java

@@ -209,24 +209,30 @@ public CompletableFuture<Set<GreenhouseResult>> checkRecipe(Greenhouse gh) {
      * @return set of results from the check
      */
     private Set<GreenhouseResult> checkRecipeAsync(CompletableFuture<Set<GreenhouseResult>> r, Greenhouse gh) {
-        AsyncWorldCache cache = new AsyncWorldCache(gh.getWorld());
-        Set<GreenhouseResult> result = new HashSet<>();
+        AsyncWorldCache cache = new AsyncWorldCache(addon, gh.getWorld());
         long area = gh.getArea();
-        Map<Material, Integer> blockCount = new EnumMap<>(Material.class);
 
         // Look through the greenhouse and count what is in there
-        for (int y = gh.getFloorHeight(); y< gh.getCeilingHeight();y++) {
-            for (int x = (int) (gh.getBoundingBox().getMinX()+1); x < gh.getBoundingBox().getMaxX(); x++) {
-                for (int z = (int) (gh.getBoundingBox().getMinZ()+1); z < gh.getBoundingBox().getMaxZ(); z++) {
-                    Material t = cache.getBlockType(x, y, z);
-                    if (!t.equals(Material.AIR)) {
-                        blockCount.putIfAbsent(t, 0);
-                        blockCount.merge(t, 1, Integer::sum);
-                    }
-                }
-            }
+        Map<Material, Integer> blockCount = countBlocks(gh, cache);
+
+        // Calculate % water, ice and lava ratios and check them
+        Set<GreenhouseResult> result = checkRatios(blockCount, area);
+
+        // Compare to the required blocks
+        Map<Material, Integer> missingBlocks = requiredBlocks.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue() - blockCount.getOrDefault(e.getKey(), 0)));
+        // Remove any entries that are 0 or less
+        missingBlocks.values().removeIf(v -> v <= 0);
+        if (!missingBlocks.isEmpty()) {
+            result.add(GreenhouseResult.FAIL_INSUFFICIENT_BLOCKS);
+            gh.setMissingBlocks(missingBlocks);
         }
-        // Calculate % water, ice and lava ratios
+        // Return to main thread to complete
+        Bukkit.getScheduler().runTask(addon.getPlugin(), () -> r.complete(result));
+        return result;
+    }
+
+    private Set<GreenhouseResult> checkRatios(Map<Material, Integer> blockCount, long area) {
+        Set<GreenhouseResult> result = new HashSet<>();
         double waterRatio = (double)blockCount.getOrDefault(Material.WATER, 0)/area * 100;
         double lavaRatio = (double)blockCount.getOrDefault(Material.LAVA, 0)/area * 100;
         int ice = blockCount.entrySet().stream().filter(en -> en.getKey().equals(Material.ICE)
@@ -254,45 +260,55 @@ private Set<GreenhouseResult> checkRecipeAsync(CompletableFuture<Set<GreenhouseR
         if (iceCoverage > 0 && iceRatio < iceCoverage) {
             result.add(GreenhouseResult.FAIL_INSUFFICIENT_ICE);
         }
-        // Compare to the required blocks
-        Map<Material, Integer> missingBlocks = requiredBlocks.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue() - blockCount.getOrDefault(e.getKey(), 0)));
-        // Remove any entries that are 0 or less
-        missingBlocks.values().removeIf(v -> v <= 0);
-        if (!missingBlocks.isEmpty()) {
-            result.add(GreenhouseResult.FAIL_INSUFFICIENT_BLOCKS);
-            gh.setMissingBlocks(missingBlocks);
-        }
-        // Return to main thread to complete
-        Bukkit.getScheduler().runTask(addon.getPlugin(), () -> r.complete(result));
         return result;
     }
 
+    private Map<Material, Integer> countBlocks(Greenhouse gh, AsyncWorldCache cache) {
+        Map<Material, Integer> blockCount = new EnumMap<>(Material.class);
+        for (int y = gh.getFloorHeight(); y< gh.getCeilingHeight();y++) {
+            for (int x = (int) (gh.getBoundingBox().getMinX()+1); x < gh.getBoundingBox().getMaxX(); x++) {
+                for (int z = (int) (gh.getBoundingBox().getMinZ()+1); z < gh.getBoundingBox().getMaxZ(); z++) {
+                    Material t = cache.getBlockType(x, y, z);
+                    if (!t.equals(Material.AIR)) {
+                        blockCount.putIfAbsent(t, 0);
+                        blockCount.merge(t, 1, Integer::sum);
+                    }
+                }
+            }
+        }
+        return blockCount;
+    }
+
     /**
      * Check if block should be converted
      * @param b - block to check
      */
     public void convertBlock(Block b) {
         Material bType  = b.getType();
-        // Check if there is a block conversion for this block, as while the rest of the method wont do anything if .get() returns nothing anyway it still seems to be quite expensive
+        // Check if there is a block conversion for this block, as while the rest of the method won't do anything if .get() returns nothing anyway it still seems to be quite expensive
         if(conversionBlocks.keySet().contains(bType)) {
-            for(GreenhouseBlockConversions conversion_option : conversionBlocks.get(bType)) {
-
-                // Roll the dice before bothering with checking the surrounding block as I think it's more common for greenhouses to be filled with convertable blocks and thus this dice roll wont be "wasted"
-                if(ThreadLocalRandom.current().nextDouble() < conversion_option.probability()) {
-                    // Check if any of the adjacent blocks matches the required LocalMaterial, if there are any required LocalMaterials
-                    if(conversion_option.localMaterial() != null) {
-                        for(BlockFace adjacent_block : ADJ_BLOCKS) {
-                            if(b.getRelative(adjacent_block).getType() == conversion_option.localMaterial()) {
-                                b.setType(conversion_option.newMaterial());
-                                break;
-                            }
-                        }
-                    } else {
+            for(GreenhouseBlockConversions conversionOption : conversionBlocks.get(bType)) {
+                rollTheDice(b, conversionOption);
+            }
+        }
+    }
+
+    private void rollTheDice(Block b, GreenhouseBlockConversions conversion_option) {
+        // Roll the dice before bothering with checking the surrounding block as I think it's more common for greenhouses to be filled with convertable blocks and thus this dice roll wont be "wasted"
+        if(ThreadLocalRandom.current().nextDouble() < conversion_option.probability()) {
+            // Check if any of the adjacent blocks matches the required LocalMaterial, if there are any required LocalMaterials
+            if(conversion_option.localMaterial() != null) {
+                for(BlockFace adjacent_block : ADJ_BLOCKS) {
+                    if(b.getRelative(adjacent_block).getType() == conversion_option.localMaterial()) {
                         b.setType(conversion_option.newMaterial());
+                        break;
                     }
                 }
+            } else {
+                b.setType(conversion_option.newMaterial());
             }
         }
+
     }
 
     /**
@@ -376,7 +392,7 @@ public boolean spawnMob(Block b) {
         }
         // Center spawned mob
         Location spawnLoc = b.getLocation().clone().add(new Vector(0.5, 0, 0.5));
-        boolean result = getRandomMob()
+        return getRandomMob()
                 // Check if the spawn on block matches, if it exists
                 .filter(m -> Optional.of(m.mobSpawnOn())
                         .map(b.getRelative(BlockFace.DOWN).getType()::equals)
@@ -396,7 +412,6 @@ public boolean spawnMob(Block b) {
                                 return true;
                             }).orElse(false);
                 }).orElse(false);
-        return result;
     }
 
     /**
@@ -435,7 +450,10 @@ private Optional<GreenhousePlant> getRandomPlant(boolean underwater) {
         // Grow a random plant that can grow
         double r = random.nextDouble();
         Double key = underwater ? underwaterPlants.ceilingKey(r) : plantTree.ceilingKey(r);
-        return key == null ? Optional.empty() : Optional.ofNullable(underwater ? underwaterPlants.get(key) : plantTree.get(key));
+        if (key == null) {
+            return Optional.empty();
+        }
+        return Optional.ofNullable(underwater ? underwaterPlants.get(key) : plantTree.get(key));
     }
 
     /**
@@ -461,11 +479,9 @@ public int getWaterCoverage() {
     public boolean growPlant(GrowthBlock block, boolean underwater) {
         Block bl = block.block();
         return getRandomPlant(underwater).map(p -> {
-            if (bl.getY() != 0 && canGrowOn(block, p)) {
-                if (plantIt(bl, p)) {
-                    bl.getWorld().spawnParticle(Particle.SNOWBALL, bl.getLocation(), 10, 2, 2, 2);
-                    return true;
-                }
+            if (bl.getY() != 0 && canGrowOn(block, p) && plantIt(bl, p)) {
+                bl.getWorld().spawnParticle(Particle.SNOWBALL, bl.getLocation(), 10, 2, 2, 2);
+                return true;
             }
             return false;
         }).orElse(false);
@@ -554,14 +570,12 @@ private boolean placeLichen(Block bl) {
         BlockFace d = null;
         boolean waterLogged = false;
         for (BlockFace adj : ADJ_BLOCKS) {
-            if (b.getRelative(adj).getType().equals(Material.AIR)) {
+            Material type = b.getRelative(adj).getType();
+            if (type.equals(Material.AIR) || type.equals(Material.WATER)) {
                 d = adj;
-                break;
-            }
-            // Lichen can grow under water too
-            if (b.getRelative(adj).getType().equals(Material.WATER)) {
-                d = adj;
-                waterLogged = true;
+                if (type.equals(Material.WATER)) {
+                    waterLogged = true;
+                }
                 break;
             }
         }