Editor: Fix AssetLib is not updating size and stuck #86051
Conversation
|
Forgot to run clang-format. 🤦♂️ Just amended with formatting fixed. |
|
Okay, this is strange. This was working before rebasing over latest master, but now it's not recalculating every time I resize the editor. I'm going to mark this as a draft and I will request review after I have taken a closer look at this. |
|
Okay, this should be ready for review now. The problem was that the I got this fixed and amended my commit. I also updated the commit message with the details of the edits I made to the Please let me know if you have any questions about the implementation. Thanks! |
Calinou
added
topic:assetlib
cherrypick:4.2
|
@GrammAcc, It looks like it'll fix my issue on Linux using a tile manager. I'll keep you posted when it gets merged and 4.3 comes out. Thanks for putting in the work. |
|
I realized this morning that there is a better way to ensure that we don't overwrite the tooltip text with the truncated text. I currently have an if check for it in the Also, I am working through the Accelerated C++ book in order to improve my contribution efficiency and quality, so hopefully my future PRs will be easier to review. :) |
This is an enhancement to the fix originally provided in #80555. When I submitted the original PR I did not account for resizing the editor window at runtime, so the number of columns for asset lib search results was recalculating on resize, but the asset title truncation threshold was not being recalculated, so columns were not spacing correctly after resizing the editor. I added a simple for loop to the NOTIFICATION_RESIZE event of the EditorAssetLibrary class that loops through the currently displayed assets and recalculates their title truncation length. I just used a for loop over the `asset_items->get_child_count` and fetched each EditorAssetLibraryItem with `asset_items->get_child` since I wanted to avoid explicit memory allocation inside the notification handler. I also modified the `EditorAssetLibraryItem::clamp_width` method that I originally added in #80555 to fix a couple of bugs: - When the method was called during a resize, it would set the tooltip text to the current value of the title's truncated text, so we lost the full text tooltip on resize if the title's text was truncated previously. - If resizing to a smaller column width that caused truncation of the title text, then resizing to a larger width that did not need truncation, the title text would not reset to the full text, so we ended up with truncated titles when we had the space for the full text. I changed this to set the tooltip text in the `EditorAssetLibraryItem::configure` method, so it isn't updated when resizing the editor. Since the tooltip text is now guaranteed to be the full title text, I used it in the `clamp_width` method to determine the size of the text for clamping the width. I was unable to reproduce the specific scaling problems reported in #84471 (comment), and I think that specific problem may be Windows-specific. However, I am unable to confirm this since I do not have a Windows machine to test on, and I could not get Godot to run inside a virtual machine. I think that this commit will likely fix the issue as reported in #84471 since it forces recalculation of the truncation threshold on NOTIFICATION_RESIZE, but I can't confirm without being able to reproduce that issue on my machine. In any case, this commit fixes a UX problem with the previous fix provided in #80555 by ensuring consistent UI scaling and layout when resizing the editor at runtime, so I think it is worth adding to the engine independent of any specific issue.
|
I updated the PR with a better implementation. Details are in the commit message. I removed the full_title_text private member of the EditorAssetLibraryItem that I had originally added and used the tooltip text as the full text instead. By moving the |
|
I'm closing this since #88761 was merged and seems to solve the same problem. |
akien-mga
added
archived
and removed
cherrypick:4.2
This is an enhancement to the fix originally provided in #80555.
When I submitted the original PR I did not account for resizing the editor window at runtime, so the number of columns for asset lib search results was recalculating on resize, but the asset title truncation threshold was not being recalculated, so columns were not spacing correctly after resizing the editor.
I added a simple for loop to the NOTIFICATION_RESIZE event of the EditorAssetLibrary class that loops through the currently displayed assets and recalculates their title truncation length. I just used a for loop over the
asset_items->get_child_countand fetched each EditorAssetLibraryItem withasset_items->get_childsince I wanted to avoid explicit memory allocation inside the notification handler.Please let me know if this is not a good solution or if the use of
static_castis discouraged.I tested this solution locally, and it fixes the problems I mentioned in #84471 (comment).
I think this will fix the scaling problem as reported in #84471 (comment) since it forces recalculation of the asset titles, but I am unable to confirm for sure since I can't test this on Windows.
This PR doesn't resolve the TODO:
https://github.com/godotengine/godot/blob/15a03ed98e63590c85dac62405d785e1b7ca7fed/editor/plugins/asset_library_editor_plugin.cpp#L67C1-L67C53
I haven't figured out exactly how to get the font size with TextServer yet, so I will submit a separate PR for that small refactor eventually. I wanted to submit this fix now though since this fixes an actual flaw in the original.