-
Notifications
You must be signed in to change notification settings - Fork 73
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
Parameter customizer #16
Conversation
Will be moved to separate PR
A few bugs I've found (also general ideas that don't need to be done anytime soon but just put down as I thought of them)
Sorry for the long list, I've bolded what I think are more 'bugs' that probably need to be fixed for a merge, other stuff is just nice extras which we can deal with in the future! |
I think that was what I was initially expecting but i definitely agree in it looking a bit scary to new users... maybe removing the first drop down but keeping to load preset section separate and at the top so it is distinct from all the complicated controls? |
Have done a bit of work on this, think I have a good solution regarding UX. Here's a TODO list:
Not ideal to be working on the same branch but this way everything is contained in this PR. |
Thanks for taking the initiative on moving forward with the changes. My life went to hell in various ways over the past couple of months, which has had me in varying states of disfunction. Funnily enough, I added working on this PR to my to-do list for the next week just this morning. Let me know if there's any particular task you would like me to tackle; at this point I'm losing the trees for the forest (pun fully intended). |
I'm just glad to have a bit of time to get back into this project - have forgotten writing most of this code... some of it isn't that bad! I've managed to fix up most of the outright problems and neatened things up. Currently the default tree is a right mess - not sure if the best thing is to update the defaults for all the parameters or to try and set the tree type and load the parameters at startup. I'm happy to do the parameter names and descriptions as I have a pretty good knowledge having coded the generator itself. I'll also convert my descriptions from latex into a markdown wiki doc or something though this can be a longer term project. Once that's all done (and with some testing) I think it's pretty much ready for a proper release! Edit: only other thing to consider is animation using armatures, I think the way the curves are built should lend itself to armatures fairly well but it will probably be a significant amount of work to implement |
Ok I'm pretty happy with everything, only remaining problem is that some params have option for level 0 that don't do anything. I don't know if we can/it's worth having some of the branch parameters with 4 levels and some with 3 without the layout going weird and it breaking parameter indices... Might be best just to leave as is. It would be great if you could give everything a test and check you can't find any problems. I might have a crack at armatures and animation soonish (any input welcome!). Otherwise, as long as your testing doesn't find anything, I'm happy to merge this and post the add-on on some forums and see if people want to use it! |
Leaf bend was making things look worse in my view (I don't think I ever got the implementation quite right...) so I have simplified it. |
I really dig the new options layout; nice work! There are still some kinks, but I think the addon's almost ready for prime time after we take care of the ones I've found today. The one thing I'd like to see added before prime time is a little bit of documentation. I'll copy the parameter descriptions you've given over into the README file as a bare-minimum. I think that would honestly be good enough for launch. |
Yeah having the description in the README is a good start in terms of documentation, I'd like to have some visual descriptions of the different parameter effects ultimately but that will take some time to do... Nice one picking up those issues, let me know when you're happy with everything/if there are any other issues you want to discuss! If it helps this is the original descriptions (in LaTeX) that I copied those from:
|
Well, the description for **Radius Modifier** is missing, and it appears to be missing in the LaTeX as well.
|
Ah yeah forgot about that, it's a super hacky one I added specifically for the weeping willow. It basically is a manual override for the branch radius at a given level (normally defined based on the length and parent radius). In that case the branches are extremely long so are much too thick using the normal parameters, so I used radius_mod of 0.1 for the offending level of branches. We should probably dissuade people from using it generally as it is better to try and follow the normal parameterisation in almost all cases to give the most realistic branching structure. |
It's done! Full customizer GUI, including saving and loading to/from parameter files. The UX could be improved a bit with better label names, although I took the liberty of changing a few of them.