[Proposal] Added Drag and Drop Feature for TimeLine Events for Issue #29

ellogwen commented 3 years ago

This Proposal-Draft reflects the wanted behaviour for #29.

You now can drag and drop events into the timeline. There is a small issues with the special blocks like the if-branching. Therefor this is just a proposal/draft because I didn't wanted to rework some of your stuff to make this PR work properly without asking you first for permission to do so.

Since you do special behaviour in the button callback signals for the IfBranching and the Choice, I would need to refactor this part of your code, if you give me permission to do so.

However, this is just a draft anyway, because you certainly want to change the color scheme (I chose lightpink :P).

You can drag and drop the nodes anywhere at the timeline. The default button behaviour still works as before (adds below selected item or at the end of the timeline)

Important Godot editor has a know bug according to drag and drop and SpinBoxes. When you drag and drop stuff over a SpinBox control, the mouse gets vanished to minus viewport size and stays there until you restart the editor. This bug occurs on some operating systems. To work around this, I provided a script and attached it to your SpinBox control in the WaitSeconds event scene. This script just disables the drag and drop feature for that particular SpinBox control.

ellogwen commented 3 years ago

Erm, git somehow merged my first pull request into this PR. I hope this is not an issue.

coppolaemilio commented 3 years ago

No worries, I'll review both and see what the issues you mentioned are 👍🏻

arnaudvergnet commented 3 years ago

Looks neat! A few things I would change:

Maybe use a blue color for the destination to match the selected color
Automatically select a dragged-in event

I also made a PR #132 modifying dragging behavior inside the timeline, we should make sure it does not create conflict with your PR.

coppolaemilio commented 3 years ago

I tested the PR and it looks really nice! Agree with Arnaud points about making the color of the destination the same as the selected node theme and automatically select the dragged in event. My feedback is:

If possible, make the "drag name" (the little box with the name of the node you are draggin) to get the text from the button label instead.
Remove the new node from the pr so it only includes the changes you made to the drag and drop. We can handle the other PR in a separated thread

ellogwen commented 3 years ago

@coppolaemilio my last commit should reflect your change requests. Drag name is now the button label (or a default if not possible) and I reverted the accidental commit that got mixed in. Note: If you accept my other PR with the call node, it needs the eventbutton script attached to the button in the timeline to make new nodes drag and dropable.

@arnaudvergnet my last commit should reflect your change requests. the dropzone is now the default light blue from the editor and the node gets selected when added.

How can I test my commit with your PR? i am not that git familar and I know basic stuff, but not mixing pull requests together. maybe if you have a few minutes and can explain it to me in the discord, than I am happy to test it. Do i need to create a new PR based on your PR?

Important There is still the issue that If-branches and Question nodes don't add the extra nodes when dragging in. In order to do this the clean way, I would need to remove some stuff you hardcoded in the button events callbacks in the TimelineEditor.gd

coppolaemilio commented 3 years ago

I believe the next steps should be merging main into this branch and resolve the conflicts

ellogwen commented 3 years ago

Oh boy, sometimes I hate git :P Okay, I hand merged (hope it worked) the current main into the feature and couldnt see any immediate issues. @arnaudvergnet Thanks @coppolaemilio for the hint and sorry for beeing so bad at git

arnaudvergnet commented 3 years ago

Oh boy, sometimes I hate git :P Okay, I hand merged (hope it worked) the current main into the feature and couldnt see any immediate issues. @arnaudvergnet Thanks @coppolaemilio for the hint

Perfect! To handle merge conflicts, the best way is to use a good software like intellij or vscode with the "git branch" plugin, it helps a lot.

coppolaemilio commented 3 years ago

I'm ready to push this to main, but I'll be creating some issues and assigning them to you since they are related to this feature. Thanks a lot, it really makes working with timelines a lot better!

dialogic-godot / dialogic

[Proposal] Added Drag and Drop Feature for TimeLine Events for Issue #29 #131