[Editor] Change TransformationGizmo Update to be synchronous and change entity duplication behavior#3186
Open
Basewq wants to merge 1 commit into
Open
[Editor] Change TransformationGizmo Update to be synchronous and change entity duplication behavior#3186Basewq wants to merge 1 commit into
Basewq wants to merge 1 commit into
Conversation
…tity duplication from gizmo to EditorGameEntityTransformService.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Details
This serves as a bit of prep work leading to #3183.
The purpose of this PR is to remove the async part of
TransformationGizmo.Update.The only two places where async is used is:
W/E/Rkeys)Ctrl-> Drag gizmo to duplicate entities (note: an existing quirk is you must focus in the scene beforeCtrlkey down is detected, which is not fixed in this PR).My changes and reasoning:
No longer await UI invoke. This is an example of
game -> editor -> game, which relies on theeditor -> gamereturn to set update the transform gizmo mode (refer to Related Issue section for a bit more context).It now just treats it as a fire-and-forget request to the editor, which will 'eventually' update the mode back to the game thread.
I'm not really sure if there is a cleaner solution, other than either 'predicting' the mode change (ie. update directly as well), or block the gizmo until the editor 'confirms' the request.
Moved the entity duplication logic out of the transform gizmo and into EditorGameEntityTransformService.
This makes the transform gizmo reusable for other things which may or may not have 'duplication' logic (eg. spline tool).
Another change, which might be more debatable, is now the initial duplication is only done on the game side (temp cloned entities), then at the end of the transform the request is sent to the editor.
Doing it this way also means there is only one undo/redo transaction, whereas the previous behavior had two transactions (1st transaction on the drag start to generate entities in place, then a 2nd transaction for the position update), ie. you had to undo twice to actually remove the duplicated objects.
One other quirk is when duplicating a large number of entities (eg. use 3rd Person Platformer template and duplicate the entire 'Platform' folder via
Ctrl+ drag), the original behavior has a large initial freeze since it clones at the start.This change makes the large freeze occur at the end of the transform (along with a progress bar popup if longer than 500ms), however this does introduce a different quirk where the preview entities disappear then the real objects are added afterwards.
(New functionality) There is also now the ability to hit Escape key to cancel the transform when you're in the middle of transforming (no transaction goes the editor).
Related Issue
Prep work for #3183
Regarding further context for change 1:
There is a bit of concern with the current architecture of the 'game services', which may need additional thoughts on.
The issue is the back and forth marshaling between 'WPF UI thread' (ie. where the asset/quantum data live) and the 'game thread' (ie. live scene editor).
There are some 'game services' that contain both the game scene data and the 'view model'/UI thread data (ie. implement
IEditorGameServiceandIEditorGameViewModelServiceas a single class), and can end up doing things like'game thread' -> await InvokeAsync to 'UI thread' -> InvokeAsync to 'game thread', which is exactly what the gizmo code was doing.Regarding further context for change 2:
The future goal is to change classes that inherit
EditorGameMouseServiceBaseto only have sync update methods, so they can ultimately be bundled through some 'edit mode' class and updated in some specified order (and optionally enabled/disabled depending on the mode), rather than the free-for-all approach that's happening now (they will still have the option to register an independent task via ScriptSystem if they need it).Types of changes
Not sure if doc change is required or not,
TranslationGizmo/ScaleGizmo/RotationGizmoare public, however these gizmos are part ofStride.Assets.Presentation, which the general user cannot accidentally stumble into due to the lack of proper plugin integration.Checklist