Repair broken loader, since #388

[mottosso]

Loader broke with #388, this fixes that.

However, there's reference and initialization to global state happening from within the GUI; that isn't nice. Initialisation should happen during api.install(), and all state should reside in avalon/__init__.py. Leaving this for now as technical debt.

[BigRoy]

Note that the initialization happens during UI as it requires an active Qt Application instance to exist which does not have to be true during api.install () as it could run command-line only without graphical interfaces.

[davidlatwe]

Note that the initialization happens during UI as it requires an active Qt Application instance to exist

Hmmm, I think those two, refresh_family_config and refresh_group_config does not require an App instance to be existed, they were just updating icons.

[mottosso]

It's looking like they create actual instances of Qt object.

https://github.com/getavalon/core/blob/48823f4d6369b9648e74709d2f63d69b89a8bcf8/avalon/tools/loader/lib.py#L96

That should probably happen within the GUI itself, rather than in conjunction with making database queries.

[davidlatwe]

I just scanned the differences between before & after #388(#418), and there's a bit more broken stuff. The avalon.tools.loader.app has lost lots of things which was from #391. And looks like only app.py has been affected, others does not.

[BigRoy]

It's looking like they create actual instances of Qt object.

It creates actual Qt icons, yes. That's why it needs a QApplication instance.

This was also one of those things that I believe is referenced across multiple tools and it's to optimize the loading time of the icons. You wouldn't want do to that in the .data() method of the models as those would then be recreated very very often.

Basically it's persisting the icons for reuse.

That should probably happen within the GUI itself, rather than in conjunction with making database queries.

I believe this is actually used across multiple tools/widgets/models, but not too sure. I think this was also one of the reasons we wanted to refactor gui and things. To ensure the cross-referencing across tools would be resolved.

Referencing #427

[mottosso]

The avalon.tools.loader.app has lost lots of things which was from #391

Oh that's not good. I wonder if everything was lost from that PR? It's possible that when merging 388 that the wrong app.py was picked; merging an older one instead of the current one. Having a look at this now. Which reminds me we need some way of launching tools without a host for debugging and development, something like python -m pyblish_qml --demo. Something to stress test every supported function.

You wouldn't want do to that in the .data() method of the models as those would then be recreated very very often.

No that's true, but there may be a better way to solve this, whilst at the same time making it clear that it is an optimisation rather than functional feature; by using a cache.

def data(self, index, parent):
  ...
  if role == DecorationRole:
    try:
      return self._icon_cache[index]
    except KeyError:
      icon = heavy_compute_of_qicon()
      self._icon_cache[index] = icon
      return icon

[BigRoy]

by using a cache.

This was exactly that. A cache for re-use, but then across multiple tools. Feel free to improve and submit a PR

[davidlatwe]

It creates actual Qt icons, yes. That's why it needs a QApplication instance.

@BigRoy Ah, yes. Sorry, did not know that, thought only widgets were required. :P

I wonder if everything was lost from that PR?

@mottosso Others are fine, just checked with branch comparing. :)

[mottosso]

This was exactly that. A cache for re-use, but then across multiple tools. Feel free to improve and submit a PR

Yes, no sorry. I mean an explicit cache; something others can see "ah, that's an optimisation". There is currently no indication that these refresh_ functions do anything like that, and certainly should not be throwing errors if they fail. That's not a cache, that's magic.