AUTOMATIC1111 / stable-diffusion-webui-extensions

Extension index for stable-diffusion-webui
483 stars 261 forks source link

Add SD-PPP extension #324

Closed zombieyang closed 3 months ago

zombieyang commented 3 months ago

Info

Checklist:

w-e-w commented 3 months ago

normally I will test an extension and verify that it's working before merging it to index but because I don't use Photoshop I cannot test extention my self, so I'm asking on Discord if anyone that can test if no one responds to my text request I will merge it to index without testing

zombieyang commented 3 months ago

normally I will test an extension and verify that it's working before merging it to index but because I don't use Photoshop I cannot test extention my self, so I'm asking on Discord if anyone that can test if no one responds to my text request I will merge it to index without testing

Hi, have you found someone for test yet?

w-e-w commented 3 months ago

nope~ I asked in Discord and some guy in Discord asked for me in other Discors, but after that no one replied basically...

for the record I was waiting for a week to see if anyone replies


could you write better descriptions in your readme documentation of what your extension does

the way I see it the short description in the index tells me more about the extension than your entire readme

I think SD/Comfy will keep growing fast in the future. So, instead of running SD/Comfy in Photoshop, this tool provides a option that make Photoshop become a widget of SD/ComfyUI. To help you use SD/ComfyUI more easily.

the way I read this is

~~I think SD/Comfy will keep growing fast in the future.

unnecessary info

So, instead of running SD/Comfy in Photoshop,

with my knowledge I'm assuming that your referencing AbdullahAlfaraj/Auto-Photoshop-StableDiffusion-Plugin not everyone will understand

this tool provides a option that make Photoshop become a widget of SD/ComfyUI.

the term "widget" can't be interpreted so many ways that it's not actually very informative

To help you use SD/ComfyUI more easily.

sure I guess


I probably should have told this you earlier there's some potential issues with your code base

note I have not tested the code so this is just from pure code review standpoint

issue is mostly with the structure of the repo and the naming of objects

  1. don't use common names name like library for you ... well librarys... and names like sd_init nodes the name space is shared across all extensions, so if anyone else decide to also use the same name bad things might happen rename it to something unique like the, my recommendation is basically used the repository name for your library directory https://github.com/w-e-w/sd-webui-xyz-addon

  2. there should only be 1 entry point of extention I'm not sure how it is for comfy but for webui every py file scripts will be executed individually sometimes this can cause issues because certain scripts may be executed multiple times it's best to keep everything else except the main entry point of your script inside a library

    again you can reference my extension above I only have one file inside scripts which import other things I want if any code needs to be executed not just on imported (restart / launch) but also on ui reload (webui footer) then code needs to be called in main script

if there are specific reasons you did what you did then discard my recommendations


this is hilarious by the way https://github.com/zombieyang/sd-ppp/blob/62ac5e36666e1ec6e4b8f885c3a595dfd0575dd1/install.py#L6 I recommend changing the description to something I'm more appropriate, of course it's up to you



since no one replies to my test request I would like you to send a short video showing that your extension does indeed work

also add something like "Get Image from Photoshop, send Image to Photoshop" so one the lines of so the one can actually understand what your extension does as a glance

I will merge the extension after you sent a shot video and updated the repo with better descriptions

w-e-w commented 3 months ago

also if you have any extension to send it in the future don't fill the added date that is automatically generated by the merge workflow

w-e-w commented 3 months ago

@zombieyang I saw you made some changes to your extension structure 👍 and you also update the readme, even though I still think the readme still have room for improvement, I'll leave it up to you in the future

do you wish me to merge this into the index now?

zombieyang commented 3 months ago

@zombieyang I saw you made some changes to your extension structure 👍 and you also update the readme, even though I still think the readme still have room for improvement, I'll leave it up to you in the future

do you wish me to merge this into the index now?

sure, thank you, I was going to make a video this weekend :)

thanks for all your advice especially the import strategy of SD extension really confuse me for a while.

w-e-w commented 3 months ago

import strategy of SD extension really confuse me for a while.

we need better documentation ...... ( ̄ρ ̄)..zzZZ


don't you don't need to read this I'm just putting it here so if the day comes that I decides to write documentations I'll have something to draw upon

  1. .py in scripts dir are "executed", I'm not sure the proper term to describe this, it's like importing but it will really import if you reload the UI, and every scripts will be executed once on UI launch / reload , even if it's already imported by some other .py
  2. the extentions root is added tp sys.path (PYTHONPATH), you can import you own modules directly by the module name from the extentions root
  3. do NOT use extensions.extention_name.modules to import extension modules or script extensions could be installed under a custom name, it also would be installed in a different directory if --data-path is set
  4. preload.py, is called very early on during launch of web UI, before we parsing of command line args, if you wish to added command line args it hear
  5. install.py it's also executed very early on similar to `preload.py, but it is executed in a separate sub process, typically this is used to install new dependencies,
  6. defining specific class object and using callbacks allows you to extend web UI
  7. javascript dir allow you to add extra JavaScript that will be loaded to the web page
  8. localizations allows you to define localizations
  9. metadata.ini can be use to define dependencies to other extensions and load order if necessary
  10. most webui paths can be found in modules.paths.py modules.paths_internal.py
  11. setting can be added during on_ui_settings callback with shared.opts.add_option or by updateing shared.options_templates.update(shared.options_section((..., ...), {...})

gradio

  1. elem_id is optional but if used should be unique for scripts.Scripts.ui best to use scripts.Scripts.elem_id(str) to create the elem_id