- Amsterdam
-
Asset system (development lead), UI (developer & module coordindator), VR (initial development).
- Joined on
2013-12-12 -
-
Agree that it's fishy, but don't think we can do much about it - or that it's worth it at least. Essentially I'd want a std::weak_ptr::get().
There are plenty of places where we get the asset…
I'd prefer if the asset library is designed as the sole owner. It just makes more sense conceptually, but also because plenty of asset system functions require the asset to be registered in a…
Would be good to avoid iterating over every node here, it looks like this is called twice on every redraw. Once for the header, once for the main region. I doubt this code path would be much of a bottle neck in practice though, and this seems to be the only simple way to do this for now.
Have you considered using ui::BasicTreeViewItem as base class? It can help avoid some bolierplate, by default it just adds a label with an optional icon.
Although the extra level of…
@JacquesLucke would you mind checking the C++ side of things, mainly appropriate use of std::weak_ptr/std::shared_ptr? I guess that would help Bastien review if this is a good way to go about it.
Not super thrilled about updating things just since it's nice to have a newer version, if it solves an actual bug, or resolves a security issue it's a different story though, however the last…
From what I can tell the "currently this is a runtime flag" part is wrong too.
Would recommend just removing the function and keeping the logic in place. Keeping it in a function hides more information than it's useful.
Code wise looking fine, but I'll leave up the decision to remove this in 4.3 to the animation & rigging module.
This function is now always called since it's on both branches of the if (region == nullptr) check.
You figured it out correctly :)
I think instead of completely freeing the preview, it's better to just clear it so the icon-id won't change. So we need a way to clear the preview with the…