fix/loading_failure_logs_even_if_loaded (#168)

* fix/loading_failure_logs_even_if_loaded

the skill launcher would report that plugin skills failed to load and still load them, the backwards compat log was not in the correct place

* fix/fallbackSkill init

was hiding by previous loading error

ovos_workshop/skill_launcher.py

@@ -433,23 +433,27 @@ def _create_skill_instance(self,
             (bool): True if skill was loaded successfully.
         """
         skill_module = skill_module or self.skill_module
+        skill_creator = None
+        if skill_module:
+            try:
+                # in skill classes __new__ should fully create the skill object
+                skill_class = get_skill_class(skill_module)
+                self.instance = skill_class(bus=self.bus, skill_id=self.skill_id)
+                return self.instance is not None
+            except Exception as e:
+                LOG.warning(f"Skill load raised exception: {e}")
 
-        try:
-            # in skill classes __new__ should fully create the skill object
-            skill_class = get_skill_class(skill_module)
-            self.instance = skill_class(bus=self.bus, skill_id=self.skill_id)
-            return self.instance is not None
-        except Exception as e:
-            LOG.warning(f"Skill load raised exception: {e}")
+            try:
+                # attempt to use old style create_skill function entrypoint
+                skill_creator = get_create_skill_function(skill_module) or \
+                    self.skill_class
+            except Exception as e:
+                LOG.exception(f"Failed to load skill creator: {e}")
+                self.instance = None
+                return False
 
-        try:
-            # attempt to use old style create_skill function entrypoint
-            skill_creator = get_create_skill_function(skill_module) or \
-                self.skill_class
-        except Exception as e:
-            LOG.exception(f"Failed to load skill creator: {e}")
-            self.instance = None
-            return False
+        if not skill_creator and self.skill_class:
+            skill_creator = self.skill_class
 
         # if the signature supports skill_id and bus pass them
         # to fully initialize the skill in 1 go
@@ -463,19 +467,19 @@ def _create_skill_instance(self,
             LOG.warning(f"Legacy skill: {e}")
             self.instance = skill_creator()
 
-        try:
-            # finish initialization of skill if we didn't manage to inject
-            # skill_id and bus kwargs.
-            # these skills only have skill_id and bus available in initialize,
-            # not in __init__
-            log_deprecation("This initialization is deprecated. Update skill to"
-                            "handle passed `skill_id` and `bus` kwargs",
-                            "0.1.0")
-            if not self.instance.is_fully_initialized:
+        if not self.instance.is_fully_initialized:
+            try:
+                # finish initialization of skill if we didn't manage to inject
+                # skill_id and bus kwargs.
+                # these skills only have skill_id and bus available in initialize,
+                # not in __init__
+                log_deprecation("This initialization is deprecated. Update skill to"
+                                "handle passed `skill_id` and `bus` kwargs",
+                                "0.1.0")
                 self.instance._startup(self.bus, self.skill_id)
-        except Exception as e:
-            LOG.exception(f'Skill __init__ failed with {e}')
-            self.instance = None
+            except Exception as e:
+                LOG.exception(f'Skill __init__ failed with {e}')
+                self.instance = None
 
         return self.instance is not None
 
@@ -515,7 +519,7 @@ def load(self, skill_class: Optional[callable] = None) -> bool:
         LOG.info('ATTEMPTING TO LOAD PLUGIN SKILL: ' + self.skill_id)
         self._skill_class = skill_class or self._skill_class
         if not self._skill_class:
-            raise RuntimeError(f"_skill_class not defined for {self.skill_id}")
+            raise RuntimeError(f"skill_class not defined for {self.skill_id}")
         return self._load()
 
     def _load(self):

ovos_workshop/skills/fallback.py

@@ -68,7 +68,7 @@ def __new__classic__(cls, *args, **kwargs):
             # return V2 as expected, V1 will eventually be dropped
             return FallbackSkillV2(*args, **kwargs)
         cls.__bases__ = (FallbackSkillV1, FallbackSkill, _MetaFB)
-        return super().__new__(cls, *args, **kwargs)
+        return super().__new__(cls)
 
     @backwards_compat(classic_core=__new__classic__,
                       pre_008=__new__classic__)
@@ -78,7 +78,7 @@ def __new__(cls, *args, **kwargs):
             # return V2 as expected, V1 will eventually be dropped
             return FallbackSkillV2(*args, **kwargs)
         cls.__bases__ = (FallbackSkillV2, FallbackSkill, _MetaFB)
-        return super().__new__(cls, *args, **kwargs)
+        return super().__new__(cls)
 
     @classmethod
     def make_intent_failure_handler(cls, bus: MessageBusClient):