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

Make possible to favorite properties in the inspector #97415

Merged

Conversation

YeldhamDev
Copy link
Member

@YeldhamDev YeldhamDev commented Sep 24, 2024

Closes godotengine/godot-proposals#4274.

2024-09-24.10-57-26.mp4

Limitations

Incompatible with built-in scripts, as validating the favorites file with them in mind would mean extra loading of scenes at startup.

Sponsored By: 🐺 Lone Wolf Technology / 🍀 W4 Games.

@timothyqiu
Copy link
Member

Favoriting Button's tooltip_text and text makes two "Text" properties 🤣

image

The group "Tooltip" is lost here.

If this is not expected, I think displaying "Tooltip Text" is not an ideal solution as it means doubling translator's work for grouped properties. Maybe displaying a group that's always unfolded?

@Mickeon
Copy link
Contributor

Mickeon commented Sep 24, 2024

This is not necessarily related to this proposal as the rest of the Inspector suffers from it too, but I don't like that there's no way to know that Inspector categories can be right-clicked (not even a text color on hover). This makes the "Unfavourite all" option undiscoverable.

@tetrapod00
Copy link
Contributor

tetrapod00 commented Sep 24, 2024

Could we make Node3D.Position, Node3D.Rotation, and Node3D.Scale (and the corresponding 2D properties) favorites by default? This would make the default editor show those properties at the top of the inspector, which is a very common request. It would also increase the discoverability of this feature.

Favoriting/reordering the entire Node2D/Node3D property block to the top of the inspector would also be a way to do this, but this PR seems to only allow favoriting individual properties.

@ryevdokimov
Copy link
Contributor

Could we make Node3D.Position, Node3D.Rotation, and Node3D.Scale (and the corresponding 2D properties) favorites by default? This would make the default editor show those properties at the top of the inspector, which is a very common request. It would also increase the discoverability of this feature.

I think the inspector can get crowded pretty easily and would prefer not to have additional and redundant data shown by default.

@tetrapod00
Copy link
Contributor

I think the inspector can get crowded pretty easily and would prefer not to have additional and redundant data shown by default.

Notably this implementation pulls properties to the top, it doesn't duplicate them. Setting these properties as favorites by default does obscure the fact that they are Node3D properties, though.
godot windows editor x86_64_sPe583lPty
godot windows editor x86_64_QlHUVfnqBK

@ryevdokimov
Copy link
Contributor

ryevdokimov commented Sep 24, 2024

Notably this implementation pulls properties to the top, it doesn't duplicate them. Setting these properties as favorites by default does obscure the fact that they are Node3D properties, though.

I should have paid attention more, thanks for pointing that out. That is surprising though, and not what I would expect for the reason you stated. I am ambivalent about the fact it does that, but I think it makes a different argument against having them be favorited by default.

@jaydensipe
Copy link
Contributor

jaydensipe commented Sep 24, 2024

Setting these properties as favorites by default does obscure the fact that they are Node3D properties, though.

Wondering if something could be done like this, where it would show the Object/Class name before the property?

test

@tetrapod00
Copy link
Contributor

My view is that for most workflows involving the editor, you want to access position, rotation, and scale most often, at the top of the inspector, then you want to access properties you defined on your classes, then you want to access properties on built-in parent classes. As is, the ordering of properties is mostly good except for the fact that transform properties are hidden at the bottom of the list.

If most people setting up their editor will favorite these, then we should do it by default. The favorites are stored in editor settings and are per-project, so whichever default is chosen does matter - each new project will be affected by it. It will be three clicks to change it, whichever default we choose, so let's choose the default that serves the most people. Of course, if the preference is evenly split, we can decide in favor of no default favorites.

@ryevdokimov
Copy link
Contributor

If most people setting up their editor will favorite these, then we should do it by default.

Agreed.

If/when this gets approved and merged, I would suggest a separate proposal and PR be made.

@tetrapod00
Copy link
Contributor

Also, to bikeshed the name, I do slightly prefer the old "Pinned Properties" name from the original proposal.

@Mickeon
Copy link
Contributor

Mickeon commented Sep 24, 2024

If we settle on "pinning" properties, the "Pin property" feature has to be renamed to something else. To be very fair, its name has been potentially misleading as is, already.

@tetrapod00
Copy link
Contributor

If we settle on "pinning" properties, the "Pin property" feature has to be renamed to something else. To be very fair, its name has been potentially misleading as is, already.

Oh, I thought that there was already something called "property pinning" but googling that led me to the original proposal for this new feature. "Pinning properties" definitely describes this feature more than the existing one. But changing the name of an existing feature is probably too much, just to have a slightly better name for the new feature.

@KoBeWi
Copy link
Member

KoBeWi commented Sep 25, 2024

It can be called Sticky Properties or something similar.

@YeldhamDev YeldhamDev force-pushed the i_love_all_my_properties_equally branch from 2344646 to f7bc7ce Compare September 25, 2024 03:07
@YeldhamDev
Copy link
Member Author

Alright, now properties are shown with their respective sections (always unfolded), to both give context, and prevent ones with the same name mixing up.
image

@pineapplemachine
Copy link

I foresee not finding properties in the expected place, because they are moved rather than duplicated, being a potential source of confusion and irritation for larger projects where you might not remember what properties were favorited and might have scrolled past the favorites category without noticing the property there.

At minimum I'd really like to see an editor toggle for whether favorited properties are moved or duplicated.

@KoBeWi
Copy link
Member

KoBeWi commented Oct 15, 2024

  • Favorite and Pin should be in the same separator group.
    image
  • I noticed Favorites sub-sections can't be folded. If it's not much problem, they should be foldable, unfolded by default.
  • Some weird bug:
godot.windows.editor.dev.x86_64_zSxpkGX2Dl.mp4

@KoBeWi
Copy link
Member

KoBeWi commented Oct 16, 2024

Patch that removes override menu:
menu_no_override.txt
The regular menu is created on demand. Makes the code simpler.

@YeldhamDev YeldhamDev force-pushed the i_love_all_my_properties_equally branch from af3a712 to 0e77a90 Compare October 16, 2024 15:59
@YeldhamDev
Copy link
Member Author

Separator removed and iterators fixed.

@YeldhamDev YeldhamDev force-pushed the i_love_all_my_properties_equally branch from 0e77a90 to cbe2ea6 Compare October 16, 2024 16:28
@YeldhamDev
Copy link
Member Author

@KoBeWi I added your patch to the code.

@Repiteo Repiteo removed the request for review from a team October 16, 2024 16:41
Copy link
Member

@KoBeWi KoBeWi 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.

After #97352 we can refer script favorites by UID instead of path, which would make the FileSystem Dock changes unnecessary.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected.

image

Note that this doesn't affect the remote inspector, which doesn't support favorite properties.

This makes me wonder if we should bring further attention to the Favorites section, e.g. by using a colored icon, or by adding a tinted background:

image

image

@YeldhamDev YeldhamDev force-pushed the i_love_all_my_properties_equally branch from cbe2ea6 to a828796 Compare October 28, 2024 23:39
@YeldhamDev YeldhamDev force-pushed the i_love_all_my_properties_equally branch from a828796 to d678b09 Compare October 29, 2024 00:18
Copy link
Member

@akien-mga akien-mga 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 to me. I've been expecting this kind of feature for a long time!

@akien-mga akien-mga modified the milestones: 4.x, 4.4 Nov 11, 2024
@Repiteo Repiteo merged commit ed3de25 into godotengine:master Nov 11, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 11, 2024

Thanks!

@nuclear
Copy link

nuclear commented Nov 22, 2024

Could there be an option to have the favorited items show up at the top and where they used to be? I already see myself looking for some aspects because I forgot they're at the top.

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.

Implement inspector "Property pinning"