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

T35541 block code in bottom panel #117

Merged
merged 12 commits into from
Jul 9, 2024
Merged

Conversation

dylanmccall
Copy link
Contributor

@dylanmccall dylanmccall requested review from manuq and wnbaum July 3, 2024 23:46
@dylanmccall dylanmccall marked this pull request as draft July 3, 2024 23:46
@dylanmccall
Copy link
Contributor Author

dylanmccall commented Jul 3, 2024

This is technically ready to review, but I'm marking it as a draft because we should wait for additional feedback related to the design.

The big change here is removing the block code editor as a main panel, and instead adding it to the bottom bar:

Screenshot from 2024-07-03 16-49-38

As before, it appears automatically if a BlockCode node is selected. In addition, it appears if a node containing a BlockCode node is selected. That is useful, because now we can work with the Block Code editor while manipulating objects in the canvas! For example, the Ball in the scene is highlighted because I clicked it, and the block code editor shows the code associated with it.

The editor is very, very busy here, but fortunately Godot does have a button on the bottom right corner to expand the bottom panel, which results in a similar experience to what we have right now:

Screenshot from 2024-07-03 16-52-33

With Block Code in the bottom panel, we gain another neat trick: we can be cleverer about empty states. In particular, since we are working in the same context as the rest of the scene, the Block Code editor can provide helper text if the scene is empty, and it can guide the user to creating a BlockCode node when they have selected a node in the scene. For example, here I have selected the Camera2D node in the scene, and the editor displays a convenient button to add block code for that node:

Screenshot from 2024-07-04 18-03-32

… or override block code for a node, if the node came with its own BlockCode. (An edge case, but now easier to deal with).

Screenshot from 2024-07-04 18-02-50

@dylanmccall dylanmccall force-pushed the T35541-block-code-in-bottom-panel branch 2 times, most recently from 73d8e14 to 5264ba3 Compare July 5, 2024 01:02
@manuq
Copy link
Contributor

manuq commented Jul 5, 2024

@dylanmccall this is a huge improvement! I'm not sure about displaying the Block Code editor for if the parent node with BlockCodeNode attached is selected in the Scene Tree. Here I tried the branch adding blocks to AnimationPlayer and TileMap. When I click these nodes, sometimes I get their corresponding editors (Animation bottom dock, TileMap bottom dock) and sometimes the blocks editor.

@dylanmccall dylanmccall marked this pull request as ready for review July 5, 2024 18:56
@dylanmccall
Copy link
Contributor Author

dylanmccall commented Jul 5, 2024

@dylanmccall this is a huge improvement! I'm not sure about displaying the Block Code editor for if the parent node with BlockCodeNode attached is selected in the Scene Tree. Here I tried the branch adding blocks to AnimationPlayer and TileMap. When I click these nodes, sometimes I get their corresponding editors (Animation bottom dock, TileMap bottom dock) and sometimes the blocks editor.

Hm, okay, I was wondering if that might cause conflicts. In 0026aab I changed this so the editor only opens itself if the user chooses a Block Code node explicitly, which should solve the problem there and is better in line with how the Animation and Tile Map panels behave.

@wnbaum
Copy link
Contributor

wnbaum commented Jul 5, 2024

I just tested this out on my end. I like the behavior of opening the Block Code panel when a BlockCode node is selected, but just switching the script (not opening the panel) when a parent to a BlockCode node is selected. I think I like this layout better - on my small laptop it is a few extra clicks to get it looking organized, but on larger screens it's way better. It would be good to get some feedback from the learning team to see what they think about the new layout. I wonder if they think being able to see the scene/properties of nodes is worth the layout change.

This new bottom panel control will appear whenever a BlockCode node is
present in the scene.

https://phabricator.endlessm.com/T35541
To improve the experience when using block coding side by side with the
2D or 3D canvas, the Block Code panel will display block code associated
with the selected node.

https://phabricator.endlessm.com/T35541
When editing a block code resource in the context of a scene, make sure
we are saving the result in the same scene by creating a new copy. This
improves the workflow when editing BlockCode nodes from instantiated
child scenes: a user is unlikely to expect a change with the Block Code
editor in one scene to modify a different scene file.

https://phabricator.endlessm.com/T35541
This reorganizes the empty state widgets in BlockCanvas, adding a new
set of actions when the user selects a node which contains block code,
but the block code can't be edited. This commit also includes changes
to better handle when a BlockCode node is inside a node from a different
scene but with editable children.

https://phabricator.endlessm.com/T35541
We want the buttons in the block canvas empty state to have a visible
outline against the default background. We can achieve this by using the
"InspectorActionButton" type variation, which is used for a similar
situation in the Godot editor's built in Inspector panel.

https://phabricator.endlessm.com/T35541
@dylanmccall dylanmccall force-pushed the T35541-block-code-in-bottom-panel branch from 0026aab to 875a7c8 Compare July 8, 2024 18:07
In order for a node to be selected on the 2D canvas while its associated
BlockCode is being edited, automatically select the current BlockCode
node's parent. This requires some changes in BlockCodePlugin to deal
with cases when multiple nodes are selected in the Scene panel and in
the Inspector panel.

https://phabricator.endlessm.com/T35541
With the change to always select a BlockCode node's parent along with
the BlockCode node itself, it is difficult to select just a BlockCode
node to delete it. To work around this, add a button to delete the
current block code node.

https://phabricator.endlessm.com/T35541
@dylanmccall dylanmccall force-pushed the T35541-block-code-in-bottom-panel branch from 85adf70 to ed99a84 Compare July 8, 2024 23:17
@dylanmccall
Copy link
Contributor Author

This got a +1 from @jofilizola. I added a couple bits based on her feedback:

  • Selecting a BlockCode node causes its parent node to be selected, which gives us better feedback in the 2D canvas.
  • (To go with this, I added a way in the Block Code editor to delete a block code script without having to fight with the Scene panel).
  • I made that "Add Block Code" button stand out against the background.

Happy to merge this any time :)

Copy link
Contributor

@wnbaum wnbaum left a comment

Choose a reason for hiding this comment

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

Awesome stuff! Once I get my zooming and panning merged I think it will go really great with this.

One thing I noticed is that when selecting a BlockCode node there is a significant amount of lag on my end, presumably from loading all the blocks in (not the fault of this PR at all). We should figure out what's causing that.

@wnbaum wnbaum merged commit d82ce7d into main Jul 9, 2024
2 checks passed
@wnbaum wnbaum deleted the T35541-block-code-in-bottom-panel branch July 9, 2024 18:15
This pull request was closed.
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