Fix drawing slider with touch inserts a random control point at beginning #28870
+209
−61
Conversation
This file contains 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
… manager This is opposite to how it works in the real game, where the input manager is updated first before anything else below it. Fixing this shows more logical behaviour from placement blueprints (i.e. this fixes `TestSliderDrawingViaTouch` still not succeeding despite the success in `TestTapSliderButtonThenDragAfterTappingPlayfield`)
Seemingly happening due to the test adding an unnecessary step that doesn't move the touch, and therefore making the touch trigger a right click due to not moving in time.
Tapping the compose area after selecting a tool should begin placement (commit = true) of the object provided by the selected tool.
Turns out `HitCirclePlacementBlueprint` suffers from this exact same issue, since it finalises placement on `OnMouseDown`, while the snap result at that point is from last frame instead of current one. This fix ensures that the snap result is updated before the blueprints even handle any input event. Feels better in general.
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.
As can be seen in the video attached in #26509 (comment), when drawing a slider after touching the playfield, a control point is inserted at the point where the user put their finger before drawing.
This is caused by
SliderPlacementBlueprintreading the snap result of the last frame instead of the current frame where the touch input was triggered, therefore beginning a curve with the old mouse position.The solution in this PR might not sit well with everyone because it's essentially a schedule underneath a three-line comment block, but I would like to say that the flow at which the blueprint receives the snap result warrants a schedule so that the placement begins with the snap result constructed based off the current update frame.
Note that
PlacementBlueprintTestScenerequired a refactor so that the solution works. While this might feel like the solution is flaky,PlacementBlueprintTestScenewas written in a way that operates opposite to how the editor naturally works. By that I mean the test scene updates the snap result before the test input manager's update flow, so even ifbeginCurveis scheduled in the mouse event, it'll execute before the next snap result update (in other words, the snap result update flow in the test scene always reads the input state of the last frame)