[BUG] Plugins: Decky Loader doesn't instantiate and call plugin classes correctly

[theCapypara]

Please confirm

• I have searched existing issues
• This issue is not a duplicate of an existing one
• I have checked the common issues section in the readme file
• I have attached logs to this bug report (failure to include logs will mean your issue will not be responded too).

Bug Report Description

Plugins ship with a main.py that define a single Python class Plugin.

At the moment Decky doesn't actually instantiate that class, it just calls _main and _unload with the class itself as self, which is incorrect and leads to bugs and confusion when trying to interact with self inside Plugin. Most notably simple method calls like self.method() fail, complaining about self not being an instance of the object. The workaround for this is Plugin.method(self), but this is pretty jank.

See:

decky-loader/backend/plugin.py

Line 90 in 6d086fb

get_event_loop().create_task(self.Plugin._main(self.Plugin))

Expected Behaviour

Decky should just instantiate the Plugin class normally and call methods on it like on any other standard Python object. Alternatively, _main and _unload (and possibly any plugin method called by the frontend?) should be marked properly with @classmethod

SteamOS version

n/a

Selected Update Channel

Stable

Have you modified the read-only filesystem at any point?

n/a

Logs

n/a

[theCapypara]

Some thoughts I had on this on Discord (conversation starting at https://canary.discord.com/channels/960281551428522045/960284327445418044/1131286537145958410):

even if there's a reason instances of Plugin should not be created, then @classmethod would be a better solution. if that was the way to go it would probably be possible to check if _main etc. are wrapped in @classmethod and if not just fall back to the old way
Python is pretty flexible here (probably too much so, see this issue even existing ahaha)
I could imagine the reason for it being done this way is so that Decky doesn't need to also keep track of all the plugin instances?
though i guess it needs to kinda do that anyway, otherwise how would it know what plugin is even loaded
A simple solution for a V2 could be to just utilize the standard python __init__ and __del__ instead
That could also be backwards compatible, by just falling back to v1 if either _main or _unload exist

[XanderXAJ]

I've also come across this and I can confirm it is confusing. 😄 The current Plugin "class" is treated more like a module/namespace, so it's confusing when self doesn't always work as expected. I think it works for lifecycle methods and API requests because it's manually passed in, but breaks as soon as you call your own methods from them.

I'm currently working around it by making a separate class and instantiating that myself, treating Plugin like an API layer and moving all the "business logic" to the other class. This code is probably easier to understand and test, albeit it's also more verbose. Still, it doesn't take away from how confusing the Plugin's self can be!

Possible fixes

If we're speculating on designs that would allow proper instantiation while remaining compatible, I'd consider adding a new static/class method where the plugin developer can return an instance of the plugin. This doesn't replace the existing _main etc. lifecycle methods.

Allowing the plugin developer to return an instance allows configuring the Plugin with parameters and provides inversion of control, making the Plugin code more easily testable.

For example:

class Plugin:
    @staticmethod
    def instance() -> Plugin:
        # Plugin developer can grab configs needed for instantiating the plugin class here
        return Plugin()

    def __init__(self) -> None:
        # Plugin developer can perform whatever fast initialisation they need to here -- _main still exists for slower initialisation using asyncio
        pass

Decky Loader could pass in configuration to the instance() method, once again to allow the Plugin to be configured up front and make it more testable -- although the plugin author can pull that config out of decky_plugin to the same end.

Here are quick examples of all the configuration possibilities I've mentioned:

# Loader could pass in lots of parameters...
class Plugin:
    @staticmethod
    def instance(plugin_path, bin_path, ...) -> Plugin:
        # Plugin developer can grab configs needed for instantiating the plugin class here
        return Plugin(plugin_path=plugin_path)



# ... But it might be more future proof to have a data class instead:
@dataclass
class PluginConfig:
    plugin_dir: Path
    runtime_dir: Path
    ... # Feel free to add more later -- it'll be backwards-compatible manner

class Plugin:
    @staticmethod
    def instance(config: PluginConfig) -> Plugin:
        # Plugin developer can grab configs needed for instantiating the plugin class here
        return Plugin(plugin_config=config)



# ... Or decky_plugin can continue to be used:
class Plugin:
    @staticmethod
    def instance() -> Plugin:
        # Plugin developer can grab configs needed for instantiating the plugin class here
        return Plugin(
            settings_dir=decky_plugin.DECKY_PLUGIN_SETTINGS_DIR,
            ...
        )

When it comes to loading the plugin, potentially this could be as simple as detecting the presence of instance and using it if available:

decky-loader/backend/plugin.py

Line 85 in 6d086fb

self.Plugin = module.Plugin

self.Plugin = module.Plugin
if hasattr(self.Plugin, "instance") and callable(self.Plugin.instance):
    self.Plugin = self.Plugin.instance(...)

However I can see a lot of manual passing of self.Plugin to Plugin methods (perhaps because of the lack of instantiation?), so it may not be quite as simple. Still, hopefully the idea comes across clearly.

[AAGaming00]

fixed on websockets branch