fix command validation and add new type of interaction#73
Conversation
📝 WalkthroughWalkthroughAdds player-executed command and player-chat interaction actions, including registration, aliases, help text, tab completion, persistence, runtime execution, and duplicate interaction filtering. ChangesInteraction action support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Sender
participant NPCCommand
participant NPC
participant InteractionHandler
participant Player
Sender->>NPCCommand: Add run_command_player interaction
NPCCommand->>NPC: Store and save interaction action
NPCCommand-->>Sender: Report action count
InteractionHandler->>Player: Execute substituted command
Player-->>InteractionHandler: Command result or failure
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
spaceNPC/src/main/java/me/tofaa/entitylib/npc/command/NPCCommand.java (1)
1609-1669: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the interaction addition logic to reduce duplication.
The logic for handling
run_command_playerandplayer_chatduplicates the action creation, saving, and success message formatting used byrun_commandandmessage. You can extract this into a single path by mapping theactionTypeStrto the appropriate constant and value string, then executing the boilerplate once.♻️ Example refactor
String finalActionType; String finalValue; String actionFriendlyName; if (actionTypeStr.equals("run_command") || actionTypeStr.equals("cmd") || actionTypeStr.equals("command")) { finalActionType = InteractionAction.RUN_COMMAND; finalValue = valueWithoutSlash; actionFriendlyName = "command"; } else if (actionTypeStr.equals("run_command_player") || actionTypeStr.equals("player_cmd") || actionTypeStr.equals("player_command")) { finalActionType = InteractionAction.RUN_COMMAND_PLAYER; finalValue = valueWithoutSlash; actionFriendlyName = "player command"; } else if (actionTypeStr.equals("player_chat") || actionTypeStr.equals("chat")) { finalActionType = InteractionAction.PLAYER_CHAT; finalValue = value; actionFriendlyName = "player chat"; } else if (actionTypeStr.equals("message") || actionTypeStr.equals("msg") || actionTypeStr.equals("say")) { finalActionType = InteractionAction.MESSAGE; finalValue = value; actionFriendlyName = "message"; } else { sender.sendMessage(mm("<red>Unknown action type: <yellow>" + actionTypeStr)); sender.sendMessage(mm("<gray>Actions: <white>run_command, run_command_player, player_chat, message")); return true; } InteractionAction action = new InteractionAction(interactionType, finalActionType, finalValue); opts.addInteraction(interactionType, action); storage.saveNPC(npc); List<InteractionAction> actions = opts.getInteractions(interactionType); sender.sendMessage(mm("<green>Added " + actionFriendlyName + " to <yellow>" + interactionType.name().toLowerCase() + "<green>: <white>" + (finalValue.equals(valueWithoutSlash) && !finalValue.isEmpty() && value.startsWith("/") ? "/" : "") + finalValue)); sender.sendMessage(mm("<gray>Total actions for " + interactionType.name().toLowerCase() + ": <white>" + actions.size()));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spaceNPC/src/main/java/me/tofaa/entitylib/npc/command/NPCCommand.java` around lines 1609 - 1669, Consolidate the duplicated interaction handling in the action-type branch around run_command, run_command_player, message, and player_chat by mapping each alias to its InteractionAction constant, value, and display name. Then perform InteractionAction creation, opts.addInteraction, storage.saveNPC, action-count lookup, and success messages through one shared path, preserving slash formatting for command values and existing behavior for each action type.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@spaceNPC/src/main/java/me/tofaa/entitylib/npc/command/NPCCommand.java`:
- Around line 1609-1669: Consolidate the duplicated interaction handling in the
action-type branch around run_command, run_command_player, message, and
player_chat by mapping each alias to its InteractionAction constant, value, and
display name. Then perform InteractionAction creation, opts.addInteraction,
storage.saveNPC, action-count lookup, and success messages through one shared
path, preserving slash formatting for command values and existing behavior for
each action type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8f588088-27e7-4847-84c9-597b955b8dee
📒 Files selected for processing (3)
spaceNPC/src/main/java/me/tofaa/entitylib/npc/command/NPCCommand.javaspaceNPC/src/main/java/me/tofaa/entitylib/npc/interactions/InteractionAction.javaspaceNPC/src/main/java/me/tofaa/entitylib/npc/interactions/InteractionHandler.java
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spaceNPC/src/main/java/me/tofaa/entitylib/npc/NPCListenerManager.java`:
- Around line 68-71: Update the sneaking-state handling used by
NPCListenerManager’s interaction flow so isShift reflects the player’s current
sneaking status. Either restore tracking for START_SNEAKING and STOP_SNEAKING
events or read the player’s sneaking state directly, ensuring shift-click
interactions reach handleInteraction with the correct value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 225b9a5e-fc77-46c7-ae76-644ebc77eaeb
📒 Files selected for processing (1)
spaceNPC/src/main/java/me/tofaa/entitylib/npc/NPCListenerManager.java
| boolean isLeftClick = action == WrapperPlayClientInteractEntity.InteractAction.ATTACK; | ||
| boolean isShift = playersHoldingShift.contains(playerId); | ||
|
|
||
| if (isLeftClick) { | ||
| handleInteraction(event, playerId, isLeftClick, isShift, npc); | ||
| } else { | ||
| handleInteraction(event, playerId, isLeftClick, isShift, npc); | ||
| } | ||
| handleInteraction(event, playerId, isLeftClick, isShift, npc); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for usages of `playersHoldingShift` to verify it's never added to.
rg 'playersHoldingShift\.add' srcRepository: Tofaa2/EntityLib
Length of output: 198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Top-level files:\n'
git ls-files | sed -n '1,120p'
printf '\nRelevant file outline:\n'
ast-grep outline spaceNPC/src/main/java/me/tofaa/entitylib/npc/NPCListenerManager.java --view expanded || true
printf '\nSearch for playersHoldingShift references:\n'
rg -n 'playersHoldingShift' spaceNPC/src/main/java || trueRepository: Tofaa2/EntityLib
Length of output: 9831
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository root:"
pwd
echo
echo "Candidate Java files around NPCListenerManager:"
git ls-files '*NPCListenerManager.java' '*EntityLib*.java' '*Listener*.java' | sed -n '1,120p'
echo
echo "All references to playersHoldingShift:"
rg -n 'playersHoldingShift' . || trueRepository: Tofaa2/EntityLib
Length of output: 2139
Missing sneaking state keeps isShift false. The only playersHoldingShift.add(...) is commented out, so shift-clicks are never detected here. Track START_SNEAKING/STOP_SNEAKING or read sneaking state directly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@spaceNPC/src/main/java/me/tofaa/entitylib/npc/NPCListenerManager.java` around
lines 68 - 71, Update the sneaking-state handling used by NPCListenerManager’s
interaction flow so isShift reflects the player’s current sneaking status.
Either restore tracking for START_SNEAKING and STOP_SNEAKING events or read the
player’s sneaking state directly, ensuring shift-click interactions reach
handleInteraction with the correct value.
Summary by CodeRabbit