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

The nodes I suggested #202

Closed
wants to merge 6 commits into from
Closed

Conversation

Silversith
Copy link
Contributor

Please have a look and see what you think, I'm no expert at this, but I tried.
Had to do one fix for the "optional inputs" on the app.js and added a custom.json save + preview image.

@GrimblyGorn
Copy link

Everything here seems to be working so far for me. I haven't tested the custom nodes yet since I'm testing some of my own currently. Not sure what that patch.diff file was about but removing it didn't affect me seemingly, so I left it out.

Also, I left out the Delete portion. It felt too prominent and dangerous for my taste. That should possibly be stuck separately or perhaps toggled from the little gear menu for those that don't want or need it. If it were stuck in the little gear menu with a toggle, possibly it could show the default directory and allow changing/setting it. This could be handy, knowing it's set to delete a Test directory is fine I think. More so if it said which as it offered. I may just be rambling now though :)

Anyways, thanks. The optional inputs are nice. I'm sure the new nodes are too :)

@Silversith
Copy link
Contributor Author

Silversith commented Mar 24, 2023

Everything here seems to be working so far for me. I haven't tested the custom nodes yet since I'm testing some of my own currently. Not sure what that patch.diff file was about but removing it didn't affect me seemingly, so I left it out.

Also, I left out the Delete portion. It felt too prominent and dangerous for my taste. That should possibly be stuck separately or perhaps toggled from the little gear menu for those that don't want or need it. If it were stuck in the little gear menu with a toggle, possibly it could show the default directory and allow changing/setting it. This could be handy, knowing it's set to delete a Test directory is fine I think. More so if it said which as it offered. I may just be rambling now though :)

Anyways, thanks. The optional inputs are nice. I'm sure the new nodes are too :)

After doing a lot of testing on my own, using the method I was using results in the following problems;

  • More likely to mess up clothing
  • More likely to mess up hands
  • More generic faces/eyes (less variety)

All in all, it doesn't seem to be a great method, I'll see what else I can find or figure out

@Silversith
Copy link
Contributor Author

@comfyanonymous Please have a look and take some of the items I've added
I'm obviously a beginner at this, but there's a few things that might be needed;

  • Add Optional Inputs on Nodes
  • Notes Node
  • Saved Images List Node

Items just for me:

  • Delete Images Button on Float Menu (to clear the Saved Images List Node, it clears the entire output directory)
  • The Custom KSampler that outputs the seed and gets the seed as an input is probably junk, I don't think I understood how things work correctly

@comfyanonymous
Copy link
Owner

Yes, can you make separate pull requests for these so I can review them separately since they are not tied together.

I'm not interested by a "delete all" button but a route to delete specific files in the output folder could be useful.

Optional inputs are good.

What does your Saved Images List Node do?

Your custom ksampler is no longer needed since this got merged: #243

@Silversith
Copy link
Contributor Author

Will do when I next get a gap.
The save image list just shows all the images in the output directory. It does get a bit much if there's more than 50 images since it slows down your browser from all the ram use. It was my quick and dirty solution for a file browser. I'll probably write a standalone node that doesn't hold the images in cache later.

Merge Latest code from ComfyAnon to Silversith
@Silversith Silversith closed this Mar 31, 2023
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