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.24
+mod_version=0.4.26
 maven_group=com.ejclaw.videoplayer
 archives_base_name=video_player
 

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

@@ -22,10 +22,30 @@ import net.minecraft.server.level.ServerPlayer;
 import net.minecraft.server.permissions.Permissions;
 import net.minecraft.world.level.block.Block;
 
-/** SPEC §4.5.1 — {@code /videoPlace <pos> <facing> <width> <height> <url>} */
+/**
+ * SPEC §4.5.1 — {@code /videoPlace} has two accepted forms:
+ *
+ * <ul>
+ *   <li><b>Legacy 5-arg</b>: {@code /videoPlace <pos> <facing> <width> <height> <url>}
+ *       — 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"));
     }
@@ -38,12 +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");
@@ -54,7 +95,12 @@ public final class VideoPlaceCommand {
         }
         int width  = IntegerArgumentType.getInteger(ctx, "width");
         int height = IntegerArgumentType.getInteger(ctx, "height");
-        String raw = StringArgumentType.getString(ctx, "url").trim();
+        // -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 = 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.
@@ -75,6 +121,8 @@ public final class VideoPlaceCommand {
         be.setWidth(width);
         be.setHeight(height);
         be.setUrl(url);
+        be.setVolume(placeVolume);
+        be.setMuted(placeMuted);
 
         CompoundTag nbt = be.toNbt();
         for (ServerPlayer p : PlayerLookup.tracking(level, pos)) {