feat: implement /random command by Pa-dej · Pull Request #1951 · Pumpkin-MC/Pumpkin

Pa-dej · 2026-04-01T04:11:42Z

Description

Implements the vanilla /random command.

Supported syntax:

/random value <range> - result visible only to sender
/random roll <range> - result broadcast to all players
/random value <range> <sequence> - sample from named sequence
/random roll <range> <sequence> - same with broadcast
/random reset <sequence> [seed] [includeWorldSeed] [includeSequenceId]
/random reset * - reset all sequences

Implementation notes:

Sequence names validated as namespaced IDs (lowercase only),
error position matches vanilla behavior.
Tab completion suggests existing sequence names
Sequences persisted in level.dat via RandomSequence in LevelData
(same pattern as GameRules - vanilla uses separate random_sequences.dat,
can be migrated in follow-up PR once world/data/ infrastructure exists)
xoroshiro128 RNG for deterministic sequence generation.

Testing

Valid/invalid range inputs tested manually in-game
Sequence reset + deterministic replay verified.
cargo fmt, cargo clippy, cargo test - all pass.

Q2297045667 · 2026-04-03T02:30:11Z

We now have a new command system.
Please refer to this PR to reimplement the command.
#1918

- Generate random integers with value (private) and roll (broadcast) modes - Named sequences with deterministic xoroshiro128 RNG - Sequence reset with seed, includeWorldSeed, includeSequenceId options - Strict namespaced ID validation matching vanilla error positions - Tab completion for existing sequence names - Sequence state persisted in level.dat via RandomSequence in LevelData

Pa-dej · 2026-04-05T16:28:54Z

Ported to the new command system as requested.

Snowiiii · 2026-04-06T08:20:23Z

Hey, Thank you for your PR

Please use Arg consumers which are already implemented, you should not implement them in the Command File
Don't use Literals use translations

I think all Arg consumers you need are already implemented

Pa-dej · 2026-04-08T13:23:31Z

Fixed both review points:

Replaced custom arg consumers with existing ones
(ResourceLocationArgumentConsumer, BoundedNumArgumentConsumer, BoolArgConsumer)
All messages now use vanilla translation keys from en_us.json

SomeYellowGuy · 2026-04-08T16:03:15Z

Doesn't this go back to the old command dispatcher?

Laptop59

Overall, this PR still needs some work to be done before it has the potential to be merged.

Laptop59 · 2026-04-09T13:02:23Z

+struct RangeArgumentType;
+
+impl ArgumentType for RangeArgumentType {
+    type Item = (i32, i32);
+
+    fn parse(&self, reader: &mut StringReader) -> Result<Self::Item, CommandSyntaxError> {
+        let start = reader.cursor();
+
+        let min = if reader.peek() == Some('.') {
+            None
+        } else {
+            Some(reader.read_int()?)
+        };
+
+        let has_separator = reader.peek() == Some('.') && reader.peek_with_offset(1) == Some('.');
+
+        let (min, max) = if has_separator {
+            reader.expect('.')?;
+            reader.expect('.')?;
+
+            let max = match reader.peek() {
+                None => None,
+                Some(c) if c.is_whitespace() => None,
+                _ => Some(reader.read_int()?),
+            };
+
+            (min.unwrap_or(i32::MIN), max.unwrap_or(i32::MAX))
+        } else {
+            let Some(value) = min else {
+                reader.set_cursor(start);
+                return Err(error_types::READER_EXPECTED_INT.create(reader));
+            };
+
+            (value, value)
+        };
+
+        let range_size = i64::from(max) - i64::from(min) + 1;
+        if range_size < 2 {
+            return Err(
+                error_types::DISPATCHER_PARSE_EXCEPTION.create_without_context(
+                    TextComponent::translate(
+                        translation::COMMANDS_RANDOM_ERROR_RANGE_TOO_SMALL,
+                        [],
+                    ),
+                ),
+            );
+        }
+        if range_size > 2_147_483_646 {
+            return Err(
+                error_types::DISPATCHER_PARSE_EXCEPTION.create_without_context(
+                    TextComponent::translate(
+                        translation::COMMANDS_RANDOM_ERROR_RANGE_TOO_LARGE,
+                        [],
+                    ),
+                ),
+            );
+        }
+
+        Ok((min, max))
+    }
+
+    fn client_side_parser(&'_ self) -> JavaClientArgumentType<'_> {
+        JavaClientArgumentType::String(StringProtoArgBehavior::SingleWord)
+    }
+}


You should remove this part of the code as the RangeArgumentType has been implemented in the argument_types module, which can also be used by other commands.

Removed, switched to shared IntRangeArgumentType. Kept the /random-specific size checks (>= 2, <= 2_147_483_646) in the executor since those are command constraints, not general range constraints.

Laptop59 · 2026-04-09T13:04:48Z

@@ -148,13 +209,13 @@ impl InclusiveRange {

        let range_size = i64::from(max) - i64::from(min) + 1;
        if range_size < 2 {
-            return Err(CommandFailed(TextComponent::translate(
+            return Err(CommandError::CommandFailed(TextComponent::translate(
                translation::COMMANDS_RANDOM_ERROR_RANGE_TOO_SMALL,
                [],
            )));
        }
        if range_size > 2_147_483_646 {
-            return Err(CommandFailed(TextComponent::translate(
+            return Err(CommandError::CommandFailed(TextComponent::translate(
                translation::COMMANDS_RANDOM_ERROR_RANGE_TOO_LARGE,
                [],
            )));
@@ -164,9 +225,10 @@ impl InclusiveRange {
    }
 }


This should be removed as it's an artifact of the previous commits in this PR, and that we already have the argument type required.

Laptop59 · 2026-04-09T13:07:36Z

+#[cfg(test)]
+fn parse_i32_range_bound(raw: &str) -> Result<i32, CommandError> {
+    raw.parse::<i32>().map_err(|_| {
+        CommandError::CommandFailed(TextComponent::translate(
+            translation::PARSING_INT_INVALID,
+            [TextComponent::text(raw.to_string())],
+        ))
+    })
+}


Uses the older CommandError, was this supposed to be removed?

Removed, was leftover code.

Laptop59 · 2026-04-09T13:09:51Z

+fn get_optional_argument<'a, T: 'static>(
+    context: &'a CommandContext,
+    name: &str,
+) -> Result<Option<&'a T>, CommandSyntaxError> {
+    if context.arguments.contains_key(name) {
+        context.get_argument(name).map(Some)
+    } else {
+        Ok(None)
+    }
+}


A utility method for optionally parsing an argument is 'very hacky'. The executor should know what arguments to parse and what arguments not to parse after command parsing from its stored values

Removed. ResetExecutor now uses explicit ResetMode variants wired from the command tree, so each path knows exactly which arguments to parse.

Laptop59 · 2026-04-09T13:11:57Z

+fn parse_sequence_arg<'a>(context: &'a CommandContext) -> Result<&'a str, CommandSyntaxError> {
+    let sequence = context.get_argument::<String>(ARG_SEQUENCE)?;
+    let raw_arg = RawArg {
+        value: sequence,
+        start: 0,
+        end: sequence.len(),
+        input: sequence,
+    };
+
+    validate_sequence_name(raw_arg)
+}


The sequence argument is an identifier (formerly resource location) containing a namespace and path, not a string. We don't have parsing for identifiers yet. A way to parse them will be required before this argument can be implemented properly.

Added ResourceLocationArgumentType to argument_types/core and used it for ARG_SEQUENCE. Client-side parser is now ResourceLocation. validate_sequence_name() stays in the executor for server-side validation.

Laptop59 · 2026-04-09T13:14:10Z

+fn level_two_requirement(
+    source: &CommandSource,
+) -> std::pin::Pin<Box<dyn std::future::Future<Output = bool> + Send + '_>> {
+    Box::pin(async move { source.output.has_permission_lvl(PermissionLvl::Two) })
+}


Instead of checking if the sender has permission level 2, we should create a permission and check if it has that, so that the permission level of it can be changed as desired (by server owners, for example) without conflicts.

Replaced with explicit permissions minecraft:command.random.sequence and minecraft:command.random.reset, both registered with Op(Two) and checked via .requires(...)

Laptop59 · 2026-04-09T13:15:36Z

+    #[test]
+    fn parse_valid_closed_range() {
+        let range = InclusiveRange::parse("1..10").expect("range should parse");
+        assert_eq!(range.min, 1);
+        assert_eq!(range.max, 10);
+    }
+
+    #[test]
+    fn parse_valid_open_lower_bound_range() {
+        let range = InclusiveRange::parse("..-2147483647").expect("range should parse");
+        assert_eq!(range.min, i32::MIN);
+        assert_eq!(range.max, -2_147_483_647);
+    }
+
+    #[test]
+    fn parse_valid_open_upper_bound_range() {
+        let range = InclusiveRange::parse("2147483646..").expect("range should parse");
+        assert_eq!(range.min, 2_147_483_646);
+        assert_eq!(range.max, i32::MAX);
+    }
+
+    #[test]
+    fn reject_single_value_range() {
+        assert!(matches!(
+            InclusiveRange::parse("5"),
+            Err(CommandError::CommandFailed(_))
+        ));
+    }
+
+    #[test]
+    fn reject_reversed_range() {
+        assert!(matches!(
+            InclusiveRange::parse("10..1"),
+            Err(CommandError::CommandFailed(_))
+        ));
+    }
+
+    #[test]
+    fn reject_too_large_range_size() {
+        assert!(matches!(
+            InclusiveRange::parse("-2147483648..2147483647"),
+            Err(CommandError::CommandFailed(_))
+        ));
+    }


The range module in argument_types has tests for parsing different types of ranges, so we don't need this part of the code.

Removed, range tests are covered in argument_types/range.

Laptop59 · 2026-04-09T13:16:49Z

+            let mut path_raw_args = raw_args.clone();
+            match Self::try_find_suggestions_on_path(
+                src,
+                server,
+                &path,
+                tree,
+                &mut path_raw_args,
+                cmd,
+            )
+            .await


These changes look like an artifact of previous commits of this PR with the older dispatcher. raw_args was made mutable during those changes and probably should be reverted to be immutable again.

Reverted dispatcher.rs to master baseline.

Snowiiii · 2026-04-21T14:40:45Z

@Pa-dej There should be now an Identifer in master already, also please fix the conflict

Q2297045667 added the command Anything command or chat-related label Apr 3, 2026

Pa-dej added 2 commits April 5, 2026 20:03

fix: address clippy warning

4bec4a8

Pa-dej force-pushed the fix/random-sequence-bugs branch from ff02045 to 4bec4a8 Compare April 5, 2026 15:05

feat(command): reimplement /random with new command system

0ae4d58

fix-command-address-random-review-feedback

fce9ae6

feat(command): reimplement /random with new command system

fb8f07a

Laptop59 reviewed Apr 9, 2026

View reviewed changes

fix(command): address review comments on /random

43b7758

Uh oh!

Conversation

Pa-dej commented Apr 1, 2026

Description

Testing

Uh oh!

Q2297045667 commented Apr 3, 2026

Uh oh!

Pa-dej commented Apr 5, 2026

Uh oh!

Snowiiii commented Apr 6, 2026

Uh oh!

Pa-dej commented Apr 8, 2026

Uh oh!

SomeYellowGuy commented Apr 8, 2026

Uh oh!

Laptop59 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Snowiiii commented Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants