-
Notifications
You must be signed in to change notification settings - Fork 433
Implement a CustomCombo node #7142
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,113 @@ | ||||||
| import { shallowReactive } from 'vue' | ||||||
|
|
||||||
| import { useChainCallback } from '@/composables/functional/useChainCallback' | ||||||
| import type { CanvasPointerEvent } from '@/lib/litegraph/src/types/events' | ||||||
| import { LGraphNode } from '@/lib/litegraph/src/LGraphNode' | ||||||
| import { LLink } from '@/lib/litegraph/src/litegraph' | ||||||
| import { app } from '@/scripts/app' | ||||||
|
|
||||||
| function applyToGraph(this: LGraphNode, extraLinks: LLink[] = []) { | ||||||
| if (!this.outputs[0].links?.length || !this.graph) return | ||||||
|
|
||||||
| const links = [ | ||||||
| ...this.outputs[0].links.map((l) => this.graph!.links[l]), | ||||||
| ...extraLinks | ||||||
| ] | ||||||
| let v = this.widgets?.[0].value | ||||||
| // For each output link copy our value over the original widget value | ||||||
| for (const linkInfo of links) { | ||||||
| const node = this.graph?.getNodeById(linkInfo.target_id) | ||||||
| const input = node?.inputs[linkInfo.target_slot] | ||||||
| if (!input) { | ||||||
| console.warn('Unable to resolve node or input for link', linkInfo) | ||||||
| continue | ||||||
| } | ||||||
|
|
||||||
| const widgetName = input.widget?.name | ||||||
| if (!widgetName) { | ||||||
| console.warn('Invalid widget or widget name', input.widget) | ||||||
| continue | ||||||
| } | ||||||
|
|
||||||
| const widget = node.widgets?.find((w) => w.name === widgetName) | ||||||
| if (!widget) { | ||||||
| console.warn(`Unable to find widget "${widgetName}" on node [${node.id}]`) | ||||||
| continue | ||||||
| } | ||||||
|
|
||||||
| widget.value = v | ||||||
| widget.callback?.( | ||||||
| widget.value, | ||||||
| app.canvas, | ||||||
| node, | ||||||
| app.canvas.graph_mouse, | ||||||
| {} as CanvasPointerEvent | ||||||
| ) | ||||||
| } | ||||||
| } | ||||||
|
Comment on lines
+9
to
+47
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Extract shared logic to reduce duplication. This function duplicates significant logic from Consider extracting the shared logic into a common utility function that both implementations can use, perhaps accepting a value-transformer function to handle the text-replacement difference. Example approach: // In a shared utility file
function propagateWidgetValue(
node: LGraphNode,
outputIndex: number,
value: any,
extraLinks: LLink[] = []
) {
if (!node.outputs[outputIndex].links?.length || !node.graph) return
const links = [
...node.outputs[outputIndex].links.map((l) => node.graph!.links[l]),
...extraLinks
]
for (const linkInfo of links) {
const targetNode = node.graph?.getNodeById(linkInfo.target_id)
const input = targetNode?.inputs[linkInfo.target_slot]
// ... rest of logic
}
}🤖 Prompt for AI Agents |
||||||
|
|
||||||
| function onNodeCreated(this: LGraphNode) { | ||||||
| this.applyToGraph = useChainCallback(this.applyToGraph, applyToGraph) | ||||||
|
|
||||||
| const comboWidget = this.widgets![0] | ||||||
| const values = shallowReactive<string[]>([]) | ||||||
| comboWidget.options.values = values | ||||||
|
|
||||||
| const updateCombo = () => { | ||||||
| values.splice( | ||||||
| 0, | ||||||
| values.length, | ||||||
| ...this.widgets!.filter( | ||||||
| (w) => w.name.startsWith('option') && w.value | ||||||
| ).map((w) => `${w.value}`) | ||||||
| ) | ||||||
| if (app.configuringGraph) return | ||||||
| if (values.includes(`${comboWidget.value}`)) return | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Clarify string coercion intent. The comparison uses string template coercion If intentional, consider adding a comment: - if (values.includes(`${comboWidget.value}`)) return
+ // Coerce to string to handle mixed types
+ if (values.includes(`${comboWidget.value}`)) returnOr use explicit conversion for clarity: - if (values.includes(`${comboWidget.value}`)) return
+ if (values.includes(String(comboWidget.value))) return📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| comboWidget.value = values[0] ?? '' | ||||||
| comboWidget.callback?.(comboWidget.value) | ||||||
| } | ||||||
| comboWidget.callback = useChainCallback(comboWidget.callback, () => | ||||||
| this.applyToGraph!() | ||||||
| ) | ||||||
|
|
||||||
| function addOption(node: LGraphNode) { | ||||||
| if (!node.widgets) return | ||||||
| const newCount = node.widgets.length - 1 | ||||||
| node.addWidget('string', `option${newCount}`, '', () => {}) | ||||||
| const widget = node.widgets.at(-1) | ||||||
| if (!widget) return | ||||||
|
|
||||||
| let value = '' | ||||||
| Object.defineProperty(widget, 'value', { | ||||||
| get() { | ||||||
| return value | ||||||
| }, | ||||||
| set(v) { | ||||||
| value = v | ||||||
| updateCombo() | ||||||
| if (!node.widgets) return | ||||||
| const lastWidget = node.widgets.at(-1) | ||||||
| if (lastWidget === this) { | ||||||
| if (v) addOption(node) | ||||||
| return | ||||||
| } | ||||||
| if (v || node.widgets.at(-2) !== this || lastWidget?.value) return | ||||||
| node.widgets.pop() | ||||||
| node.computeSize(node.size) | ||||||
| this.callback(v) | ||||||
| } | ||||||
| }) | ||||||
| } | ||||||
| addOption(this) | ||||||
| } | ||||||
|
|
||||||
| app.registerExtension({ | ||||||
| name: 'Comfy.CustomCombo', | ||||||
| beforeRegisterNodeDef(nodeType, nodeData) { | ||||||
| if (nodeData?.name !== 'CustomCombo') return | ||||||
| nodeType.prototype.onNodeCreated = useChainCallback( | ||||||
| nodeType.prototype.onNodeCreated, | ||||||
| onNodeCreated | ||||||
| ) | ||||||
| } | ||||||
| }) | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Add defensive check for widgets array.
The code assumes
this.widgets[0]exists without validation. While the node definition likely ensures this, adding a defensive check improves robustness.📝 Committable suggestion
🤖 Prompt for AI Agents