- Amsterdam
-
Asset system (development lead), UI (developer & module coordindator), VR (initial development).
- Joined on
2013-12-12 -
-
This should use a Python docstring, like in my snippet.
Instead of repeating asset_file_handle.asset_data every time, just declare an asset_data variable.
Would move this into the ASSETBROWSER_PT_metadata() class, to keep the context more clear. Outside of that class a name like add_metadata_prop() isn't very clear. (Also there's no point in @staticmethod outside of a class).
Let's not have functions with such big-ish if-else if-else blocks. Simply set a dedicated tooltip callback for assets, this can be much more concise then. All we need here is the asset representation I think, all the other variables/data just add noise.
Let's treat asset and file tooltips as separate patches. For assets things are straight forward, for files further discussion may be needed (like which information to display there, formatting, etc). I'm also not convinced the tooltip is the best place for this information, we could also display file information in the sidebar or in some extra region (like some other file browsers do).
While using "minimal" .blend reading, BLO_version_from_file() still has to open a .blend file from disk, which can't just be assumed to be entirely trivial. E.g. on network drives, displaying this tooltip could freeze Blender for seconds, since everything happens on the main thread. We could query the version on file list loading (which uses a thread).
Would add a function to avoid duplication, give further info in a comment, and to make clear that each conditional follows the same logic:
Again System defined is a bit vague. Would just add a /** See #AssetTrait. */ comment here.
Suggest: "Keywords describing type characteristics of an asset. Used to determine how an asset should behave, and to provide additional information and filtering options to the user"
Could you remove the ensure_traits_fn callbacks for now as noted in blender/blender#105808 (comment)? We don't need most of these traits yet, I rather add traits as needed, and don't set a whole bunch of unused traits just because they might be useful in future. (Although to a degree we may have to do that for forwards compatibility.)
Why would we store the UI description as trait? But again, I wouldn't even define the ensure_traits_fn anyway, and just let the nodes module decide how they want to use traits.