-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Quality of Life update #964
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -145,9 +145,38 @@ public static object HandleCommand(JObject @params) | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case "restore_package": | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return RestorePackage(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Undo/Redo | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case "undo": | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string groupName = Undo.GetCurrentGroupName(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Undo.PerformUndo(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string message = string.IsNullOrEmpty(groupName) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? "Undo performed (stack may be empty)." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : $"Undid: {groupName}"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (EditorApplication.isPlaying) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| message += " Warning: undo during play mode may have unexpected effects."; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new SuccessResponse(message, new | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| undone_group = string.IsNullOrEmpty(groupName) ? (string)null : groupName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| next_group = Undo.GetCurrentGroupName() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case "redo": | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Undo.PerformRedo(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string nextGroup = Undo.GetCurrentGroupName(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string message = "Redo performed."; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (EditorApplication.isPlaying) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| message += " Warning: redo during play mode may have unexpected effects."; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new SuccessResponse(message, new | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| current_group = string.IsNullOrEmpty(nextGroup) ? (string)null : nextGroup | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+151
to
+174
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string groupName = Undo.GetCurrentGroupName(); | |
| Undo.PerformUndo(); | |
| string message = string.IsNullOrEmpty(groupName) | |
| ? "Undo performed (stack may be empty)." | |
| : $"Undid: {groupName}"; | |
| if (EditorApplication.isPlaying) | |
| message += " Warning: undo during play mode may have unexpected effects."; | |
| return new SuccessResponse(message, new | |
| { | |
| undone_group = string.IsNullOrEmpty(groupName) ? (string)null : groupName, | |
| next_group = Undo.GetCurrentGroupName() | |
| }); | |
| } | |
| case "redo": | |
| { | |
| Undo.PerformRedo(); | |
| string nextGroup = Undo.GetCurrentGroupName(); | |
| string message = "Redo performed."; | |
| if (EditorApplication.isPlaying) | |
| message += " Warning: redo during play mode may have unexpected effects."; | |
| return new SuccessResponse(message, new | |
| { | |
| current_group = string.IsNullOrEmpty(nextGroup) ? (string)null : nextGroup | |
| }); | |
| try | |
| { | |
| string groupName = Undo.GetCurrentGroupName(); | |
| Undo.PerformUndo(); | |
| string message = string.IsNullOrEmpty(groupName) | |
| ? "Undo performed (stack may be empty)." | |
| : $"Undid: {groupName}"; | |
| if (EditorApplication.isPlaying) | |
| message += " Warning: undo during play mode may have unexpected effects."; | |
| return new SuccessResponse(message, new | |
| { | |
| undone_group = string.IsNullOrEmpty(groupName) ? (string)null : groupName, | |
| next_group = Undo.GetCurrentGroupName() | |
| }); | |
| } | |
| catch (Exception ex) | |
| { | |
| return new ErrorResponse($"Failed to perform undo: {ex.Message}"); | |
| } | |
| } | |
| case "redo": | |
| { | |
| try | |
| { | |
| Undo.PerformRedo(); | |
| string nextGroup = Undo.GetCurrentGroupName(); | |
| string message = "Redo performed."; | |
| if (EditorApplication.isPlaying) | |
| message += " Warning: redo during play mode may have unexpected effects."; | |
| return new SuccessResponse(message, new | |
| { | |
| current_group = string.IsNullOrEmpty(nextGroup) ? (string)null : nextGroup | |
| }); | |
| } | |
| catch (Exception ex) | |
| { | |
| return new ErrorResponse($"Failed to perform redo: {ex.Message}"); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Undo/redo handlers ignore the boolean return value from Undo.PerformUndo/Redo.
Undo.PerformUndo()/Undo.PerformRedo()returnboolto indicate whether an operation actually occurred, but the handlers always report success, even when the stack is empty. Consider using the return value to tailor the message (or surface an error) when nothing was undone/redone so CLI consumers, especially automation, can reliably detect no-op cases.Suggested implementation:
To fully address your comment for both undo and redo:
Apply an analogous change in the
"redo"case:bool performed = Undo.PerformRedo();performed(e.g., "Nothing to redo (redo stack is empty)." vs "Redo performed." / $"Redid: {groupName}").performedflag in the response payload and adjust any group-related fields accordingly.If there are tests or CLI consumers that parse the response, update or add tests to assert on the new
performedfield and the updated messages for no-op scenarios.