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

Added ability to unfold editor sections when dragging and dropping. #41138

Conversation

EricEzaM
Copy link
Contributor

@EricEzaM EricEzaM commented Aug 9, 2020

Also added editor setting to control the delay used before unfold occurs.

Closes #37524

Edit: oh, and I also cleaned up the gui_input() function a bit. Everything it was doing is encapsulated in unfold() and fold(), so we might as well use those functions since they are there. Now the code is much easier to follow. It's basically just should_unfold ? unfold() : fold()

5YD4QIsyi1

@EricEzaM EricEzaM force-pushed the open-inspector-section-on-drag-and-drop-hover branch from ffd211b to fc5d2e4 Compare August 9, 2020 09:47
@pouleyKetchoupp pouleyKetchoupp added this to the 4.0 milestone Aug 9, 2020
@fire
Copy link
Member

fire commented Aug 9, 2020

Does this code also give the ability to expand all the folds for a specific property? Was trying to get that to work.

@EricEzaM
Copy link
Contributor Author

EricEzaM commented Aug 9, 2020

Does this code also give the ability to expand all the folds for a specific property? Was trying to get that to work.

All the folds for a specific property? Not sure what you mean by that, could you clarify if possible?

@fire
Copy link
Member

fire commented Aug 9, 2020

So I have a property albedo_texture, but it's hidden inside of a "Albedo" group. So I want to show the albedo texture from code, but it's nested and the group nearest to it is not related because albedo_texture is a top level property (like without "/").

[Edited]

I think that is a different issue unrelated, looking at your code.

@EricEzaM
Copy link
Contributor Author

EricEzaM commented Aug 9, 2020

Ah yeah I think I understand what you mean. Like if you had:

  • Section 1
    • Section 2
      • Property

You want to be able to unfold Section 1 and 2 by querying to say "unfold all sections for 'property'".

This PR hasn't got anything to do with that functionality, no.

@fire
Copy link
Member

fire commented Aug 11, 2020

Is it possible to set a highlight on the section? Currently there's no visual feedback.

[Edited] It's more complicated. Not sure if it's worth setting the highlight on drop and then when you move away set a timer to unset the highlight. However, it's worth trying to show activity.

fire added a commit to godot-extended-libraries/godot-fire that referenced this pull request Aug 11, 2020
Also added editor setting to control the delay used before unfold occurs.

Co-authored-by: Eric M <itsjusteza@gmail.com>

godotengine/godot#41138
fire added a commit to godot-extended-libraries/godot-fire that referenced this pull request Aug 11, 2020
Also added editor setting to control the delay used before unfold occurs.

Co-authored-by: Eric M <itsjusteza@gmail.com>

godotengine/godot#41138
@EricEzaM
Copy link
Contributor Author

@fire It might actually be worth implementing a highlight whenever a user hovers over a section, no matter if they are dragging or not. I think that would be another PR tho.

@EricEzaM EricEzaM force-pushed the open-inspector-section-on-drag-and-drop-hover branch from fc5d2e4 to 618e8aa Compare August 11, 2020 23:04
@@ -231,6 +231,9 @@ class EditorInspectorSection : public Container {
Color bg_color;
bool foldable;

Timer *dropping_unfold_timer;
Copy link
Member

Choose a reason for hiding this comment

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

= nullptr.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't understand. memnew(Timer) is used in the constructor. Why does it need = nullptr? That is not used anywhere else in the codebase where a Timer* is defined.

Copy link
Member

Choose a reason for hiding this comment

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

ManagedCallableMiddleman *managed_callable_middleman = memnew(ManagedCallableMiddleman);
Here's an example of:

Timer *dropping_unfold_timer = memnew(Timer);

Copy link
Contributor Author

@EricEzaM EricEzaM Aug 11, 2020

Choose a reason for hiding this comment

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

Yeah so that's in the header, I'm saying the timer is initialised in the constructor:

https://github.com/godotengine/godot/pull/41138/files#diff-554c9c97362d5a6f63b7244253542ffeR1312

I'm just wondering why this needs to explicitly specify = nullptr when I can't seem to find that happening anywhere else in the codebase. For example:

Timer *range_click_timer;

It is then initialised in the constructor:

range_click_timer = memnew(Timer);
range_click_timer->connect("timeout", callable_mp(this, &SpinBox::_range_click_timeout));
add_child(range_click_timer);

Copy link
Member

Choose a reason for hiding this comment

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

I don't remember exactly, but every so often Akien runs a script that moves the construction stuff to initialization.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah ok, no worries. I was just wondering if there was some convention as I couldn't see other cases of this being used.

@stijn-h
Copy link
Contributor

stijn-h commented Aug 15, 2020

I have an old PR (#39650) that implements unfolding with highlighting. Without highlighting it's hard to know that you can even unfold. Also, if you look at the code you can see it is not that much work, so I think you should add it to your PR.

fire added a commit to godot-extended-libraries/godot-fire that referenced this pull request Aug 16, 2020
Also added editor setting to control the delay used before unfold occurs.

Co-authored-by: Eric M <itsjusteza@gmail.com>

godotengine/godot#41138
@EricEzaM
Copy link
Contributor Author

Updated. You can only expand sections if the sections contains properties which accept the drag data. Section is also highlighted as such:

ed_section_highlight_drag

@EricEzaM EricEzaM force-pushed the open-inspector-section-on-drag-and-drop-hover branch from 618e8aa to b34a7b5 Compare August 18, 2020 01:56
@EricEzaM EricEzaM force-pushed the open-inspector-section-on-drag-and-drop-hover branch from b34a7b5 to de7f8a9 Compare August 18, 2020 01:57
@EricEzaM
Copy link
Contributor Author

EricEzaM commented Aug 18, 2020

Sorry... forgot to rebase before pushing so some unnecessary reviewers were added. Apologies. They can be removed if a mod sees this and can do so.

@EricEzaM EricEzaM force-pushed the open-inspector-section-on-drag-and-drop-hover branch from de7f8a9 to 484c951 Compare August 18, 2020 02:05
@EricEzaM EricEzaM requested a review from a team as a code owner August 18, 2020 02:05
@EricEzaM EricEzaM force-pushed the open-inspector-section-on-drag-and-drop-hover branch from 484c951 to 24eeb89 Compare August 18, 2020 02:07
fire added a commit to godot-extended-libraries/godot-fire that referenced this pull request Aug 18, 2020
Also added editor setting to control the delay used before unfold occurs.

Co-authored-by: Eric M <itsjusteza@gmail.com>

godotengine/godot#41138
@fire
Copy link
Member

fire commented Aug 18, 2020

Looks great. [Tested on my backport to 3.2.]

@akien-mga akien-mga requested a review from groud August 18, 2020 17:37
Also added editor setting to control the delay used before unfold occurs.
@EricEzaM EricEzaM force-pushed the open-inspector-section-on-drag-and-drop-hover branch from 24eeb89 to 7cc1b0f Compare August 25, 2020 09:59
@EricEzaM EricEzaM requested review from groud and removed request for a team August 25, 2020 10:01
@akien-mga akien-mga merged commit d7c77f6 into godotengine:master Aug 25, 2020
@akien-mga
Copy link
Member

Thanks!

@EricEzaM EricEzaM deleted the open-inspector-section-on-drag-and-drop-hover branch August 25, 2020 12:34
fire added a commit to godot-extended-libraries/godot-fire that referenced this pull request Aug 26, 2020
Also added editor setting to control the delay used before unfold occurs.

Co-authored-by: Eric M <itsjusteza@gmail.com>

godotengine/godot#41138
fire added a commit to godot-extended-libraries/godot-fire that referenced this pull request Aug 26, 2020
Also added editor setting to control the delay used before unfold occurs.

Co-authored-by: Eric M <itsjusteza@gmail.com>

godotengine/godot#41138
fire added a commit to godot-extended-libraries/godot-fire that referenced this pull request Aug 26, 2020
Also added editor setting to control the delay used before unfold occurs.

Co-authored-by: Eric M <itsjusteza@gmail.com>

godotengine/godot#41138
fire added a commit to godot-extended-libraries/godot-fire that referenced this pull request Aug 26, 2020
Also added editor setting to control the delay used before unfold occurs.

Co-authored-by: Eric M <itsjusteza@gmail.com>

godotengine/godot#41138
fire added a commit to godot-extended-libraries/godot-fire that referenced this pull request Aug 30, 2020
Also added editor setting to control the delay used before unfold occurs.

Co-authored-by: Eric M <itsjusteza@gmail.com>

godotengine/godot#41138
fire added a commit to godot-extended-libraries/godot-fire that referenced this pull request Aug 30, 2020
Also added editor setting to control the delay used before unfold occurs.

Co-authored-by: Eric M <itsjusteza@gmail.com>

godotengine/godot#41138
fire added a commit to godot-extended-libraries/godot-fire that referenced this pull request Sep 5, 2020
Also added editor setting to control the delay used before unfold occurs.

Co-authored-by: Eric M <itsjusteza@gmail.com>

godotengine/godot#41138
fire added a commit to godot-extended-libraries/godot-fire that referenced this pull request Sep 5, 2020
Also added editor setting to control the delay used before unfold occurs.

Co-authored-by: Eric M <itsjusteza@gmail.com>

godotengine/godot#41138
fire added a commit to godot-extended-libraries/godot-fire that referenced this pull request Sep 20, 2020
Also added editor setting to control the delay used before unfold occurs.

Co-authored-by: Eric M <itsjusteza@gmail.com>

godotengine/godot#41138
fire added a commit to godot-extended-libraries/godot-fire that referenced this pull request Sep 22, 2020
Also added editor setting to control the delay used before unfold occurs.

Co-authored-by: Eric M <itsjusteza@gmail.com>

godotengine/godot#41138
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Open inspector tabs by hovering over them during drag and drop
6 participants