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

BlockCanvas: Implement drag & drop the node's property from Inspector #296

Merged
merged 3 commits into from
Nov 5, 2024

Conversation

starnight
Copy link
Contributor

@starnight starnight commented Oct 30, 2024

Dragging the node's property from Inspector shows the object's type as "obj_property". So, allow the "obj_property" type in _can_drop_data() and handle it in _drop_data(). Then, implement getting the property's value as a Variable block with _drop_obj_property() in detail.

And, if the modifier CTRL key is pressed, then drop a setting the property's value block.

https://phabricator.endlessm.com/T35649

@starnight starnight force-pushed the T35649-drop-object-property branch 2 times, most recently from a529d11 to 00d4842 Compare October 30, 2024 10:36
@starnight
Copy link
Contributor Author

Here is the demo of how to drag & drop the node's property from Inspector
Screencast From 2024-10-30 18-27-59.webm

. new(
&"%s_get_%s" % [obj_name, prop_name],
obj_name,
"The %s property" % prop_name,
Copy link
Member

Choose a reason for hiding this comment

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

BTW a few weeks ago I looked into whether it was possible to look up the documentation for a property from a gdscript addon. Sadly, it's not.

var block_definition = (
BlockDefinition
. new(
&"%s_get_%s" % [obj_name, prop_name],
Copy link
Member

Choose a reason for hiding this comment

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

How about a setter?

Copy link
Contributor

Choose a reason for hiding this comment

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

I loosely remember Orchestrator (or was Godot 3 visual scripting?) using a modifier key while dropping to generate a setter. I don't remember if Shift or Ctrl.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could add the property get & setter to the Variables section of the picker when it has been dragged from the inspector?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we could add the property get & setter to the Variables section of the picker when it has been dragged from the inspector?

This is worth a new feature ticket.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both:

I loosely remember Orchestrator (or was Godot 3 visual scripting?) using a modifier key while dropping to generate a setter. I don't remember if Shift or Ctrl.

and

Maybe we could add the property get & setter to the Variables section of the picker when it has been dragged from the inspector?

acknowledge #296 (comment)!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After think twice, using a modifier key while dropping to generate a setter mentioned in #296 (comment) is a good, simple and quick implementation for now.

@starnight
Copy link
Contributor Author

Here is the demo including both set and get blocks of the property dragged from Inspector.
Screencast From 2024-10-31 15-15-53.webm

Copy link
Contributor

@manuq manuq left a comment

Choose a reason for hiding this comment

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

Only a couple nits requested. I notice that the property names in the catalog have regressed, so I will open an issue and a separate PR for them. This follows what's in the catalog, so it's OK.

addons/block_code/ui/block_canvas/block_canvas.gd Outdated Show resolved Hide resolved
addons/block_code/ui/block_canvas/block_canvas.gd Outdated Show resolved Hide resolved
addons/block_code/ui/block_canvas/block_canvas.gd Outdated Show resolved Hide resolved
addons/block_code/ui/block_canvas/block_canvas.gd Outdated Show resolved Hide resolved
@manuq
Copy link
Contributor

manuq commented Nov 4, 2024

@starnight this is the regression I was talking about: #303 . It is explained in the issue. If you agree, please adjust this PR accordingly.

Extract dropping nodes codes as function _drop_node from _drop_data, if
the type is "nodes". So, other types such as "obj_property" can be added
later.

https://phabricator.endlessm.com/T35649
Dragging the node's property from Inspector shows the object's type as
"obj_property". So, allow the "obj_property" type in _can_drop_data()
and handle it in _drop_data(). Then, implement getting the property's
value as a Variable block with _drop_obj_property() in detail.

https://phabricator.endlessm.com/T35649
… if CTRL is pressed

Drag & drop the node's set property block from Inspector if the modifier
key CTRL is pressed. To detect pressing CTRL key, hook the input event
and set the _modifier_ctrl flag with the CTRL key's pressed state. If
the _modifier_ctrl is true, then drop a set Variable block. Otherwise,
it should be a get Variable block of the property.

https://phabricator.endlessm.com/T35649
Copy link
Contributor

@manuq manuq left a comment

Choose a reason for hiding this comment

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

Looks good now! Please note that the Ctrl shortcut to obtain a setter is a bit hidden.

@manuq manuq merged commit 11e00e1 into main Nov 5, 2024
3 checks passed
@manuq manuq deleted the T35649-drop-object-property branch November 5, 2024 11:38
starnight added a commit that referenced this pull request Nov 6, 2024
Add how to drag & drop node's property as getter & setter blocks from
the Inspector dock.

This follows #296.
starnight added a commit that referenced this pull request Nov 6, 2024
Add how to drag & drop node's property as getter & setter blocks from
the Inspector dock.

This follows #296.

https://phabricator.endlessm.com/T35649
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants