diff --git a/ovos_workshop/skill_launcher.py b/ovos_workshop/skill_launcher.py index 726871a4..7da40e78 100644 --- a/ovos_workshop/skill_launcher.py +++ b/ovos_workshop/skill_launcher.py @@ -409,30 +409,29 @@ def _create_skill_instance(self, skill_module=None): """ skill_module = skill_module or self.skill_module try: - skill_creator = get_create_skill_function(skill_module) or \ - self.skill_class - - # create the skill - # if the signature supports skill_id and bus pass them - # to fully initialize the skill in 1 go + # in skill classes __new__ should fully create the skill object try: - # many skills do not expose this, if they don't allow bus/skill_id kwargs - # in __init__ we need to manually call _startup - self.instance = skill_creator(bus=self.bus, - skill_id=self.skill_id) - # skills will have bus and skill_id available as soon as they call super() - except: - self.instance = skill_creator() - - if hasattr(self.instance, "is_fully_initialized"): - LOG.warning(f"Deprecated skill signature! Skill class should be" - f" imported from `ovos_workshop.skills`") - is_initialized = self.instance.is_fully_initialized - else: - is_initialized = self.instance._is_fully_initialized - if not is_initialized: - # finish initialization of skill class + skill_class = get_skill_class(skill_module) + self.instance = skill_class(bus=self.bus, skill_id=self.skill_id) + except: # guess it wasnt subclassing from ovos_workshop (fail here ?) + + # attempt to use old style create_skill function entrypoint + skill_creator = get_create_skill_function(skill_module) or self.skill_class + + # if the signature supports skill_id and bus pass them to fully initialize the skill in 1 go + try: + # skills that do will have bus and skill_id available as soon as they call super() + self.instance = skill_creator(bus=self.bus, + skill_id=self.skill_id) + except: + # most old skills do not expose bus/skill_id kwargs + self.instance = skill_creator() + + # 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__ + if not self.instance._is_fully_initialized: self.instance._startup(self.bus, self.skill_id) + except Exception as e: LOG.exception(f'Skill __init__ failed with {e}') self.instance = None diff --git a/ovos_workshop/skills/base.py b/ovos_workshop/skills/base.py index c942acad..5d5c3909 100644 --- a/ovos_workshop/skills/base.py +++ b/ovos_workshop/skills/base.py @@ -170,11 +170,44 @@ class BaseSkill: """Base class for mycroft skills providing common behaviour and parameters to all Skill implementations. This base class does not require `mycroft` to be importable - Args: - name (str): skill name + skill_launcher.py used to be skill_loader-py in mycroft-core + + for launching skills one can use skill_launcher.py to run them standalone (eg, docker), + but the main objective is to make skills work more like proper python objects and allow usage of the class directly + + the considerations are: + + - most skills in the wild dont expose kwargs, so dont accept skill_id or bus + - most skills expect a loader class to set up the bus and skill_id after object creation + - skills can not do pythonic things in init, instead of doing things after super() devs are expected to use initialize() which is a mycroft invention and non-standard + - main concern is that anything depending on self.skill_id being set can not be used in init method (eg. self.settings and self.file_system) + - __new__ uncouples the skill init from a helper class, making skills work like regular python objects + - the magic in `__new__` is just so we dont break everything in the wild, since we cant start requiring skill_id and bus args + + KwArgs: + name (str): skill name - DEPRECATED + skill_id (str): unique skill identifier bus (MycroftWebsocketClient): Optional bus connection """ + def __new__(cls, *args, **kwargs): + if "skill_id" in kwargs and "bus" in kwargs: + skill_id = kwargs["skill_id"] + bus = kwargs["bus"] + try: + # skill follows latest best practices, accepts kwargs and does its own init + return super().__new__(cls, skill_id=skill_id, bus=bus) + except: + # skill did not update its init method, let's do some magic to init it manually + skill = super().__new__(cls, *args, **kwargs) + skill._startup(bus, skill_id) + return skill + + # skill loader was not used to create skill object, we are missing the kwargs + # skill wont be fully inited, please move logic to initialize + LOG.warning(f"{cls.__name__} not fully inited, self.bus and self.skill_id will only be available in self.initialize") + return super().__new__(cls) + def __init__(self, name=None, bus=None, resources_dir=None, settings: JsonStorage = None, gui=None, enable_settings_manager=True,