Merge pull request #137 from SpigotBasics/dev

Improved error messages

common/src/main/kotlin/com/github/spigotbasics/common/Either.kt

@@ -105,14 +105,13 @@ sealed class Either<out L, out R> {
      */
     fun <NewL, NewR> mapBoth(
         transformLeft: (L) -> NewL,
-        transformRight: (R) -> NewR
+        transformRight: (R) -> NewR,
     ): Either<NewL, NewR> =
         when (this) {
             is Left -> Left(transformLeft(value))
             is Right -> Right(transformRight(value))
         }
 
-
     companion object {
         /**
          * Constructs an Either object from two nullable values. If both are null, or both are non-null, an exception is thrown.
@@ -134,4 +133,4 @@ sealed class Either<out L, out R> {
             }
         }
     }
-}
+}

core/src/main/kotlin/com/github/spigotbasics/core/command/parsed/ArgumentPath.kt

@@ -29,9 +29,9 @@ class ArgumentPath<T : CommandContext>(
     fun matches(
         sender: CommandSender,
         args: List<String>,
-    ): Either<PathMatchResult, Pair<Int, List<Message>>> {
+    ): Either<PathMatchResult, Pair<Double, List<Message>>> {
         val greedyArg = arguments.indexOfFirst { it.second.greedy }
-        var firstErrorIndex = -1
+        var firstErrorIndex = -1.0
 
         // Each provided arg must be parseable by its corresponding CommandArgument
         val errors = mutableListOf<Message>()
@@ -48,6 +48,10 @@ class ArgumentPath<T : CommandContext>(
 
             if (currentArgResult is Either.Right) {
                 errors.add(Basics.messages.notEnoughArgumentsGivenForArgument(argument.name))
+                val missingArguments = currentArgResult.value
+                // "First error" is the first missing argument, adjusted by 0.5 so that arguments with matching paths are preferred
+                val returnValue = (index + 1) - missingArguments + 0.5
+                firstErrorIndex = if (firstErrorIndex == -1.0) returnValue else min(firstErrorIndex, returnValue) // TODO - is this correct?
                 return@forEach
             }
 
@@ -57,7 +61,7 @@ class ArgumentPath<T : CommandContext>(
 
             if (parsed == null) {
                 val error = argument.errorMessage(sender, currentArg)
-                firstErrorIndex = if (firstErrorIndex == -1) index else min(firstErrorIndex, index)
+                firstErrorIndex = if (firstErrorIndex == -1.0) index.toDouble() else min(firstErrorIndex, index.toDouble())
                 errors.add(error)
             }
         }
@@ -67,7 +71,7 @@ class ArgumentPath<T : CommandContext>(
         if (greedyArg == -1) {
             if (args.size > arguments.sumOf { it.second.length }) {
                 return Either.Right(
-                    arguments.size to listOf(Basics.messages.tooManyArguments),
+                    arguments.size.toDouble() to listOf(Basics.messages.tooManyArguments),
                 )
             }
         }
@@ -82,7 +86,7 @@ class ArgumentPath<T : CommandContext>(
         givenArgs: List<String>,
         commandArguments: List<CommandArgument<*>>,
         greedyPosition: Int,
-    ): Either<String, Nothing?> {
+    ): Either<String, Int> {
         val result = accumulateArguments0(argIndex, givenArgs, commandArguments, greedyPosition)
         logger.debug(400, "Accumulated arguments @ $argIndex  ----- $result")
         return result
@@ -93,14 +97,15 @@ class ArgumentPath<T : CommandContext>(
         givenArgs: List<String>,
         commandArguments: List<CommandArgument<*>>,
         greedyPosition: Int,
-    ): Either<String, Nothing?> {
+    ): Either<String, Int> {
         // Count length of combined arguments before this one
         var myStartIndex = commandArguments.subList(0, argIndex).sumOf { it.length }
-        val myLength = commandArguments[argIndex].length
-        val myEndIndex = myStartIndex + myLength
+        val mySupposedLength = commandArguments[argIndex].length
+        val myEndIndex = myStartIndex + mySupposedLength
+        var missingArguments = mySupposedLength - (givenArgs.size - myStartIndex)
 
         if (myEndIndex > givenArgs.size) {
-            return Either.Right(null)
+            return Either.Right(missingArguments) // TODO: Is this correct?
         }
 
         val expectedLengthWithoutGreedy = commandArguments.filter { !it.greedy }.sumOf { it.length }
@@ -126,8 +131,10 @@ class ArgumentPath<T : CommandContext>(
         val lengthAfterMe = commandArguments.subList(argIndex + 1, commandArguments.size).sumOf { it.length }
         myStartIndex = givenArgs.size - lengthAfterMe - 1
 
+        missingArguments = mySupposedLength - (givenArgs.size - myStartIndex)
+
         if (myStartIndex + 1 > givenArgs.size) {
-            return Either.Right(null)
+            return Either.Right(missingArguments)
         }
 
         return Either.Left(givenArgs.subList(myStartIndex, givenArgs.size).joinToString(" "))

core/src/main/kotlin/com/github/spigotbasics/core/command/parsed/ParsedCommandExecutor.kt

@@ -42,13 +42,14 @@ class ParsedCommandExecutor<T : CommandContext>(
         }
 
         // Sort paths by the number of arguments they expect
-        val sortedPaths = paths.sortedBy { -it.arguments.size }
+        // val sortedPaths = paths.sortedBy { -it.arguments.size }
+        val sortedPaths = paths.sortedBy { -it.arguments.sumOf { arg -> arg.second.length } }
 
         var shortestPathFailure: ParseResult.Failure? = null
         var bestMatchResult: PathMatchResult? = null
         var errors: List<Message>? = null
         var missingPermission: Permission? = null
-        var lastErrorIndex = -1
+        var lastErrorIndex = -1.0
 
         sortedPaths.forEach { path ->
 
@@ -61,7 +62,7 @@ class ParsedCommandExecutor<T : CommandContext>(
                 // TODO: Maybe collect all error messages? Right now, which error message is shown depends on the order of the paths
                 //  That means more specific ones should be registered first
                 val (firstErrorIndex, newErrors) = matchResult.value
-                if (lastErrorIndex == -1 ||
+                if (lastErrorIndex == -1.0 ||
                     // errors!!.size > newErrors.size // Old version - use the one with the least errors
                     lastErrorIndex < firstErrorIndex
                 ) {

core/src/main/kotlin/com/github/spigotbasics/core/command/parsed/arguments/TripleContextCoordinatesArg.kt

@@ -4,14 +4,27 @@ import com.github.spigotbasics.core.model.TripleContextCoordinates
 import org.bukkit.command.CommandSender
 
 class TripleContextCoordinatesArg(name: String) : CommandArgument<TripleContextCoordinates>(name) {
-    override fun parse(sender: CommandSender, value: String): TripleContextCoordinates? {
+    override fun parse(
+        sender: CommandSender,
+        value: String,
+    ): TripleContextCoordinates? {
         return try {
             TripleContextCoordinates.parse(value)
         } catch (_: Exception) {
             null
         }
     }
 
+    override fun tabComplete(
+        sender: CommandSender,
+        typing: String,
+    ): List<String> {
+        // println(typing)
+        // TODO: Add selectors and ~ ~~ for tabcomplete if has permission
+        // TODO: That requires passing the concat-ed string to the tabComplete method
+        return super.tabComplete(sender, typing)
+    }
+
     override val greedy = true
     override val length = 3
-}
+}

core/src/main/kotlin/com/github/spigotbasics/core/model/TripleContextCoordinates.kt

@@ -15,14 +15,16 @@ data class TripleContextCoordinates(
     val pitch: Float = 0f,
     val pitchRelative: Relativity = Relativity.RELATIVE_TO_FIRST,
 ) {
-
     enum class Relativity {
         ABSOLUTE,
         RELATIVE_TO_FIRST,
         RELATIVE_TO_SECOND,
     }
 
-    fun toLocation(first: Location, second: Location = first): Location {
+    fun toLocation(
+        first: Location,
+        second: Location = first,
+    ): Location {
         val nX = applyRelativity(xRelative, x, first.x, second.x)
         val nY = applyRelativity(yRelative, y, first.y, second.y)
         val nZ = applyRelativity(zRelative, z, first.z, second.z)
@@ -38,19 +40,19 @@ data class TripleContextCoordinates(
             val (y, yRelative) = parseSingle(parts[1])
             val (z, zRelative) = parseSingle(parts[2])
 
-            val (yaw, yawRelative) = if (parts.size > 3) {
-                parseSingle(parts[3])
-            } else {
-                Pair(0.0, Relativity.RELATIVE_TO_FIRST)
-            }
-
-            val (pitch, pitchRelative) = if (parts.size > 4) {
-                parseSingle(parts[4])
-            } else {
-                Pair(0.0, Relativity.RELATIVE_TO_FIRST)
-            }
-
+            val (yaw, yawRelative) =
+                if (parts.size > 3) {
+                    parseSingle(parts[3])
+                } else {
+                    Pair(0.0, Relativity.RELATIVE_TO_FIRST)
+                }
 
+            val (pitch, pitchRelative) =
+                if (parts.size > 4) {
+                    parseSingle(parts[4])
+                } else {
+                    Pair(0.0, Relativity.RELATIVE_TO_FIRST)
+                }
 
             return TripleContextCoordinates(
                 x,
@@ -66,15 +68,25 @@ data class TripleContextCoordinates(
             )
         }
 
-        fun applyRelativity(relativity: Relativity, value: Double, first: Double, second: Double): Double {
+        fun applyRelativity(
+            relativity: Relativity,
+            value: Double,
+            first: Double,
+            second: Double,
+        ): Double {
             return when (relativity) {
                 Relativity.ABSOLUTE -> value
                 Relativity.RELATIVE_TO_FIRST -> first + value
                 Relativity.RELATIVE_TO_SECOND -> second + value
             }
         }
 
-        fun applyRelativity(relativity: Relativity, value: Float, first: Float, second: Float): Float {
+        fun applyRelativity(
+            relativity: Relativity,
+            value: Float,
+            first: Float,
+            second: Float,
+        ): Float {
             return when (relativity) {
                 Relativity.ABSOLUTE -> value
                 Relativity.RELATIVE_TO_FIRST -> first + value
@@ -92,5 +104,4 @@ data class TripleContextCoordinates(
             }
         }
     }
-
 }

modules/basics-tp/src/main/kotlin/com/github/spigotbasics/modules/basicstp/BasicsTpModule.kt

@@ -4,7 +4,6 @@ import com.github.spigotbasics.common.Either
 import com.github.spigotbasics.core.command.parsed.arguments.SelectorMultiEntityArg
 import com.github.spigotbasics.core.command.parsed.arguments.SelectorSingleEntityArg
 import com.github.spigotbasics.core.command.parsed.arguments.TripleContextCoordinatesArg
-import com.github.spigotbasics.core.command.parsed.arguments.XYZCoordsArg
 import com.github.spigotbasics.core.messages.Message
 import com.github.spigotbasics.core.messages.tags.providers.LocationTag
 import com.github.spigotbasics.core.module.AbstractBasicsModule
@@ -56,20 +55,20 @@ class BasicsTpModule(context: ModuleInstantiationContext) : AbstractBasicsModule
                 }
             }
 
-            // @entities x y z
+            // @entities @entity
             path {
                 arguments {
                     named("targets", SelectorMultiEntityArg("Entities to teleport"))
-                    named("destination", TripleContextCoordinatesArg("Destination Coordinates"))
+                    named("destination", SelectorSingleEntityArg("Destination Entity"))
                 }
                 permissions(permissionOthers)
             }
 
-            // @entities @entity
+            // @entities x y z
             path {
                 arguments {
                     named("targets", SelectorMultiEntityArg("Entities to teleport"))
-                    named("destination", SelectorSingleEntityArg("Destination Entity"))
+                    named("destination", TripleContextCoordinatesArg("Destination Coordinates"))
                 }
                 permissions(permissionOthers)
             }