Sign resilience and correctness fixes

sign-paper/src/main/kotlin/app/simplecloud/plugin/sign/paper/PaperSignStateManager.kt

@@ -2,7 +2,6 @@ package app.simplecloud.plugin.sign.paper
 
 import app.simplecloud.plugin.sign.paper.dispatcher.PaperPlatformDispatcher
 import app.simplecloud.plugin.sign.shared.command.SignStateManager
-import app.simplecloud.plugin.sign.shared.config.location.SignLocation
 import kotlinx.coroutines.withContext
 import net.kyori.adventure.text.Component
 import org.bukkit.Location
@@ -31,20 +30,4 @@ class PaperSignStateManager(
             sign.update(true)
         }
     }
-
-    override suspend fun updateSign(location: SignLocation, lines: List<Component>) {
-        withContext(dispatcher.getDispatcher()) {
-            val mappedLocation = bootstrap.signManager.map(location)
-
-            (mappedLocation as? Sign)?.apply {
-                arrayOf(Side.FRONT, Side.BACK).forEach { side ->
-                    getSide(side).apply {
-                        lines().indices.forEach { i -> line(i, lines[i]) }
-                    }
-                }
-
-                update(true)
-            }
-        }
-    }
 }

sign-paper/src/main/kotlin/app/simplecloud/plugin/sign/paper/PaperSignsPluginBootstrap.kt

@@ -7,6 +7,7 @@ import app.simplecloud.plugin.sign.paper.rule.PlayerRuleContext
 import app.simplecloud.plugin.sign.paper.sender.PaperCommandSender
 import app.simplecloud.plugin.sign.paper.sender.PaperCommandSenderMapper
 import app.simplecloud.plugin.sign.paper.service.PaperSignService
+import app.simplecloud.plugin.sign.paper.util.signFacing
 import app.simplecloud.plugin.sign.shared.CloudSign
 import app.simplecloud.plugin.sign.shared.SignManager
 import app.simplecloud.plugin.sign.shared.command.SignCommand
@@ -26,7 +27,6 @@ import org.bukkit.Location
 import org.bukkit.Material
 import org.bukkit.block.Sign
 import org.bukkit.block.sign.Side
-import org.bukkit.block.data.Directional
 import org.bukkit.plugin.java.JavaPlugin
 import org.incendo.cloud.execution.ExecutionCoordinator
 import org.incendo.cloud.paper.PaperCommandManager
@@ -139,8 +139,8 @@ class PaperSignsPluginBootstrap : PluginBootstrap {
             return
         }
 
-        val directional = sign.block.blockData as? Directional ?: return
-        val behindBlock = sign.block.getRelative(directional.facing.oppositeFace)
+        val facing = sign.block.blockData.signFacing() ?: return
+        val behindBlock = sign.block.getRelative(facing.oppositeFace)
 
         if (behindBlock.type != material) {
             behindBlock.type = material

sign-paper/src/main/kotlin/app/simplecloud/plugin/sign/paper/sender/PaperCommandSender.kt

@@ -1,18 +1,20 @@
 package app.simplecloud.plugin.sign.paper.sender
 
 import app.simplecloud.plugin.sign.paper.PaperSignsPlugin
+import app.simplecloud.plugin.sign.paper.util.resolveSignDirection
 import app.simplecloud.plugin.sign.shared.config.location.SignLocation
 import app.simplecloud.plugin.sign.shared.sender.SignCommandSender
 import app.simplecloud.plugin.sign.shared.utils.SignCommandMessages
 import io.papermc.paper.command.brigadier.CommandSourceStack
 import kotlinx.coroutines.withContext
 import net.kyori.adventure.text.Component
 import net.kyori.adventure.text.minimessage.MiniMessage
+import org.bukkit.Bukkit
 import org.bukkit.FluidCollisionMode
+import org.bukkit.Location
 import org.bukkit.block.Sign
 import org.bukkit.entity.Player
 
-@Suppress("UnstableApiUsage")
 class PaperCommandSender(
     val sourceStack: CommandSourceStack
 ) : SignCommandSender {
@@ -24,10 +26,9 @@ class PaperCommandSender(
         val player = sourceStack.sender as? Player ?: return null
 
         return withContext(PaperSignsPlugin.instance.bootstrap.platformDispatcher.getDispatcher()) {
-            val targetBlock =
-                player.getTargetBlockExact(maxDistance, FluidCollisionMode.NEVER) ?: return@withContext null
+            val targetBlock = player.getTargetBlockExact(maxDistance, FluidCollisionMode.NEVER)
 
-            if (targetBlock.state !is Sign) {
+            if (targetBlock == null || targetBlock.state !is Sign) {
                 player.sendMessage(MiniMessage.miniMessage().deserialize(SignCommandMessages.SIGN_NOT_FOUND))
                 return@withContext null
             }
@@ -36,8 +37,18 @@ class PaperCommandSender(
                 world = targetBlock.world.name,
                 x = targetBlock.x.toDouble(),
                 y = targetBlock.y.toDouble(),
-                z = targetBlock.z.toDouble()
+                z = targetBlock.z.toDouble(),
+                direction = targetBlock.blockData.resolveSignDirection()
             )
         }
     }
+
+    override suspend fun teleport(location: SignLocation): Boolean {
+        val player = sourceStack.sender as? Player ?: return false
+        val world = Bukkit.getWorld(location.world) ?: return false
+
+        return withContext(PaperSignsPlugin.instance.bootstrap.platformDispatcher.getDispatcher()) {
+            player.teleport(Location(world, location.x, location.y, location.z))
+        }
+    }
 }

sign-paper/src/main/kotlin/app/simplecloud/plugin/sign/paper/service/PaperSignService.kt

@@ -2,6 +2,7 @@ package app.simplecloud.plugin.sign.paper.service
 
 import app.simplecloud.api.CloudApi
 import app.simplecloud.plugin.sign.paper.PaperSignsPluginBootstrap
+import app.simplecloud.plugin.sign.paper.util.resolveSignDirection
 import app.simplecloud.plugin.sign.shared.CloudSign
 import app.simplecloud.plugin.sign.shared.LocationMapper
 import app.simplecloud.plugin.sign.shared.config.location.LocationsConfig
@@ -55,6 +56,7 @@ class PaperSignService(private val bootstrap: PaperSignsPluginBootstrap) : SignS
             location.world.name,
             location.x,
             location.y,
-            location.z
+            location.z,
+            location.block.blockData.resolveSignDirection()
         )
 }

sign-paper/src/main/kotlin/app/simplecloud/plugin/sign/paper/util/SignFacing.kt

@@ -0,0 +1,42 @@
+package app.simplecloud.plugin.sign.paper.util
+
+import org.bukkit.block.BlockFace
+import org.bukkit.block.data.BlockData
+import org.bukkit.block.data.Rotatable
+import org.bukkit.block.data.type.WallSign
+
+/**
+ * Resolves the direction a sign block is facing, independent of whether it's a
+ * wall sign (6-way facing) or a standing sign (16-way rotation).
+ */
+fun BlockData.signFacing(): BlockFace? = when (this) {
+    is WallSign -> facing
+    is Rotatable -> rotation.toCardinal()
+    else -> null
+}
+
+fun BlockData.resolveSignDirection(): String? = when (this) {
+    is WallSign -> "WALL:${facing.name}"
+    is Rotatable -> "STANDING:${rotation.name}"
+    else -> null
+}
+
+/**
+ * Standing-sign rotation is 16-way; snaps it down to the nearest of the 4
+ * horizontal cardinal faces so it can be used to find an adjacent block.
+ */
+private fun BlockFace.toCardinal(): BlockFace = when (this) {
+    BlockFace.NORTH_NORTH_WEST, BlockFace.NORTH, BlockFace.NORTH_NORTH_EAST, BlockFace.NORTH_WEST ->
+        BlockFace.NORTH
+
+    BlockFace.EAST_NORTH_EAST, BlockFace.EAST, BlockFace.EAST_SOUTH_EAST, BlockFace.NORTH_EAST ->
+        BlockFace.EAST
+
+    BlockFace.SOUTH_SOUTH_EAST, BlockFace.SOUTH, BlockFace.SOUTH_SOUTH_WEST, BlockFace.SOUTH_EAST ->
+        BlockFace.SOUTH
+
+    BlockFace.WEST_SOUTH_WEST, BlockFace.WEST, BlockFace.WEST_NORTH_WEST, BlockFace.SOUTH_WEST ->
+        BlockFace.WEST
+
+    else -> BlockFace.NORTH
+}

sign-shared/src/main/kotlin/app/simplecloud/plugin/sign/shared/SignManager.kt

@@ -40,6 +40,7 @@ class SignManager<T : Any>(
     )
     private val layoutRepository = LayoutRepository(
         directoryPath.resolve("layouts"),
+        ruleRegistry,
     )
 
     private val serializers = TypeSerializerCollection.defaults().childBuilder().apply {
@@ -160,16 +161,16 @@ class SignManager<T : Any>(
 
     private fun startUpdateSignJob() {
         updateJob = scope.launch {
-            try {
-                while (isActive) {
+            while (isActive) {
+                try {
                     updateLayoutIndexes()
                     updateSigns()
-                    delay(UPDATE_INTERVAL)
+                } catch (exception: CancellationException) {
+                    throw exception
+                } catch (exception: Exception) {
+                    logger.error("Error in update job", exception)
                 }
-            } catch (exception: CancellationException) {
-                throw exception
-            } catch (exception: Exception) {
-                logger.error("Error in update job", exception)
+                delay(UPDATE_INTERVAL)
             }
         }
     }
@@ -264,6 +265,7 @@ class SignManager<T : Any>(
 
     private suspend fun updateLayoutIndexes() {
         layoutRepository.getAll().forEach { layout ->
+            if (layout.frames.isEmpty()) return@forEach
             if (state.shouldUpdateFrame(layout.name, layout.frameUpdateInterval)) {
                 state.updateFrameIndex(layout.name, layout.frames.size)
             }

sign-shared/src/main/kotlin/app/simplecloud/plugin/sign/shared/command/SignCommand.kt

@@ -8,6 +8,7 @@ import app.simplecloud.plugin.sign.shared.sender.SignCommandSender
 import app.simplecloud.plugin.sign.shared.service.SignService
 import app.simplecloud.plugin.sign.shared.utils.SignCommandMessages
 import app.simplecloud.plugin.sign.shared.utils.SignCommandPermission
+import com.google.common.base.Suppliers
 import com.google.common.cache.CacheBuilder
 import kotlinx.coroutines.*
 import kotlinx.coroutines.future.await
@@ -17,6 +18,7 @@ import net.kyori.adventure.text.minimessage.tag.resolver.Placeholder
 import org.incendo.cloud.Command
 import org.incendo.cloud.CommandManager
 import org.incendo.cloud.description.Description
+import org.incendo.cloud.parser.standard.DoubleParser
 import org.incendo.cloud.parser.standard.IntegerParser
 import org.incendo.cloud.parser.standard.StringParser
 import org.incendo.cloud.suggestion.BlockingSuggestionProvider
@@ -41,6 +43,13 @@ class SignCommand<C : SignCommandSender, T>(
         .expireAfterWrite(5, TimeUnit.MINUTES)
         .build<String, Component>()
 
+    private val groupSuggestionsSupplier = Suppliers.memoizeWithExpiration(
+        { fetchGroupSuggestions() }, 10, TimeUnit.SECONDS
+    )
+    private val persistentServerSuggestionsSupplier = Suppliers.memoizeWithExpiration(
+        { fetchPersistentServerSuggestions() }, 10, TimeUnit.SECONDS
+    )
+
     companion object {
         private const val LOCATIONS_PER_PAGE = 5
         private const val MAX_SIGN_DISTANCE = 8
@@ -72,6 +81,7 @@ class SignCommand<C : SignCommandSender, T>(
                 createAddPersistentCommand(baseCommand),
                 createRemoveCommand(baseCommand),
                 createRemoveGroupCommand(baseCommand),
+                createTpCommand(baseCommand),
             ).forEach { command(it) }
         }
     }
@@ -123,7 +133,7 @@ class SignCommand<C : SignCommandSender, T>(
                 commandScope.launch {
                     handleSignOperation(
                         context.sender(),
-                        context.get("group"),
+                        context["group"],
                         SignOperation.ADD
                     ) { location, group ->
                         executeAddGroupSign(location, group)
@@ -159,7 +169,7 @@ class SignCommand<C : SignCommandSender, T>(
             .commandDescription(Description.of("Unregister a SimpleCloud Sign"))
             .permission(SignCommandPermission.REMOVE.node)
             .handler { context ->
-                CoroutineScope(Dispatchers.IO).launch {
+                commandScope.launch {
                     handleSignOperation(
                         context.sender(),
                         operation = SignOperation.REMOVE
@@ -183,6 +193,30 @@ class SignCommand<C : SignCommandSender, T>(
                 handleRemoveGroupCommand(context.sender(), context.getOrDefault("group", ""))
             }
 
+    private fun createTpCommand(baseCommand: Command.Builder<C>) =
+        baseCommand.literal("tp")
+            .commandDescription(Description.of("Teleport to a registered CloudSign"))
+            .required("world", StringParser.stringParser())
+            .required("x", DoubleParser.doubleParser())
+            .required("y", DoubleParser.doubleParser())
+            .required("z", DoubleParser.doubleParser())
+            .permission(SignCommandPermission.TP.node)
+            .handler { context ->
+                commandScope.launch {
+                    val location = SignLocation(
+                        world = context["world"],
+                        x = context.get("x"),
+                        y = context.get("y"),
+                        z = context.get("z"),
+                    )
+
+                    val success = context.sender().teleport(location)
+                    if (!success) {
+                        sendMessage(context.sender(), SignCommandMessages.TP_FAILED)
+                    }
+                }
+            }
+
     private fun handleListCommand(sender: SignCommandSender, group: String, page: Int = 1) {
         commandScope.launch {
             runCatching {
@@ -243,7 +277,7 @@ class SignCommand<C : SignCommandSender, T>(
 
         val paginatedLocations = groupedLocations.subList(startIndex, endIndex)
         val locationInformation = paginatedLocations.joinToString("\n") { (group, location) ->
-            """<click:run_command:/sign tp ${location.world} ${location.x.toInt()} ${location.y.toInt()} ${location.z.toInt()}><hover:show_text:'Click to teleport'>
+            """<click:run_command:/sign tp ${location.world} ${location.x} ${location.y} ${location.z}><hover:show_text:'Click to teleport'>
 <color:#a8a8a8>└─ <color:#4ade80>Group:</color> <color:#ffffff>${group}</color>
    <color:#a8a8a8>├─</color> <color:#38bdf8>World:</color> <color:#ffffff>${location.world}</color>
    <color:#a8a8a8>├─</color> <color:#38bdf8>X:</color> <color:#ffffff>${location.x}</color>
@@ -308,7 +342,7 @@ $navigationButtons
 
         val paginatedLocations = locations.subList(startIndex, endIndex)
         val locationInformation = paginatedLocations.joinToString("\n") { location ->
-            """<click:run_command:/minecraft:tp ${location.x} ${location.y} ${location.z}><hover:show_text:'Click to teleport'>
+            """<click:run_command:/sign tp ${location.world} ${location.x} ${location.y} ${location.z}><hover:show_text:'Click to teleport'>
 <color:#a8a8a8>└─ <color:#4ade80>World:</color> <color:#ffffff>${location.world}</color>
    <color:#a8a8a8>├─</color> <color:#38bdf8>X:</color> <color:#ffffff>${location.x}</color>
    <color:#a8a8a8>├─</color> <color:#38bdf8>Y:</color> <color:#ffffff>${location.y}</color>
@@ -375,6 +409,7 @@ $navigationButtons
 
     private suspend fun executeAddGroupSign(location: T, group: String): CommandResult {
         return runCatching {
+
             if (signService.getCloudSign(location) != null) {
                 return CommandResult.Error(SignCommandMessages.SIGN_ALREADY_REGISTERED)
             }
@@ -542,31 +577,30 @@ $navigationButtons
             }
         }
 
-
     private fun registeredGroupSuggestions(): BlockingSuggestionProvider<C?> =
         BlockingSuggestionProvider { _, _ ->
             signService.getAllConfigs()
                 .map { config -> Suggestion.suggestion(config.getIdentifier()) }
         }
 
     private fun groupSuggestions(): BlockingSuggestionProvider<C?> =
-        BlockingSuggestionProvider { _, _ ->
-            runBlocking {
-                signService.controllerApi.group().allGroups
-                    .await()
-                    .filterNot { it.type == GroupServerType.PROXY }
-                    .map { Suggestion.suggestion(it.name) }
-            }
-        }
+        BlockingSuggestionProvider { _, _ -> groupSuggestionsSupplier.get() }
 
     private fun persistentServerSuggestions(): BlockingSuggestionProvider<C?> =
-        BlockingSuggestionProvider { _, _ ->
-            runBlocking {
-                signService.controllerApi.persistentServer().getAllPersistentServers()
-                    .await()
-                    .map { Suggestion.suggestion(it.name) }
-            }
-        }
+        BlockingSuggestionProvider { _, _ -> persistentServerSuggestionsSupplier.get() }
+
+    private fun fetchGroupSuggestions(): List<Suggestion> = runBlocking {
+        signService.controllerApi.group().allGroups
+            .await()
+            .filterNot { it.type == GroupServerType.PROXY }
+            .map { Suggestion.suggestion(it.name) }
+    }
+
+    private fun fetchPersistentServerSuggestions(): List<Suggestion> = runBlocking {
+        signService.controllerApi.persistentServer().allPersistentServers
+            .await()
+            .map { Suggestion.suggestion(it.name) }
+    }
 
     fun cleanup() {
         commandScope.cancel()

sign-shared/src/main/kotlin/app/simplecloud/plugin/sign/shared/command/SignStateManager.kt

@@ -1,11 +1,7 @@
 package app.simplecloud.plugin.sign.shared.command
 
-import app.simplecloud.plugin.sign.shared.config.location.SignLocation
-import net.kyori.adventure.text.Component
-
 interface SignStateManager<T> {
 
     suspend fun clearSign(location: T)
-    suspend fun updateSign(location: SignLocation, lines: List<Component>)
 
 }