Skip to content

Special dice button sequencing / undo fix#14266

Open
riverwanderer wants to merge 17 commits intovassalengine:masterfrom
riverwanderer:release-3.7-SpecialDiceButtonUNDO
Open

Special dice button sequencing / undo fix#14266
riverwanderer wants to merge 17 commits intovassalengine:masterfrom
riverwanderer:release-3.7-SpecialDiceButtonUNDO

Conversation

@riverwanderer
Copy link
Copy Markdown
Collaborator

@riverwanderer riverwanderer commented Oct 3, 2025

AI (Gemini)-guided fix for 14237.

Summary from the AI sessions; changes primarily involve:

  1. Adding the lastRollResults field to track the visual state for undo.
  2. Updating the DR() method to implement the new roll/undo logic, including capturing the previousRollResults, using sendAndLog(finalCommand), and updating lastRollResults. Essentially integrating functionality from DoActionButton including loop control and pre/post Global Hotkeys.
  3. Modifying reportResults(int[] results) to only update the visual state (as the chat/property update is now handled elsewhere in DR()).
  4. Updating addTo(Buildable parent) to initialize lastRollResults.
  5. Refactoring the ShowResults command to accept and manage both rolls (new state) and previousRolls (state to restore on undo) to support the new undo functionality.
  6. Adding a two-argument constructor to ShowResults for binary compatibility.
  7. Updating encode and decode in the main class to handle the new ShowResults constructor signature

…h dice roll.

Creating a single masterCommand in DR().

Appending all actions (Hotkeys, Chat Report, Property Set, and the ShowResults command) to this single master command.

Calling GameModule.getGameModule().sendAndLog(masterCommand) only once at the end of DR().

This process ensures that a single user action (the dice roll) generates one log entry, which then only requires one undo step to revert the entire sequence, correctted in previous Commit.
…sync with dice roll."

This reverts commit 5873501.
Accidentally pushed.
…atibility, defaulting the previousRolls to EMPTY.
@riverwanderer riverwanderer linked an issue Oct 3, 2025 that may be closed by this pull request
@riverwanderer riverwanderer added this to the 3.7.19 milestone Oct 3, 2025
@riverwanderer riverwanderer added the Awaiting Testing/Feedback Change is good, but neeeds testing or feedback label Oct 3, 2025
@riverwanderer
Copy link
Copy Markdown
Collaborator Author

PR tested OK against the Undo and Remote sequencing issues reported in the linked issues.

@riverwanderer
Copy link
Copy Markdown
Collaborator Author

riverwanderer commented Oct 4, 2025

Unfortunately, the change nerfs the Symbolic Dice button's Looping and the associated Pre & Post action Global Hotkeys. Doubly unfortunate, since these functions are really redundant and it would have been better if Logging/UnDo worked correctly, even without looping (which a GHK to a normal action button can accomplish).

More work required.

@riverwanderer riverwanderer self-assigned this Oct 4, 2025
@uckelman uckelman modified the milestones: 3.7.19, 3.7.20 Dec 30, 2025
@uckelman uckelman modified the milestones: 3.7.20, 3.7.21 Feb 6, 2026
@uckelman uckelman removed this from the 3.7.21 milestone Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Awaiting Testing/Feedback Change is good, but neeeds testing or feedback

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Symbolic Dice Button breaks atomic undo

2 participants