iwkse / freesound

Blender Freesound addon
GNU General Public License v3.0
37 stars 6 forks source link

Downloading folder settings #19

Closed samytichadou closed 2 years ago

samytichadou commented 2 years ago

This PR adds some control over the download locations of the previz sounds and strip source sounds :

I tried to keep the code consistent without altering existing code, i added 2 commonly used functions at the start of the freesound.py file and a few import from os.path

Cheers !

samytichadou commented 2 years ago

I am using currently the addon, and i'm not sure if the best solution is to have the locations properties (download and previz) in the ui and scene settings, or in the addon preferences ? My initial take was it could be useful to change it regularly depending on the project, but not so sure now, what do you think ?

iwkse commented 2 years ago

Hi, thanks for this PR! I think it would be better to add it in the configuration panel, but I would rename the label for the paths. It's not clear one is for the preview. I would explicitly write preview and download.

iwkse commented 2 years ago

Thinking more about it, adding it directly to the freesound pane could be more flexible and could be possible to have separate folders for each blend file. Could be nice I think.

tin2tin commented 2 years ago

If the folder settings should be exposed in the sidebar, maybe it would be better to place them in a properties section after the search and the results sections(eg. in the end)? (My initial thought was also to place them in the configuration panel)

(Btw. I got stuck on the #18 exposing of waveform previews received from freesound. I guess it should be possible to expose them in the list control. Do any of you guys know how to do that? In #18 I got all of the loading etc. of the waveform previews)

iwkse commented 2 years ago

If the folder settings should be exposed in the sidebar, maybe it would be better to place them in a properties section after the search and the results sections(eg. in the end)? (My initial thought was also to place them in the configuration panel)

(Btw. I got stuck on the #18 exposing of waveform previews received from freesound. I guess it should be possible to expose them in the list control. Do any of you guys know how to do that? In #18 I got all of the loading etc. of the waveform previews)

Seems like a good idea. Sorry for the waveform but I really had little to no time lately. Hopefully next days it will be better and I can look into fixing the pending issues

samytichadou commented 2 years ago

So I guess i'll add these two new properties under a "scene settings" subpanel ? and maybe create a "search" subpanel also to have all freesound related ui clean and organized ? Would it be acceptable solution for both of you ? But to be clear, it would not be possible to have multiple folders per blend file, if blend files are in the same locations, "alongside" folder will always be blend_file_location/freesound_folder , with freesound_folder being the pattern set in the preferences. Are we all ok with this behavior ? Seems efficient and logic enough to me Cheers

iwkse commented 2 years ago
* I bumped the version and adds tin2tin and myself as authors in the bl_info, hope this is ok

As for this, I'd leave myself as author, just for simplicity. Otherwise such field will have to be filled with all possible developer opening a PR and contributing to the addon. I'm really thankful for the contributions and I'm glad to evaluate and accept. I'm quite open instead to add in the README a section for contributions.

samytichadou commented 2 years ago
* I bumped the version and adds tin2tin and myself as authors in the bl_info, hope this is ok

As for this, I'd leave myself as author, just for simplicity. Otherwise such field will have to be filled with all possible developer opening a PR and contributing to the addon. I'm really thankful for the contributions and I'm glad to evaluate and accept. I'm quite open instead to add in the README a section for contributions.

ok, i always did that and find it kind of logic, but do as you want

samytichadou commented 2 years ago

Just added the subpanels : image The download location part is in a box, to be able to easily add other scene settings if needed. The search subpanel is polled out if credentials are missing

I also added some informations when freesound credentials are not correct : image

I added a common class for freesound panels, and removed a few unused import.

The "conflict" detected by github is about this common class, but you can test, np about it, just another way of doing it.

iwkse commented 2 years ago

No, the conflict it's because you didn't pull changes before committing. You can rebase and force push and it will be fixed

samytichadou commented 2 years ago

No, the conflict it's because you didn't pull changes before committing. You can rebase and force push and it will be fixed

Oops, didn't check, just fixed it

samytichadou commented 2 years ago

Please remove the merge

Not sure where the merge comes from, rebase seems to did this, I ll see what I can do

samytichadou commented 2 years ago

all done

iwkse commented 2 years ago

all done

Thanks very much, I'm going to merge it