From 14de07d01eea2f6e45f78f3132b305c5728e6db5 Mon Sep 17 00:00:00 2001 From: Atrius Date: Thu, 21 May 2026 19:00:40 -0400 Subject: [PATCH] fix: #136 Add more robust block safety checks The existing checks used are fairly naive about how certain blocks get handled. The check largely worked, but blocks such as signs and other non-obstructing blocks would count as "solid", and this prevent the player from utilizing them in circumstances where the concern is merely teleport safety. To instead ensure that users can get a bit more creative with their options, a new safety service was introduced which adds handling for more explicit edge cases, and enables a new configuration option to determine if liquids count as obstructing blocks. --- build.gradle.kts | 1 + gradle/libs.versions.toml | 1 + .../config/property/SafeLiquidsProperty.kt | 17 ++ .../data/config/property/type/SafeLiquids.kt | 8 + .../waystones/service/BlockSafetyService.kt | 54 ++++ .../waystones/service/WaystoneService.kt | 4 +- .../xyz/atrius/waystones/utility/Location.kt | 12 - src/main/resources/config.yml | 1 + src/main/resources/locale-en.yml | 1 + .../service/BlockSafetyServiceTest.kt | 276 ++++++++++++++++++ .../atrius/waystones/test/ServerFunSpec.kt | 5 + 11 files changed, 366 insertions(+), 14 deletions(-) create mode 100644 src/main/kotlin/xyz/atrius/waystones/data/config/property/SafeLiquidsProperty.kt create mode 100644 src/main/kotlin/xyz/atrius/waystones/data/config/property/type/SafeLiquids.kt create mode 100644 src/main/kotlin/xyz/atrius/waystones/service/BlockSafetyService.kt create mode 100644 src/test/kotlin/xyz/atrius/waystones/service/BlockSafetyServiceTest.kt diff --git a/build.gradle.kts b/build.gradle.kts index 7dd499b9..b4c91a55 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -24,6 +24,7 @@ dependencies { detektPlugins(libs.detekt.ktlint) testImplementation(libs.mockk) testImplementation(libs.kotest.runner) + testImplementation(libs.kotest.datatest) testImplementation(libs.mockbukkit) testImplementation("io.papermc.paper:paper-api:$buildPaperVersion.build.+") } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index e363d4be..9e7f7515 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -31,6 +31,7 @@ mysql-connector = { module = "com.mysql:mysql-connector-j", version.ref = "mysql detekt-ktlint = { module = "dev.detekt:detekt-rules-ktlint-wrapper", version.ref = "detekt" } mockk = { module = "io.mockk:mockk", version.ref = "mockk" } kotest-runner = { module = "io.kotest:kotest-runner-junit5", version.ref = "kotest" } +kotest-datatest = { module = "io.kotest:kotest-framework-datatest", version.ref = "kotest" } mockbukkit = { module = "org.mockbukkit.mockbukkit:mockbukkit-v26.1.2", version.ref = "mockbukkit" } [plugins] diff --git a/src/main/kotlin/xyz/atrius/waystones/data/config/property/SafeLiquidsProperty.kt b/src/main/kotlin/xyz/atrius/waystones/data/config/property/SafeLiquidsProperty.kt new file mode 100644 index 00000000..cedaaee3 --- /dev/null +++ b/src/main/kotlin/xyz/atrius/waystones/data/config/property/SafeLiquidsProperty.kt @@ -0,0 +1,17 @@ +package xyz.atrius.waystones.data.config.property + +import org.koin.core.annotation.Single +import xyz.atrius.waystones.command.resolver.EnumArgumentType +import xyz.atrius.waystones.data.config.ConfigProperty +import xyz.atrius.waystones.data.config.property.type.SafeLiquids +import xyz.atrius.waystones.utility.sanitizedStringFormat + +@Single(binds = [ConfigProperty::class]) +class SafeLiquidsProperty : ConfigProperty( + property = "safe-liquids", + default = SafeLiquids.WATER, + parser = EnumArgumentType(SafeLiquids::class), + propertyType = SafeLiquids::class, + format = { it.sanitizedStringFormat() }, + serialize = { it.name }, +) diff --git a/src/main/kotlin/xyz/atrius/waystones/data/config/property/type/SafeLiquids.kt b/src/main/kotlin/xyz/atrius/waystones/data/config/property/type/SafeLiquids.kt new file mode 100644 index 00000000..6c196e55 --- /dev/null +++ b/src/main/kotlin/xyz/atrius/waystones/data/config/property/type/SafeLiquids.kt @@ -0,0 +1,8 @@ +package xyz.atrius.waystones.data.config.property.type + +@Suppress("unused") +enum class SafeLiquids { + NONE, + WATER, + ALL, +} diff --git a/src/main/kotlin/xyz/atrius/waystones/service/BlockSafetyService.kt b/src/main/kotlin/xyz/atrius/waystones/service/BlockSafetyService.kt new file mode 100644 index 00000000..92e27a45 --- /dev/null +++ b/src/main/kotlin/xyz/atrius/waystones/service/BlockSafetyService.kt @@ -0,0 +1,54 @@ +package xyz.atrius.waystones.service + +import org.bukkit.Location +import org.bukkit.Material +import org.koin.core.annotation.Single +import xyz.atrius.waystones.data.config.property.SafeLiquidsProperty +import xyz.atrius.waystones.data.config.property.type.SafeLiquids +import xyz.atrius.waystones.utility.UP + +@Single +class BlockSafetyService( + private val safeLiquids: SafeLiquidsProperty, +) { + // Blocks that are non-collidable but still hazardous to teleport into. + // These deal damage or apply harmful effects to entities occupying their space. + private val hazardBlocks = setOf( + Material.FIRE, + Material.SOUL_FIRE, + Material.CAMPFIRE, + Material.SOUL_CAMPFIRE, + Material.SWEET_BERRY_BUSH, + Material.WITHER_ROSE, + ) + + fun isLocationSafe(waystone: Location): Boolean { + val feet = waystone.UP + val head = feet.UP + // Check the two blocks above the waystone (head and feet clearance). + // Uses isCollidable which reflects actual entity collision geometry, + // unlike isSolid which incorrectly marks signs/pressure plates as solid. + return checkBlock(feet) && + checkBlock(head) + } + + private fun checkBlock(checkLocation: Location): Boolean { + val block = checkLocation.world + ?.getBlockAt(checkLocation) + ?: return false + val material = block.type + // Physical obstruction check: catches doors, potted plants, fences, + // redstone components with physical presence, all full blocks, + // and any blocks that may cause damage to the player + if (block.isCollidable || material in hazardBlocks) { + return false + } + // Liquid check: configurable via safe-liquids property. + // Liquids have no collision but can still be dangerous or undesirable to land in. + return when (material) { + Material.LAVA -> safeLiquids.value() == SafeLiquids.ALL + Material.WATER -> safeLiquids.value() != SafeLiquids.NONE + else -> true + } + } +} diff --git a/src/main/kotlin/xyz/atrius/waystones/service/WaystoneService.kt b/src/main/kotlin/xyz/atrius/waystones/service/WaystoneService.kt index 3a4594ee..584ade32 100644 --- a/src/main/kotlin/xyz/atrius/waystones/service/WaystoneService.kt +++ b/src/main/kotlin/xyz/atrius/waystones/service/WaystoneService.kt @@ -26,7 +26,6 @@ import xyz.atrius.waystones.manager.LocalizationManager import xyz.atrius.waystones.manager.LocalizedString import xyz.atrius.waystones.repository.WaystoneInfoRepository import xyz.atrius.waystones.utility.isActive -import xyz.atrius.waystones.utility.isSafe import xyz.atrius.waystones.utility.powerBlock import xyz.atrius.waystones.utility.sameDimension @@ -45,6 +44,7 @@ class WaystoneService( private val worldRatioService: WorldRatioService, private val limitDistance: LimitDistanceProperty, private val waystoneInfoRepository: WaystoneInfoRepository, + private val blockSafetyService: BlockSafetyService, ) { fun process(player: Player, block: Block, keyLocation: Location): Either = either { @@ -209,7 +209,7 @@ class WaystoneService( return WaystoneStatus.Unpowered(localization) } - if (!block.location.isSafe) { + if (!blockSafetyService.isLocationSafe(block.location)) { return WaystoneStatus.Obstructed(localization) } diff --git a/src/main/kotlin/xyz/atrius/waystones/utility/Location.kt b/src/main/kotlin/xyz/atrius/waystones/utility/Location.kt index bfd2c33a..3ccbffe3 100644 --- a/src/main/kotlin/xyz/atrius/waystones/utility/Location.kt +++ b/src/main/kotlin/xyz/atrius/waystones/utility/Location.kt @@ -44,18 +44,6 @@ val Location.neighbors: List val Location.locationCode get() = "${world?.name}@$blockX:$blockY:$blockZ" -// Determines if the selected block is safe to spawn on -val Location.isSafe: Boolean - get() = !listOf(UP, UP.UP) - .map { - world - ?.getBlockAt(it) - ?.type - ?.isSolid - ?: true - } - .any { it } - fun Location.rotateY(angle: Double, amp: Double = 1.0) = add( Vector( cos(angle) * amp, diff --git a/src/main/resources/config.yml b/src/main/resources/config.yml index 17a6709a..8755faf7 100644 --- a/src/main/resources/config.yml +++ b/src/main/resources/config.yml @@ -19,6 +19,7 @@ relinkable-keys: true enable-key-items: true enable-advancements: true show-compass-coordinates: true +safe-liquids: WATER key-recipe: - AIR - IRON_INGOT diff --git a/src/main/resources/locale-en.yml b/src/main/resources/locale-en.yml index 5838a8dd..d6474eb8 100644 --- a/src/main/resources/locale-en.yml +++ b/src/main/resources/locale-en.yml @@ -94,6 +94,7 @@ property-warp-animations-info: "Whether to use particle animations while warping property-default-world-ratio-info: "The world ratio to use for worlds which have no ratio set" property-base-distance-info: "The baseline for which all waystones are given as a minimum teleport distance" property-show-compass-coordinates-info: "Whether to show waystone coordinates in the compass lore upon linking" +property-safe-liquids-info: "Determines which liquids are considered safe when obstructing a waystone" default-subcommand-desc: "No description provided" ws-ratio-subcommand-desc: "Adjust the warp distance ratios between worlds" ws-reload-subcommand-desc: "Reload aspects of the plugin" diff --git a/src/test/kotlin/xyz/atrius/waystones/service/BlockSafetyServiceTest.kt b/src/test/kotlin/xyz/atrius/waystones/service/BlockSafetyServiceTest.kt new file mode 100644 index 00000000..a0e5120e --- /dev/null +++ b/src/test/kotlin/xyz/atrius/waystones/service/BlockSafetyServiceTest.kt @@ -0,0 +1,276 @@ +package xyz.atrius.waystones.service + +import io.kotest.datatest.withData +import io.kotest.matchers.shouldBe +import io.mockk.every +import io.mockk.mockk +import org.bukkit.Location +import org.bukkit.Material +import xyz.atrius.waystones.data.config.property.SafeLiquidsProperty +import xyz.atrius.waystones.data.config.property.type.SafeLiquids +import xyz.atrius.waystones.test.ServerFunSpec + +class BlockSafetyServiceTest : ServerFunSpec({ + + val checkLocation by lazy { + Location(world, 0.0, 64.0, 0.0) + } + + fun subject(policy: SafeLiquids): BlockSafetyService { + val property = mockk() + every { property.value() } returns policy + return BlockSafetyService(property) + } + + fun clearBlocksAbove() { + world.getBlockAt(0, 65, 0).type = Material.AIR + world.getBlockAt(0, 66, 0).type = Material.AIR + } + + fun placeBlockAtFeet(material: Material) { + world.getBlockAt(0, 65, 0).type = material + } + + fun placeBlockAtHead(material: Material) { + world.getBlockAt(0, 66, 0).type = material + } + + context("BlockSafetyService") { + + context("isLocationSafe") { + + test("Returns true when all blocks above are air") { + clearBlocksAbove() + val service = subject(SafeLiquids.ALL) + + service.isLocationSafe(checkLocation) shouldBe true + } + + test("Returns false when world is null") { + val nullLocation = Location(null, 0.0, 65.0, 0.0) + val service = subject(SafeLiquids.ALL) + + service.isLocationSafe(nullLocation) shouldBe false + } + + context("Physical obstructions") { + + test("Returns false when a collidable block is above at Y+1") { + clearBlocksAbove() + placeBlockAtFeet(Material.STONE) + val service = subject(SafeLiquids.ALL) + + service.isLocationSafe(checkLocation) shouldBe false + } + + test("Returns false when a collidable block is above at Y+2") { + clearBlocksAbove() + placeBlockAtHead(Material.GLASS) + val service = subject(SafeLiquids.ALL) + + service.isLocationSafe(checkLocation) shouldBe false + } + } + + context("Signs should not obstruct") { + withData( + Material.OAK_SIGN, + Material.SPRUCE_SIGN, + Material.BIRCH_SIGN, + Material.JUNGLE_SIGN, + Material.ACACIA_SIGN, + Material.DARK_OAK_SIGN, + Material.MANGROVE_SIGN, + Material.CHERRY_SIGN, + Material.BAMBOO_SIGN, + Material.CRIMSON_SIGN, + Material.WARPED_SIGN, + Material.OAK_WALL_SIGN, + Material.SPRUCE_WALL_SIGN, + Material.OAK_HANGING_SIGN, + Material.SPRUCE_HANGING_SIGN, + Material.OAK_WALL_HANGING_SIGN, + ) { material -> + clearBlocksAbove() + placeBlockAtFeet(material) + val service = subject(SafeLiquids.ALL) + + service.isLocationSafe(checkLocation) shouldBe true + } + } + + context("Doors should obstruct") { + withData( + Material.OAK_DOOR, + Material.SPRUCE_DOOR, + Material.BIRCH_DOOR, + Material.JUNGLE_DOOR, + Material.ACACIA_DOOR, + Material.DARK_OAK_DOOR, + Material.MANGROVE_DOOR, + Material.CHERRY_DOOR, + Material.BAMBOO_DOOR, + Material.CRIMSON_DOOR, + Material.WARPED_DOOR, + Material.IRON_DOOR, + ) { material -> + clearBlocksAbove() + placeBlockAtFeet(material) + val service = subject(SafeLiquids.ALL) + + service.isLocationSafe(checkLocation) shouldBe false + } + } + + context("Potted plants should obstruct") { + withData( + Material.FLOWER_POT, + Material.POTTED_OAK_SAPLING, + Material.POTTED_RED_MUSHROOM, + Material.POTTED_CACTUS, + Material.POTTED_BAMBOO, + ) { material -> + clearBlocksAbove() + placeBlockAtFeet(material) + val service = subject(SafeLiquids.ALL) + + service.isLocationSafe(checkLocation) shouldBe false + } + } + + context("Redstone components") { + + context("Physical redstone components should obstruct") { + withData( + Material.REPEATER, + Material.COMPARATOR, + Material.OBSERVER, + Material.TARGET, + Material.DAYLIGHT_DETECTOR, + Material.NOTE_BLOCK, + Material.SCULK_SENSOR, + Material.SCULK_CATALYST, + Material.SCULK_SHRIEKER, + Material.CALIBRATED_SCULK_SENSOR, + Material.REDSTONE_BLOCK, + Material.REDSTONE_LAMP, + ) { material -> + clearBlocksAbove() + placeBlockAtFeet(material) + val service = subject(SafeLiquids.ALL) + + service.isLocationSafe(checkLocation) shouldBe false + } + } + + context("Non-physical redstone components should not obstruct") { + withData( + Material.REDSTONE_WIRE, + Material.REDSTONE_TORCH, + Material.REDSTONE_WALL_TORCH, + Material.LEVER, + Material.STONE_PRESSURE_PLATE, + Material.OAK_PRESSURE_PLATE, + Material.STONE_BUTTON, + Material.OAK_BUTTON, + Material.TRIPWIRE, + Material.TRIPWIRE_HOOK, + Material.SCULK_VEIN, + ) { material -> + clearBlocksAbove() + placeBlockAtFeet(material) + val service = subject(SafeLiquids.ALL) + + service.isLocationSafe(checkLocation) shouldBe true + } + } + } + + context("Torches should not obstruct") { + withData( + Material.TORCH, + Material.SOUL_TORCH, + Material.WALL_TORCH, + Material.SOUL_WALL_TORCH, + ) { material -> + clearBlocksAbove() + placeBlockAtFeet(material) + val service = subject(SafeLiquids.ALL) + + service.isLocationSafe(checkLocation) shouldBe true + } + } + + context("Hazard blocks should obstruct") { + withData( + Material.FIRE, + Material.SOUL_FIRE, + Material.CAMPFIRE, + Material.SOUL_CAMPFIRE, + Material.SWEET_BERRY_BUSH, + Material.WITHER_ROSE, + ) { material -> + clearBlocksAbove() + placeBlockAtFeet(material) + val service = subject(SafeLiquids.ALL) + + service.isLocationSafe(checkLocation) shouldBe false + } + } + + context("Liquid policy") { + + context("Blocks lava when policy is NONE") { + + test("NONE blocks lava") { + clearBlocksAbove() + placeBlockAtFeet(Material.LAVA) + val service = subject(SafeLiquids.NONE) + + service.isLocationSafe(checkLocation) shouldBe false + } + + test("WATER blocks lava") { + clearBlocksAbove() + placeBlockAtFeet(Material.LAVA) + val service = subject(SafeLiquids.WATER) + + service.isLocationSafe(checkLocation) shouldBe false + } + + test("ALL allows lava") { + clearBlocksAbove() + placeBlockAtFeet(Material.LAVA) + val service = subject(SafeLiquids.ALL) + + service.isLocationSafe(checkLocation) shouldBe true + } + } + + test("Blocks water when policy is NONE") { + clearBlocksAbove() + placeBlockAtFeet(Material.WATER) + val service = subject(SafeLiquids.NONE) + + service.isLocationSafe(checkLocation) shouldBe false + } + + test("Allows water when policy is WATER") { + clearBlocksAbove() + placeBlockAtFeet(Material.WATER) + val service = subject(SafeLiquids.WATER) + + service.isLocationSafe(checkLocation) shouldBe true + } + + test("Allows water when policy is ALL") { + clearBlocksAbove() + placeBlockAtFeet(Material.WATER) + val service = subject(SafeLiquids.ALL) + + service.isLocationSafe(checkLocation) shouldBe true + } + } + } + } +}) diff --git a/src/test/kotlin/xyz/atrius/waystones/test/ServerFunSpec.kt b/src/test/kotlin/xyz/atrius/waystones/test/ServerFunSpec.kt index 071b7eda..e58d09c5 100644 --- a/src/test/kotlin/xyz/atrius/waystones/test/ServerFunSpec.kt +++ b/src/test/kotlin/xyz/atrius/waystones/test/ServerFunSpec.kt @@ -12,6 +12,7 @@ import org.bukkit.entity.Player import org.bukkit.event.block.Action import org.bukkit.event.player.PlayerInteractEvent import org.bukkit.inventory.ItemStack +import org.koin.core.context.stopKoin import org.mockbukkit.mockbukkit.MockBukkit import org.mockbukkit.mockbukkit.ServerMock import xyz.atrius.waystones.Waystones @@ -42,10 +43,12 @@ abstract class ServerFunSpec private constructor() : FunSpec() { private var _server: ServerMock? = null private var _plugin: Waystones? = null private var _koin: org.koin.core.Koin? = null + private var _world: World? = null internal val server: ServerMock get() = _server!! internal val plugin: Waystones get() = _plugin!! internal val koin: org.koin.core.Koin get() = _koin!! + internal val world: World get() = _world!! constructor(body: ServerFunSpec.() -> Unit = {}) : this() { extensions( @@ -54,11 +57,13 @@ abstract class ServerFunSpec private constructor() : FunSpec() { _server = MockBukkit.mock() _plugin = MockBukkit.load(Waystones::class.java) _koin = _plugin!!.getKoinApp().koin + _world = _server!!.addSimpleWorld("test_world") try { execute(spec) } finally { MockBukkit.unmock() + stopKoin() } } }