Reset snap point variables in DragManager.drag_end #75
Conversation
In the `drag_end` function, make sure to reset `previewing_snap_point` in addition to `preview_block`. This ensures that block snapping will work correctly if the same block is dragged a second time. In addition, defer cleaning up preview_block and previewing_snap_point to the end of the function. This avoids some confusion related to nested if statements. https://phabricator.endlessm.com/T35524
|
Is this one a separate snap issue? I can see it in main and in this PR: Grabacion.de.pantalla.desde.2024-06-24.16-15-07.webmBasically:
The first 2 separated blocks can't be dragged. |
It's an unrelated issue. If I start dragging a block but let it snap back to the original snap point, then start dragging it a second time, it won't snap to that snap point again. (Which is a problem because the blocks still end up looking like they're snapped together even though they aren't). I think that issue you spotted is unrelated, but I'm continuing to fiddle with this code in a separate cleanup right now so I'll see if I can solve that at the same time. |
Previously, blocks were continuing to occupy space after their children were detached, making it difficult to interact with other blocks in the canvas.
|
Oh, I figured that one out, and it does sort of relate to my other cleanups here. It's related to the block still reserving all of that space it used to occupy so clicks weren't falling through to the nodes which were detached and pushed down in the z stack. I might have hit it with a bigger hammer than necessary, but, it works. I'll stuff this all in the same PR so we don't have to merge a bunch of pull requests in order. I'm a little unsure about 02bbbbf. I think it feels nicer, and I was really happy to get rid of some weird local / global coordinate conversions. But the names of things seem wonky: there's a lot of redundancy. |
dylanmccall
force-pushed
the
T35524-block-snap-fixes
branch
from
1aedbce to
109b17c
Compare
This change moves several tightly coupled variables in DragManager into a separate class, which makes it easier to organize resources associated with a particular interaction.
Instead, check if a block is on the canvas using `Node.is_ancestor_of`. This avoids needing to keep the property in sync between multiple places.
dylanmccall
force-pushed
the
T35524-block-snap-fixes
branch
from
109b17c to
105247c
Compare
|
|
||
| func _remove_block(): | ||
| target_snap_point = null | ||
| _block.g() |
There was a problem hiding this comment.
When the block becomes removed, it shows error:
res://addons/block_code/drag_manager/drag_manager.gd:62 - Invalid call. Nonexistent function 'g' in base 'MarginContainer (EntryBlock)'.
There was a problem hiding this comment.
Oh that's so weird! What did I do to that commit? :/ Thanks for spotting. I fixed that in the newest commit :)
|
Thank you both! Dragging works much better now! |
This addresses a bunch of little things related to drag and drop in the editor. Importantly, I fixed two small bugs that occur when snapped nodes are separated from each other. There's also a bit of code cleanup, originally inspired by
previewing_snap_pointandpreview_blockinDragManagernot being in sync with each other. In the end, I put those concepts into a separateDragManager.Dragclass which manages a single drag & drop operation. That also provided a way to better organize our code related to filtering and sorting snap points, now using the stuff built in toArray.I also ended up adding a change related to https://phabricator.endlessm.com/T35525, making a block fade out slightly if it is going to be deleted.
https://phabricator.endlessm.com/T35524