Skip to content

Add D&D-style dice notation support to /roll - #317

Merged
dmccoystephenson merged 4 commits into
mainfrom
copilot/add-dice-notation-support
Jul 31, 2026
Merged

Add D&D-style dice notation support to /roll#317
dmccoystephenson merged 4 commits into
mainfrom
copilot/add-dice-notation-support

Conversation

Copilot AI commented May 3, 2026

Copy link
Copy Markdown
Contributor
  • Extend RollCommand.java to parse full dice notation (NdM, dM, NdM+K, NdM-K, plain number legacy form)
  • Use java.util.Random instead of Math.random() for better randomness
  • Add upper-bound guards (MAX_DICE=100, MAX_DIE_SIZE=10000) to prevent abuse
  • Wrap all Integer.parseInt calls in try/catch to return friendly error on oversized inputs (e.g. 999999999999d6)
  • Add MAX_MODIFIER=10000 bound to prevent integer overflow in total calculation
  • Broadcast detailed message showing notation, individual rolls, modifier, and total
  • Fix: always send roll result directly to the executing player via player.sendMessage() so the roller never sees a blank response; nearby players receive the message via sendRPMessageToPlayersWithinDistanceExcludingTarget
  • Update help text in HelpCommand.java
  • Update COMMANDS.md documentation
  • Update USER_GUIDE.md documentation

Copilot AI changed the title [WIP] Add D&D-style modifier and multi-roll support to /roll Add D&D-style dice notation support to /roll May 3, 2026
Copilot AI requested a review from dmccoystephenson May 3, 2026 18:47
@dmccoystephenson
dmccoystephenson requested a review from Copilot May 3, 2026 18:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Extends the /roll (and /dice) command to accept standard D&D-style dice notation (e.g., 2d6, d20+5) instead of only a single max integer, improving TTRPG usability while preserving legacy /roll 20 behavior.

Changes:

  • Added dice-notation parsing (via a compiled regex) and multi-die rolling with modifiers.
  • Updated the broadcast format to show individual rolls (when applicable) plus modifiers and totals.
  • Updated help text and user documentation to describe the new notation and examples.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/main/java/dansplugins/rpsystem/commands/roll/RollCommand.java Implements dice-notation parsing, bounded roll execution, and updated broadcast message formatting.
src/main/java/dansplugins/rpsystem/commands/help/HelpCommand.java Updates /roll help line to document dice notation usage.
USER_GUIDE.md Documents supported dice formats and examples for players.
COMMANDS.md Updates command reference with dice notation definition and examples.

Comment on lines +89 to +99
int sum = 0;
for (int r : rolls) {
sum += r;
}
int total = sum + modifier;

String rollsDisplay = buildRollsDisplay(rolls);
String modDisplay = modifier != 0 ? (modifier > 0 ? " +" + modifier : " " + modifier) : "";
String message = medievalRoleplayEngine.colorChecker.getNeutralAlertColor() + "" + ChatColor.ITALIC
+ player.getName() + " rolled " + notation + ": "
+ rollsDisplay + modDisplay + " = " + total;
Comment on lines +57 to +72
if (m.group(1) != null) {
// NdM form
numDice = Integer.parseInt(m.group(1));
dieSize = Integer.parseInt(m.group(2));
} else if (m.group(3) != null) {
// dM form (implicit 1 die)
numDice = 1;
dieSize = Integer.parseInt(m.group(3));
} else {
// plain number — treat as die size (legacy behaviour)
numDice = 1;
dieSize = Integer.parseInt(m.group(4));
}

if (m.group(5) != null) {
modifier = Integer.parseInt(m.group(5));
@dmccoystephenson

Copy link
Copy Markdown
Member

@copilot address comments

…ent overflow

Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Roleplay-Engine/sessions/a190ae5f-b9f4-4131-9d98-cffb87ff4fcc

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Copilot AI commented May 3, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address comments

Both issues addressed in ace163e:

  1. NumberFormatException on oversized inputs — all Integer.parseInt calls are now wrapped in a single try/catch that returns a user-friendly error message (e.g. 999999999999d6 now shows "contains a value that is too large").

  2. Modifier overflow — added MAX_MODIFIER = 10000 constant; after parsing, the modifier is checked against [-10000, 10000] and rejected with a clear message if out of range, preventing any int overflow in the total calculation.

@dmccoystephenson

Copy link
Copy Markdown
Member

@copilot I created a snapshot release using this branch, causing the CI to build a JAR. This is from a user testing that JAR:

Just a heads up all it does doesnt issue a command just goes blank. Also file says 14 not the snapshot?

Please resolve

Copilot AI commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

@copilot I created a snapshot release using this branch, causing the CI to build a JAR. This is from a user testing that JAR:

Fixed in c0fa01c. The root cause is that the roll message was only delivered through the proximity-based local-chat system (sendRPMessageToPlayersWithinDistance). If the player had previously run /local hide, the hidden-chat filter would suppress the message for them, resulting in a completely blank response.

The fix splits delivery into two paths:

  1. player.sendMessage(message) — sends the result directly to the roller, bypassing all proximity/filter logic, so they always see their own result.
  2. sendRPMessageToPlayersWithinDistanceExcludingTarget(...) — broadcasts to nearby players (excluding the roller to avoid a duplicate).

Regarding "file says 14": the JAR was correctly built from this branch. The version 1.14.0 comes from the Maven project version in pom.xml and is substituted into plugin.yml at build time — it's not a snapshot version string, just the current release number.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • hub.spigotmc.org
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.14/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.14/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.14 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.14/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/Medieval-Roleplay-Engine/Medieval-Roleplay-Engine org.codehaus.plexus.classworlds.launcher.Launcher compile -q (dns block)
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.14/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.14/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.14 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.14/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/Medieval-Roleplay-Engine/Medieval-Roleplay-Engine org.codehaus.plexus.classworlds.launcher.Launcher -f pom.xml -B -V -e -Dfindbugs.skip -Dcheckstyle.skip -Dpmd.skip=true -Dspotbugs.skip -Denforcer.skip -Dmaven.javadoc.skip (dns block)
  • jitpack.io
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.14/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.14/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.14 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.14/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/Medieval-Roleplay-Engine/Medieval-Roleplay-Engine org.codehaus.plexus.classworlds.launcher.Launcher compile -q (dns block)
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.14/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.14/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.14 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.14/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/Medieval-Roleplay-Engine/Medieval-Roleplay-Engine org.codehaus.plexus.classworlds.launcher.Launcher -f pom.xml -B -V -e -Dfindbugs.skip -Dcheckstyle.skip -Dpmd.skip=true -Dspotbugs.skip -Denforcer.skip -Dmaven.javadoc.skip (dns block)
  • repo.extendedclip.com
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.14/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.14/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.14 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.14/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/Medieval-Roleplay-Engine/Medieval-Roleplay-Engine org.codehaus.plexus.classworlds.launcher.Launcher compile -q (dns block)
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.14/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.14/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.14 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.14/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/Medieval-Roleplay-Engine/Medieval-Roleplay-Engine org.codehaus.plexus.classworlds.launcher.Launcher -f pom.xml -B -V -e -Dfindbugs.skip -Dcheckstyle.skip -Dpmd.skip=true -Dspotbugs.skip -Denforcer.skip -Dmaven.javadoc.skip (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/main/java/dansplugins/rpsystem/commands/roll/RollCommand.java:75

  • The parser currently accepts legacy numeric input with a modifier (e.g. 20+5) because the regex allows an optional [+-]\d+ after the plain-number branch. That format isn’t standard dice notation and isn’t documented in COMMANDS.md/USER_GUIDE.md, so it’s likely to confuse users. Consider rejecting modifiers when the legacy numeric form is used (and require d20+5 instead), or document it explicitly.
                if (m.group(5) != null) {
                    modifier = Integer.parseInt(m.group(5));
                }

@dmccoystephenson
dmccoystephenson merged commit 0ddc065 into main Jul 31, 2026
2 checks passed
@dmccoystephenson
dmccoystephenson deleted the copilot/add-dice-notation-support branch July 31, 2026 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add D&D-style modifier and multi-roll support to /roll

3 participants