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.

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

Sort nodes by id before saving #160

Closed
wants to merge 2 commits into from
Closed

Conversation

bonlime
Copy link

@bonlime bonlime commented Jul 18, 2024

Reopening this PR: comfyanonymous/ComfyUI#4052

Currently after even a minor modification of the workflow, it's saved in random order. This fix ensures that the nodes are sorted, which minimises the GIT diff after modification. And in general makes sense, given the fact that links are already sorted

@huchenlei
Copy link
Member

There is another place where we are saving the workflow:

setInterval(() => {
const workflow = JSON.stringify(this.graph.serialize());
localStorage.setItem("workflow", workflow);
if (api.clientId) {
sessionStorage.setItem(`workflow:${api.clientId}`, workflow);
}
}, 1000);

I feel like this should be implemented in litegraph instead of in the ComfyUI.

@huchenlei
Copy link
Member

Done in #161 with test

@huchenlei huchenlei closed this Jul 18, 2024
@christian-byrne
Copy link
Collaborator

This can be updated now too:

static graphEqual(a, b, path = "") {
if (a === b) return true;
if (typeof a == "object" && a && typeof b == "object" && b) {
const keys = Object.getOwnPropertyNames(a);
if (keys.length != Object.getOwnPropertyNames(b).length) {
return false;
}
for (const key of keys) {
let av = a[key];
let bv = b[key];
if (!path && key === "nodes") {
// Nodes need to be sorted as the order changes when selecting nodes
av = [...av].sort((a, b) => a.id - b.id);
bv = [...bv].sort((a, b) => a.id - b.id);
} else if (path === "extra.ds") {

@bonlime
Copy link
Author

bonlime commented Jul 19, 2024

Thanks for a quick implementation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants