Skip to content

Commit 51e3e90

Browse files
tastybentoclaude
andcommitted
Address Copilot review comments on PR #20
- MobSpawnReplacement.resolveOldEntityType / resolveNewEntityType: use toUpperCase(Locale.ROOT) so config parsing stays stable across server locales (Turkish 'i' vs 'I' problem). Reviewer comments at MobSpawnReplacement.java:67 and :90. - config.yml + README: rewrite the per-gamemode override note to describe the actual fallback-on-miss semantics. The current listener falls through to global chances when no per-gamemode rule fires (wrong entity / chance roll failed), and that behaviour is enshrined by testPerGameModeNetherChanceZeroFallsBackToGlobal / testPerGameModeNetherReplacementEntityMismatchFallsBackToGlobal — the docs were wrong, not the code. Reviewer comments at config.yml:31 and README.md:78. - CLAUDE.md: refresh stale build/test guidance for the 1.15.0 toolchain (Java 21 / Paper 1.21.11 / BentoBox 3.14.0-SNAPSHOT, MockBukkit + JUnit 5, Pladdon entry point, per-gamemode rule semantics with the test names to look for before changing the listener). Reviewer comment at CLAUDE.md:15. - .github/workflows/build.yml: split mvn verify from the Sonar invocation, gate the Sonar step on the PR coming from the same repo so PRs from forks (where SONAR_TOKEN is not exposed) still get a green Maven build instead of failing during analysis. Reviewer comment at build.yml:38. 42/42 tests still green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a96a099 commit 51e3e90

5 files changed

Lines changed: 30 additions & 14 deletions

File tree

.github/workflows/build.yml

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,14 @@ jobs:
3131
path: ~/.m2
3232
key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
3333
restore-keys: ${{ runner.os }}-m2
34-
- name: Build and analyze
34+
- name: Build with Maven
35+
run: mvn -B verify
36+
- name: Analyze with SonarCloud
37+
# SONAR_TOKEN is not exposed to PRs from forks, so skip the Sonar step
38+
# in that case rather than failing the workflow. Pushes and PRs from
39+
# the same repo still get analysed.
40+
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }}
3541
env:
3642
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any
3743
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
38-
run: mvn -B verify org.sonarsource.scanner.maven:sonar-maven-plugin:sonar -Dsonar.projectKey=BentoBoxWorld_ExtraMobs
44+
run: mvn -B org.sonarsource.scanner.maven:sonar-maven-plugin:sonar -Dsonar.projectKey=BentoBoxWorld_ExtraMobs

CLAUDE.md

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,35 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
44

55
## Project
66

7-
ExtraMobs is a BentoBox addon (Bukkit/Spigot plugin) that re-skins certain natural mob spawns inside GameMode-managed worlds: Zombified Piglins → Blaze/Wither Skeleton in the Nether, Endermen → Shulkers in the End, and Fish → Guardians in deep-ocean overworld biomes. The addon does not alter Minecraft's spawn rules — it listens for natural spawns and conditionally cancels + re-spawns a different entity.
7+
ExtraMobs is a BentoBox addon (Paper plugin) that re-skins certain natural mob spawns inside GameMode-managed worlds: Zombified Piglins / Piglins → Blaze/Wither Skeleton in the Nether, Endermen → Shulkers in the End, and Fish → Guardians in deep-ocean overworld biomes. The addon does not alter Minecraft's spawn rules — it listens for natural spawns and conditionally cancels + re-spawns a different entity. As of 1.15.0 it also ships a Pladdon entry point so it can be loaded directly by Paper as well as via BentoBox.
88

99
## Build & test
1010

1111
- `mvn clean package` — default goal; produces `target/ExtraMobs-<version>-LOCAL.jar`.
1212
- `mvn test` — runs the test suite.
1313
- `mvn test -Dtest=MobsSpawnListenerTest` — single test class.
1414
- `mvn test -Dtest=MobsSpawnListenerTest#methodName` — single test method.
15-
- Java 17 (`<java.version>17</java.version>` in pom.xml). Targets Spigot 1.21.3 and BentoBox 2.7.1-SNAPSHOT.
15+
- Java 21 (`<java.version>21</java.version>` in pom.xml). Targets Paper 1.21.11 and BentoBox 3.14.0-SNAPSHOT.
1616
- Build versioning is driven by Maven profiles: `-LOCAL` by default, `-b<BUILD_NUMBER>` under Jenkins CI, and a clean release version when `GIT_BRANCH=origin/master`. Don't hand-edit version strings — change `build.version` in `pom.xml`.
1717

1818
## Architecture
1919

20-
Tiny codebase, three production classes:
20+
Tiny codebase, four production classes:
2121

2222
- `ExtraMobsAddon` (`src/main/java/world/bentobox/extramobs/`) — extends `world.bentobox.bentobox.api.addons.Addon`. In `onLoad()` it loads `Settings` via BentoBox's `Config<>` (auto-creates `config.yml` from `src/main/resources/`). In `onEnable()` it iterates `getAddonsManager().getGameModeAddons()`, sets `hooked=true` if any GameMode is not in `disabledGameModes`, and registers `MobsSpawnListener`. If nothing hooks, the addon disables itself.
23-
- `config.Settings``ConfigObject` with `@StoreAt(filename="config.yml", path="addons/ExtraMobs")`. Field annotations (`@ConfigEntry`, `@ConfigComment`) drive both YAML parsing and the on-disk comment block; getters/setters are mandatory for the BentoBox config framework to bind values.
24-
- `listeners.MobsSpawnListener` — single `@EventHandler(priority=HIGHEST, ignoreCancelled=true)` on `CreatureSpawnEvent`. Only `SpawnReason.NATURAL` events are considered. Flow: resolve the GameMode via `plugin.getIWM().getAddon(world)`, bail if disabled, then branch by entity type + environment (`isIslandNether` / `isIslandEnd` / `World.Environment.NORMAL` + biome). Each branch checks a "suitable block" predicate (nether brick / purpur / prismarine, with slab+stairs variants) and rolls against the configured chance. On a successful roll the event is cancelled and `world.spawnEntity()` summons the replacement.
23+
- `ExtraMobsPladdon` — extends `Pladdon`. Lets Paper load the addon directly as a plugin (paired with `src/main/resources/plugin.yml`); returns a fresh `ExtraMobsAddon` from `getAddon()`.
24+
- `config.Settings``ConfigObject` with `@StoreAt(filename="config.yml", path="addons/ExtraMobs")`. Field annotations (`@ConfigEntry`, `@ConfigComment`) drive both YAML parsing and the on-disk comment block; getters/setters are mandatory for the BentoBox config framework to bind values. The `gamemode-settings` field is stored as `Map<String, Object>` because BentoBox's config layer doesn't model the nested-list-of-maps shape; `getReplacements(gameMode, env)` parses the raw structure into `MobSpawnReplacement` records on read.
25+
- `config.MobSpawnReplacement` — POJO for one per-gamemode rule (`old` mob, `new` mob, `chance`). `resolveOldEntityType()` / `resolveNewEntityType()` upper-case via `Locale.ROOT` then call `EntityType.valueOf` defensively.
26+
- `listeners.MobsSpawnListener` — single `@EventHandler(priority=HIGHEST, ignoreCancelled=true)` on `CreatureSpawnEvent`. Only `SpawnReason.NATURAL` events are considered. Flow: `resolveActiveGameMode(world)` returns the GameMode name (or `null` if absent / disabled), then `onEntitySpawn` dispatches by entity type + environment to `handleNetherSpawn` / `handleEndSpawn` / `handleOverworldSpawn`. Each handler first checks a "suitable block" predicate (nether brick / purpur / prismarine, with slab+stairs variants), then calls `applyGameModeReplacements` for per-gamemode rules, and only falls back to the global `nether-chances` / `end-chances` / `overworld-chance` values if no per-gamemode rule fires for that event. The four block sets are pre-computed `Set<Material>` statics on the class.
2527

2628
The "suitable location" helpers encode the design rule that drives the addon: replacement is gated on the player having built a themed structure. Changes to spawn rules almost always live in these predicates plus the dispatch branches in `onEntitySpawn`.
2729

30+
Per-gamemode rules **supplement** the globals rather than suppress them — a per-gamemode rule with `chance: 0.05` for `ZOMBIFIED_PIGLIN→WITHER_SKELETON` falls through to the global wither/blaze chances on a miss, and `PIGLIN` (different entity) always falls through. The tests `testPerGameModeNetherChanceZeroFallsBackToGlobal` and `testPerGameModeNetherReplacementEntityMismatchFallsBackToGlobal` enshrine this behaviour — keep it in mind before changing the listener.
31+
2832
## Testing notes
2933

30-
- JUnit 4 + Mockito + PowerMock (`@RunWith(PowerMockRunner.class)`). Bukkit's static `Server`/`Registry`/`Tag` are stubbed via `listeners/mocks/ServerMocks.newServer()` — call this in `@Before` whenever a test touches Bukkit statics. The surefire plugin's long `--add-opens` argLine is required for PowerMock under Java 17; don't strip it.
31-
- Jacoco excludes `**/*Names*` and `org/bukkit/Material*` to avoid synthetic-field / "Material too large to mock" failures. New tests that need to mock `Material` should rely on `ServerMocks` rather than reintroducing PowerMock static-mocking on `Material`.
34+
- JUnit 5 + Mockito 5 + MockBukkit (`org.mockbukkit.mockbukkit:mockbukkit-v1.21`). Test classes extend `CommonTestSetup` which calls `MockBukkit.mock()` in `@BeforeEach` and tears down in `@AfterEach`; it also injects the `BentoBox` singleton via `WhiteBox.setInternalState(BentoBox.class, "instance", plugin)` and statically stubs `Bukkit` + `Util`. The surefire plugin's long `--add-opens` argLine plus the leading `@{argLine}` (which late-binds the Jacoco prepare-agent javaagent) are required; don't strip either.
35+
- Jacoco excludes `**/*Names*` and `org/bukkit/Material*` to avoid synthetic-field / "Material too large to mock" failures. Coverage is reported to SonarCloud via the GitHub Actions workflow.
3236

3337
## Resources & packaging
3438

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ All chance values are decimals between `0.0` (never) and `1.0` (always). For the
7575

7676
### Per-Gamemode Overrides
7777

78-
You can define replacement rules that only apply to a specific game mode. Each gamemode may set rules for the `nether`, `end`, and/or `world` (overworld) environments. **When per-gamemode rules are present for an environment, the global defaults above are not applied for that gamemode.**
78+
You can define replacement rules that only apply to a specific game mode. Each gamemode may set rules for the `nether`, `end`, and/or `world` (overworld) environments. Rules are tried in order before the global defaults. **If a rule matches the spawning entity and its chance roll succeeds, the replacement is applied and processing stops; otherwise the global defaults above are used as a fallback** — so per-gamemode rules supplement the globals rather than replace them, and you can keep the globals as a safety net for any entity the per-gamemode block doesn't cover.
7979

8080
```yaml
8181
gamemode-settings:

src/main/java/world/bentobox/extramobs/config/MobSpawnReplacement.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package world.bentobox.extramobs.config;
22

33

4+
import java.util.Locale;
5+
46
import org.bukkit.entity.EntityType;
57

68

@@ -64,7 +66,7 @@ public EntityType resolveOldEntityType()
6466
}
6567
try
6668
{
67-
return EntityType.valueOf(old.toUpperCase());
69+
return EntityType.valueOf(old.toUpperCase(Locale.ROOT));
6870
}
6971
catch (IllegalArgumentException e)
7072
{
@@ -87,7 +89,7 @@ public EntityType resolveNewEntityType()
8789
}
8890
try
8991
{
90-
return EntityType.valueOf(newMob.toUpperCase());
92+
return EntityType.valueOf(newMob.toUpperCase(Locale.ROOT));
9193
}
9294
catch (IllegalArgumentException e)
9395
{

src/main/resources/config.yml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,12 @@ overworld-chance:
2727
# old: EntityType name of the mob to replace (e.g. ZOMBIFIED_PIGLIN)
2828
# new: EntityType name of the replacement mob (e.g. WITHER_SKELETON)
2929
# chance: Probability in the range 0.0-1.0
30-
# When per-gamemode rules are present for an environment, the global defaults
31-
# for that environment are NOT applied to that gamemode.
30+
# Per-gamemode rules are tried in order before the global defaults for that
31+
# environment. If a rule matches the spawning entity AND its chance roll
32+
# succeeds, the replacement is applied and processing stops for that event.
33+
# If no rule matches (different entity), or every matching rule's chance roll
34+
# fails, the global nether-chances / end-chances / overworld-chance values
35+
# above are applied as a fallback.
3236
# Example:
3337
# gamemode-settings:
3438
# BSkyBlock:

0 commit comments

Comments
 (0)