comfyanonymous / ComfyUI

The most powerful and modular diffusion model GUI, api and backend with a graph/nodes interface.
https://www.comfy.org/
GNU General Public License v3.0
51.45k stars 5.41k forks source link

[Proposal] Unified Plugin System #632

Open guill opened 1 year ago

guill commented 1 year ago

Goals

Today, there are several different systems for extending ComfyUI, principally:

  1. Custom nodes implemented in Python in the custom_nodes folder
  2. Custom javascript code in the web/extensions folder

In many cases, a single extension will require both of these. For example, Davemane42's popular MultiAreaConditioning nodes require that files be inserted into both folders and be kept in sync. This makes installing and uninstalling the extension both difficult and error-prone.

I'd like to propose a unified plugin system that has the following goals:

  1. Allow for plugins to be installed by simply dropping a folder into the plugins folder (or checking out a git repository there)
  2. Allow for the future addition of additional extension points by both ComfyUI itself and other related tools (e.g. ComfyBox implementations of custom UIs)
  3. Enable the creation of a plugin manager that can be used to install, uninstall, and update plugins without hard-coding all plugin information in the manager's repository.
  4. Still allow for single-file plugins for the simple case (either a single python file or a single javascript file)

Proposal

Manifest

At this point, I think it makes sense to create a manifest file to describe a plugin. This manifest would simply be a file named manifest.json in the root direcotry of the plugin. For the truly minimal case, only a name for the plugin would be required. (This would be used to solve conflicting node names as described in Issue 630):

{
    "name": "Masquerade Nodes"
}

A full manifest might look like this:

{
    "name": "Masquerade Nodes",
    "version": "1.0.0",
    "description": "Adds nodes related to masks",
    "author": "BadCafeCode",
    "update": "https://github.com/BadCafeCode/masquerade-nodes-comfyui.git",
    "extensionPoints": {
        "nodes": ".",
        "web": "./web_extension"
    }
}

Fields

Recommended Plugin Structure

While the manifest above is fairly flexible in terms of the structure of the plugin, I think it would be good to have a recommended 'best practice' that we encourage people to use. Honestly, I don't think it matters what that recommendation is as long as it's clear and consistent. My first inclination would be the following structure:

plugins/
    my_plugin/
        manifest.json
        nodes/
            __init__.py
            my_plugin.py
        web/
            my_plugin.js
            utils.js

Single File Plugins

@rvion brought up some good arguments for continuing to support single-line plugins, namely:

  1. Self-contained examples to get people started
  2. The ability to generate plugins via something like ChatGPT easily

To support those use-cases, any .py files directly in the plugins directory would be treated as a plugin with a single extension point ("nodes": "<filename.py>") and any .js files directly in the plugins directory would be treated as a plugin with a single extension point ("web": "<filename.js>"). Additional manifest information could be provided in a comment at the top of the file (starting within the first 3 lines):

Python example:

'''
Manifest = {
    "name": "My Standalone Plugin",
    "version": "1.0.0",
}
'''

Javascript example:

/*
Manifest = {
    "name": "My Standalone Plugin",
    "version": "1.0.0",
}
*/

I don't think it's particularly important that single-file plugins support all functionality that normal plugins support (e.g. multiple extension points).

Workflow Save Changes

Along with this change, I'd like to see saved workflows (as well as those embedded in images) include the following information for used nodes so that the plugin manager can install any missing plugins with one click when importing a workflow:

{
    "plugins": [
        {
            "name": "Masquerade Nodes",
            "version": "0.0.9",
            "update": "https://github.com/BadCafeCode/masquerade-nodes-comfyui.git",
        },
        {
            "name": "WAS Node Suite",
            "version": "1.0.0",
            "update": "https://github.com/WASasquatch/was-node-suite-comfyui.git",
        }
    ]
}

Outstanding Questions

  1. Should dependencies be handled in the manifest? My first inclination is no since there are so many forms dependencies can take (installing Python packages, downloading models, etc.)
  2. Should the list of provided nodes be defined in the manifest? This seems like a pain to maintain, but I think it would be really cool for ComfyUI-Manager to be able to add non-installed nodes to the node list with the ability to auto-install them when selected. For now I would lean towards not doing this, but it might be worth adding (as an optional field) in the future.
  3. Should we be more prescriptive about the structure of the plugin? The current proposal allows the layout to be arbitrary as long as it's specified in the manifest. This makes it easier for existing plugins to migrate to the new system (since all they have to do is add a manifest.json), but it might be better in the long-term to have a pre-defined layout.
  4. Should ComfyUI have a plugin manager built-in? Probably not needed for the first implementation of this, but I do think it's something that should be considered eventually.
ltdrdata commented 1 year ago
guill commented 1 year ago
  1. tags: This information can be used for convenience functions to enable tag-based searches.

Good call, this could definitely be useful.

  1. node list: This information can be used to build a database that can automatically locate missing nodes when a workflow loaded by an uninstalled extension displays them.

One possibility would be having ComfyUI automatically generate a nodes.json file in plugin directories on load. That wouldn't require either prescriptivist coding styles or manual maintenance. (Though people would have to check this auto-generated file into git for it to be available without installing the plugin.)

  1. dependent resources: This can help manage the models or additional resources that come with an extension automatically when installing it.

I have mixed feelings on this. I kind of like that most models are right now downloaded only once they're needed (rather than when the extension is installed). If I want some nodes in a node pack but don't plan to use others, I'd prefer to only download the models that I actually need.

My preference would be adding some helper functions available to plugins for downloading models so that each plugin doesn't have to implement that on its own.

  1. dependent extensions: In the future, dependencies may arise between extensions. It would be helpful to have information about this.

For this to work, we would need a way to actually reference other extensions (assuming we don't want to rely on relative directory names). While I think it could be super useful, I also think it's something that doesn't necessarily need to be in the first implementation. I'd prefer to wait and see what sorts of dependencies actually exist before designing this out.

WASasquatch commented 1 year ago

The issue I immediately see, is these are two different systems actually independent of each other. The front end-for example, relies heavily on 3rd party development from litegraph.js. There could be changes to one or the other that break the entire thing that then all developers of tools would need to be aware of.

A lot of these edits by other node systems for extensions are iffy, from what I've seen to begin with, more hacks, too, and have already broke in the past like Preprocessor nodes that were broken for a good while with no support.

Custom nodes really should be their own thing, or rely on official support form ComfyUI official functionality, not try to hack in their own bits and bobs that could break things for other mods/people.

guill commented 1 year ago

I'm not sure I totally understand @WASasquatch. Are you opposed to the existing state of .js extensions (and encouraging their creation via official support in a plugin system)? Or is there something about the plugin architecture that would make things worse than they are today?

I'll admit I haven't done a lot of work with the .js side of things, so my ideas are less well-formed there.

WASasquatch commented 1 year ago

Oh I am just wondering how mature litegraph.js and the whole front end that might see issues that would effect a node. I've already seen it with nodes relying on extensions for their operation.

ltdrdata commented 1 year ago

We need unique ID for extension, too.

A unique ID could be used to verify installation or prevent conflicts.

WASasquatch commented 1 year ago

We need unique ID for extension, too.

A unique ID could be used to verify installation or prevent conflicts.

I like the idea of using the author in places of a UUID type system, like proposed here, and in another topic I can't seem to find on my phone: https://github.com/comfyanonymous/ComfyUI/issues/630#issue-1698912807

space-nuko commented 1 year ago

I'm strongly against bundling frontend code with extensions since I don't think the frontend and backend code should be required to be loaded together. The fronted extension system assumes you're using ComfyUI's frontend and not an alternative one that might not use vanilla JS or even the same architecture.

I'm in agreement however that there really needs to be namespacing for backend nodes to avoid node ID collisions. That can be defined entirely in Python however.

I also want the database so you can easily find and download missing nodes. However a prerequisite of that feature is the assurance that all node names will be unique per extension uploaded to the database, meaning the namespacing would already be in place.

WASasquatch commented 1 year ago

I'm strongly against bundling frontend code with extensions since I don't think the frontend and backend code should be required to be loaded together. The fronted extension system assumes you're using ComfyUI's frontend and not an alternative one that might not use vanilla JS or even the same architecture.

I'm in agreement however that there really needs to be namespacing for backend nodes to avoid node ID collisions. That can be defined entirely in Python however.

I also want the database so you can easily find and download missing nodes. However a prerequisite of that feature is the assurance that all node names will be unique per extension uploaded to the database, meaning the namespacing would already be in place.

As cool as your project is, ComfyUI shouldn't be built around just replacing it, that would be disadvantageous for ComfyUI... It's modular-ability is through Extensions and Customs Nodes.

In the space of your ComfyBox, it should be a fork of ComyUI and you should, unfortunately, just manage the merges (meaning resolving conflicts) of new features downstream from ComfyUI to your fork of it, to support ComfyBox.

guill commented 1 year ago

I actually think this architecture would help ComfyBox support extensions like this. For better or worse, there are nodes that require front-end extensions. I don't think we're going to be able to get around the fact that people are going to want capabilities that just aren't available in the default repo. The ability for custom extension points means that a plugin could implement both a web front-end extension and a comfybox front-end extension (assuming ComfyBox wants to support that). I agree that as much as possible node back-ends shouldn't be dependent on a specific front-end, but I don't think that's the same as saying that they aren't related to front-end changes.

WASasquatch commented 1 year ago

I mean, you just can't get unified extensions with Python and Javascript. They're all based on websockets and APIs etc.

If extensions (web side) are loaded last, like they should be, they should be able to override and extend anything, including any ComfyUI logic or litegraph.js

If this was how the system worked, ComfyBox wouldn't need to be anything but a drop-in extension to ComfyUI.