Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix group node naming compatibility #969

Merged
merged 3 commits into from
Sep 25, 2024
Merged

Fix group node naming compatibility #969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b85b3a1
Convery legacy group node names in workflow
christian-byrne Sep 25, 2024
3ed545c
Add Playwright test
christian-byrne Sep 25, 2024
acae715
Remove hardcoded strings
christian-byrne Sep 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
252 changes: 252 additions & 0 deletions browser_tests/assets/legacy_group_node.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,252 @@
{
"last_node_id": 15,
"last_link_id": 9,
"nodes": [
{
"id": 15,
"type": "workflow/hello",
"pos": {
"0": 566,
"1": 316
},
"size": {
"0": 468.5999755859375,
"1": 582
},
"flags": {},
"order": 0,
"mode": 0,
"inputs": [
{
"name": "model",
"type": "MODEL",
"link": null,
"label": "model"
},
{
"name": "positive",
"type": "CONDITIONING",
"link": null,
"label": "positive"
},
{
"name": "negative",
"type": "CONDITIONING",
"link": null,
"label": "negative"
},
{
"name": "latent_image",
"type": "LATENT",
"link": null,
"label": "latent_image"
},
{
"name": "KSampler model",
"type": "MODEL",
"link": null,
"label": "KSampler model"
},
{
"name": "KSampler positive",
"type": "CONDITIONING",
"link": null,
"label": "KSampler positive"
},
{
"name": "KSampler negative",
"type": "CONDITIONING",
"link": null,
"label": "KSampler negative"
},
{
"name": "KSampler latent_image",
"type": "LATENT",
"link": null,
"label": "KSampler latent_image"
}
],
"outputs": [
{
"name": "LATENT",
"type": "LATENT",
"links": null,
"shape": 3,
"label": "LATENT"
},
{
"name": "KSampler LATENT",
"type": "LATENT",
"links": null,
"shape": 3,
"label": "KSampler LATENT"
}
],
"properties": {
"Node name for S&R": "workflow/hello"
},
"widgets_values": [
"enable",
0,
"randomize",
20,
8,
"euler",
"normal",
0,
10000,
"disable",
0,
"randomize",
20,
8,
"euler",
"normal",
1
]
}
],
"links": [],
"groups": [],
"config": {},
"extra": {
"groupNodes": {
"hello": {
"nodes": [
{
"id": -1,
"type": "KSamplerAdvanced",
"pos": {
"0": 351.3332824707031,
"1": 577.3333129882812
},
"size": {
"0": 315,
"1": 334
},
"flags": {},
"order": 0,
"mode": 0,
"inputs": [
{
"name": "model",
"type": "MODEL",
"link": null,
"label": "model"
},
{
"name": "positive",
"type": "CONDITIONING",
"link": null,
"label": "positive"
},
{
"name": "negative",
"type": "CONDITIONING",
"link": null,
"label": "negative"
},
{
"name": "latent_image",
"type": "LATENT",
"link": null,
"label": "latent_image"
}
],
"outputs": [
{
"name": "LATENT",
"type": "LATENT",
"links": null,
"shape": 3,
"label": "LATENT"
}
],
"properties": {
"Node name for S&R": "KSamplerAdvanced"
},
"widgets_values": [
"enable",
0,
"randomize",
20,
8,
"euler",
"normal",
0,
10000,
"disable"
],
"index": 0
},
{
"id": -1,
"type": "KSampler",
"pos": {
"0": 636,
"1": 427
},
"size": {
"0": 315,
"1": 262
},
"flags": {},
"order": 1,
"mode": 0,
"inputs": [
{
"name": "model",
"type": "MODEL",
"link": null,
"label": "model"
},
{
"name": "positive",
"type": "CONDITIONING",
"link": null,
"label": "positive"
},
{
"name": "negative",
"type": "CONDITIONING",
"link": null,
"label": "negative"
},
{
"name": "latent_image",
"type": "LATENT",
"link": null,
"label": "latent_image"
}
],
"outputs": [
{
"name": "LATENT",
"type": "LATENT",
"links": null,
"shape": 3,
"label": "LATENT"
}
],
"properties": {
"Node name for S&R": "KSampler"
},
"widgets_values": [
0,
"randomize",
20,
8,
"euler",
"normal",
1
],
"index": 1
}
],
"links": [],
"external": []
}
}
},
"version": 0.4
}
8 changes: 8 additions & 0 deletions browser_tests/groupNode.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,12 @@ test.describe('Group Node', () => {
// Ensure the link is still present
expect(await input.getLinkCount()).toBe(1)
})

test('Loads from a workflow using the legacy path separator ("/")', async ({
comfyPage
}) => {
await comfyPage.loadWorkflow('legacy_group_node')
expect(await comfyPage.getGraphNodesCount()).toBe(1)
expect(comfyPage.page.locator('.comfy-missing-nodes')).not.toBeVisible()
})
})
38 changes: 28 additions & 10 deletions src/extensions/core/groupNode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,23 @@ import { ManageGroupDialog } from './groupNodeManage'
import type { LGraphNode } from '@comfyorg/litegraph'
import { LGraphCanvas, LiteGraph } from '@comfyorg/litegraph'
import { useNodeDefStore } from '@/stores/nodeDefStore'
import { ComfyNode, ComfyWorkflowJSON } from '@/types/comfyWorkflow'

const GROUP = Symbol()

// v1 Prefix + Separator: workflow/
// v2 Prefix + Separator: workflow> (ComfyUI_frontend v1.2.63)
const PREFIX = 'workflow'
const SEPARATOR = '>'

const Workflow = {
InUse: {
Free: 0,
Registered: 1,
InWorkflow: 2
},
isInUseGroupNode(name) {
const id = `workflow>${name}`
const id = `${PREFIX}${SEPARATOR}${name}`
// Check if lready registered/in use in this workflow
if (app.graph.extra?.groupNodes?.[name]) {
if (app.graph.nodes.find((n) => n.type === id)) {
Expand Down Expand Up @@ -185,15 +191,15 @@ export class GroupNodeConfig {
this.outputVisibility = []
}

async registerType(source = 'workflow') {
async registerType(source = PREFIX) {
this.nodeDef = {
output: [],
output_name: [],
output_is_list: [],
output_is_hidden: [],
name: source + '>' + this.name,
name: source + SEPARATOR + this.name,
display_name: this.name,
category: 'group nodes' + ('>' + source),
category: 'group nodes' + (SEPARATOR + source),
input: { required: {} },
description: `Group node combining ${this.nodeData.nodes
.map((n) => n.type)
Expand All @@ -216,7 +222,7 @@ export class GroupNodeConfig {
p()
}
this.#convertedToProcess = null
await app.registerNodeDef('workflow>' + this.name, this.nodeDef)
await app.registerNodeDef(`${PREFIX}${SEPARATOR}` + this.name, this.nodeDef)
useNodeDefStore().addNodeDef(this.nodeDef)
}

Expand Down Expand Up @@ -647,7 +653,7 @@ export class GroupNodeConfig {
app.clean = function () {
for (const g in groupNodes) {
try {
LiteGraph.unregisterNodeType('workflow/' + g)
LiteGraph.unregisterNodeType(`${PREFIX}${SEPARATOR}` + g)
} catch (error) {}
}
app.clean = clean
Expand All @@ -662,11 +668,11 @@ export class GroupNodeConfig {
if (!(n.type in LiteGraph.registered_node_types)) {
missingNodeTypes.push({
type: n.type,
hint: ` (In group node 'workflow/${g}')`
hint: ` (In group node '${PREFIX}${SEPARATOR}${g}')`
})

missingNodeTypes.push({
type: 'workflow/' + g,
type: `${PREFIX}${SEPARATOR}` + g,
action: {
text: 'Remove from workflow',
callback: (e) => {
Expand Down Expand Up @@ -1380,7 +1386,7 @@ export class GroupNodeHandler {
const config = new GroupNodeConfig(name, nodeData)
await config.registerType()

const groupNode = LiteGraph.createNode(`workflow>${name}`)
const groupNode = LiteGraph.createNode(`${PREFIX}${SEPARATOR}${name}`)
// Reuse the existing nodes for this instance
groupNode.setInnerNodes(builder.nodes)
groupNode[GROUP].populateWidgets()
Expand Down Expand Up @@ -1444,16 +1450,28 @@ function addConvertToGroupOptions() {
}
}

const replaceLegacySeparators = (nodes: ComfyNode[]): void => {
for (const node of nodes) {
if (typeof node.type === 'string' && node.type.startsWith('workflow/')) {
node.type = node.type.replace(/^workflow\//, `${PREFIX}${SEPARATOR}`)
}
}
}

const id = 'Comfy.GroupNode'
let globalDefs
const ext = {
name: id,
setup() {
addConvertToGroupOptions()
},
async beforeConfigureGraph(graphData, missingNodeTypes) {
async beforeConfigureGraph(
graphData: ComfyWorkflowJSON,
missingNodeTypes: string[]
) {
const nodes = graphData?.extra?.groupNodes
if (nodes) {
replaceLegacySeparators(graphData.nodes)
await GroupNodeConfig.registerFromWorkflow(nodes, missingNodeTypes)
}
},
Expand Down
Loading
Loading