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

Fix missing tooltips caused by export_category #68641

Closed
wants to merge 1 commit into from

Conversation

garychia
Copy link
Contributor

@garychia garychia commented Nov 14, 2022

This pull request attempts to fix the issues mentioned by #64432 and #68520.

Problems found:

  1. The EditorInspector::update_tree function did not update the doc_name variable when skipping a category, which happened when show_categories was set to false and caused the following properties could not be identified (editor_inspector.cpp, line 2725).
  2. In the same function, the name of an export_category was treated as a class name as if it were a script.

Solutions:

  1. Find out the name of class a skipped category belongs to and update doc_name with it.
  2. Add the corresponding script path to hint_string of an export_category, which allows the function to find out the corresponding class.

Bugsquad edit:

@garychia garychia requested a review from a team as a code owner November 14, 2022 08:32
@Chaosus Chaosus added this to the 4.0 milestone Nov 14, 2022
@Atermnus
Copy link

Finally! Having custom resources with a lot of options and no documentation can create extremely verbose typing situations.

@YuriSizov
Copy link
Contributor

YuriSizov commented Nov 17, 2022

I don't like how hacky this whole doc_name things became, the logic is hard to follow. I feel like we should have some metadata for each property instead, which points to which class this property originates from. The way it works now is very much order dependent and, yes, easy to break when users starts to mess with custom categories.

PS. But, for what it's worth, this particular PR doesn't do anything we don't already do, and from the looks of it it should fix the issue (not tested yet).

@garychia
Copy link
Contributor Author

garychia commented Nov 17, 2022

I agree. I think my solution may not be complete although it solved the issues on my machine. There might be some bugs I did not found. I believe the update_tree function should be refactored in the future. Otherwise, it won't be easy to debug.
By the way, it seems that doc_info_cache is not updated when the doc is changed. But I'm not sure.

@akien-mga akien-mga modified the milestones: 4.0, 4.1 Feb 14, 2023
@akien-mga akien-mga modified the milestones: 4.1, 4.2 Jun 19, 2023
@akien-mga
Copy link
Member

Thanks for the contribution nonetheless!

@akien-mga akien-mga closed this Sep 30, 2023
@AThousandShips AThousandShips removed this from the 4.2 milestone Oct 1, 2023
@garychia garychia deleted the fix_inspector_tooltip branch October 1, 2023 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants