Skip to content
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

Improve undo log messages in the 2D editor for additional context #42229

Merged

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Sep 21, 2020

Undo/redo log messages will now specify the modified node's name (or number of modified nodes if several were modified). On top of that, the new position/rotation/scale/pivot offset will also be mentioned in the message.

If this feature is desired, I'll look into implementing it for the 3D editor in a future PR 🙂

Preview

Before

Move CanvasItem
Rotate CanvasItem
Resize CanvasItem
Move pivot
Move CanvasItem
# Select 4 CanvasItems…
Move CanvasItem

After

Move CanvasItem "Sprite2D" to (-3, 166)
Rotate CanvasItem "Sprite2D" to -13 degrees
Resize Control "Label" to (143, 32)
Set CanvasItem "Label" Pivot Offset to (13, 18)
Move CanvasItem "Control" to (273, -147)
# Select 4 CanvasItems…
Move 4 CanvasItems

@Calinou Calinou added cherrypick:3.x Considered for cherry-picking into a future 3.x release enhancement topic:editor usability labels Sep 21, 2020
@Calinou Calinou added this to the 4.0 milestone Sep 21, 2020
@Calinou Calinou force-pushed the 2d-editor-improve-undo-log-messages branch from fc055da to a15065e Compare September 21, 2020 21:09
drag_selection,
vformat(
TTR("Set CanvasItem \"%s\" Pivot Offset to (%d, %d)"),
drag_selection[0]->get_name(),
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure that drag_selection.size() > 0 in all this code?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think the drag_selection can be empty in that part of the code. Unless there's a way to trigger the rotation, pivot dragging, etc... without a selected node. It might happen in some weird buggy situation though.

Copy link
Member

Choose a reason for hiding this comment

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

Hm, maybe there should be some ERR_FAIL_COND(drag_selection.size()); at the start of each helper method, just to be safe?

Note that some of those methods actually mutate drag_selection based on filtering selection too.

Copy link
Member

Choose a reason for hiding this comment

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

Bump. If you think it's safe I don't mind merging as is, but we need some resolution to this comment.

@Calinou Calinou force-pushed the 2d-editor-improve-undo-log-messages branch from a15065e to 9f46b69 Compare October 19, 2020 14:26
Undo/redo log messages will now specify the modified node's
name (or number of modified nodes if several were modified).
On top of that, the new position/rotation/scale/pivot offset
will also be mentioned in the message.
@Calinou Calinou force-pushed the 2d-editor-improve-undo-log-messages branch from 9f46b69 to 996740d Compare October 19, 2020 16:59
@akien-mga akien-mga merged commit 368a464 into godotengine:master Oct 26, 2020
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.2.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants