v0.4.26: keep legacy 5-arg /videoPlace alongside new volume form

Reviewer flagged v0.4.25 broke existing command blocks by inserting
'volume' as a required arg between height and url. Split into two
Brigadier branches: legacy 5-arg defaults volume=50, muted=false; new
6-arg keeps the -1=mute shortcut. Legacy branch uses a single-token url
arg so '50 https://...' parses as the new (int) branch, not as a legacy
url starting with a digit.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

gradle.properties

@@ -5,7 +5,7 @@ org.gradle.configuration-cache=false
 
 # Mod
 mod_id=video_player
-mod_version=0.4.25
+mod_version=0.4.26
 maven_group=com.ejclaw.videoplayer
 archives_base_name=video_player
 

src/main/java/com/ejclaw/videoplayer/command/VideoPlaceCommand.java

@@ -23,15 +23,29 @@ import net.minecraft.server.permissions.Permissions;
 import net.minecraft.world.level.block.Block;
 
 /**
- * SPEC §4.5.1 — {@code /videoPlace <pos> <facing> <width> <height> <volume> <url>}.
+ * SPEC §4.5.1 — {@code /videoPlace} has two accepted forms:
  *
- * <p>{@code volume} is an integer 0..100 (percent) or {@code -1} to start muted. The percent
+ * <ul>
- * form mirrors the GUI slider; {@code -1} is a CLI shortcut so admins don't need a follow-up
+ *   <li><b>Legacy 5-arg</b>: {@code /videoPlace <pos> <facing> <width> <height> <url>}
- * {@code /videoMute} step.
+ *       — preserved for existing command blocks. Volume defaults to 50% and muted=false.
+ *       The {@code url} here is a single-token string so it doesn't swallow the {@code volume}
+ *       slot of the new form.</li>
+ *   <li><b>New 6-arg</b>: {@code /videoPlace <pos> <facing> <width> <height> <volume> <url>}
+ *       — {@code volume} is 0..100 (percent) or {@code -1} to start muted. The percent form
+ *       mirrors the GUI slider; {@code -1} is a CLI shortcut so admins don't need a follow-up
+ *       {@code /videoMute} step. {@code url} is greedy here, so URLs containing spaces (rare
+ *       but possible after URL-decoding) still work.</li>
+ * </ul>
+ *
+ * <p>Brigadier disambiguates the two by the type of the 5th argument: integer → new form,
+ * non-integer token → legacy form.
  */
 public final class VideoPlaceCommand {
     private VideoPlaceCommand() {}
 
+    /** Default volume (percent) applied to the legacy 5-arg form. */
+    private static final int DEFAULT_VOLUME_PCT = 50;
+
     public static void register(CommandDispatcher<CommandSourceStack> dispatcher) {
         dispatcher.register(build("videoPlace"));
     }
@@ -44,13 +58,33 @@ public final class VideoPlaceCommand {
                 .then(Commands.argument("facing", StringArgumentType.word())
                     .then(Commands.argument("width", IntegerArgumentType.integer(1, 32))
                         .then(Commands.argument("height", IntegerArgumentType.integer(1, 32))
+                            // New form: volume (int) + greedy url
                             .then(Commands.argument("volume", IntegerArgumentType.integer(-1, 100))
                                 .then(Commands.argument("url", StringArgumentType.greedyString())
-                                    .executes(VideoPlaceCommand::run)))))));
+                                    .executes(ctx -> runNew(ctx))))
+                            // Legacy form: single-token url, no volume slot. Single-token string
+                            // is intentional so "<int> https://..." cannot be parsed as a legacy
+                            // url that happens to start with a number — Brigadier first tries the
+                            // new branch and only falls through here if "volume" isn't an int.
+                            .then(Commands.argument("url", StringArgumentType.string())
+                                .executes(ctx -> runLegacy(ctx)))))));
     }
 
-    private static int run(com.mojang.brigadier.context.CommandContext<CommandSourceStack> ctx)
+    private static int runNew(com.mojang.brigadier.context.CommandContext<CommandSourceStack> ctx)
             throws CommandSyntaxException {
+        int volumeArg = IntegerArgumentType.getInteger(ctx, "volume");
+        String url = StringArgumentType.getString(ctx, "url");
+        return runWithValues(ctx, volumeArg, url);
+    }
+
+    private static int runLegacy(com.mojang.brigadier.context.CommandContext<CommandSourceStack> ctx)
+            throws CommandSyntaxException {
+        String url = StringArgumentType.getString(ctx, "url");
+        return runWithValues(ctx, DEFAULT_VOLUME_PCT, url);
+    }
+
+    private static int runWithValues(com.mojang.brigadier.context.CommandContext<CommandSourceStack> ctx,
+                                     int volumeArg, String rawUrl) throws CommandSyntaxException {
         CommandSourceStack src = ctx.getSource();
         ServerLevel level = src.getLevel();
         BlockPos pos = BlockPosArgument.getLoadedBlockPos(ctx, "pos");
@@ -61,13 +95,12 @@ public final class VideoPlaceCommand {
         }
         int width  = IntegerArgumentType.getInteger(ctx, "width");
         int height = IntegerArgumentType.getInteger(ctx, "height");
-        int volumeArg = IntegerArgumentType.getInteger(ctx, "volume");
         // -1 is the CLI mute shortcut; the BE keeps the underlying volume so an admin can
         // /videoMute false later without re-typing a level. Anything 0..100 sets %-volume and
         // clears mute.
         boolean placeMuted = volumeArg < 0;
         float placeVolume = placeMuted ? 0.5F : (volumeArg / 100.0F);
-        String raw = StringArgumentType.getString(ctx, "url").trim();
+        String raw = rawUrl == null ? "" : rawUrl.trim();
         // Accept either an http(s) URL or a /videoCache add <name> entry: resolveUrlOrName()
         // returns the canonical URL in both cases, or null when a non-URL string didn't match
         // any named entry.