- Amsterdam, Netherlands
- https://cessen.com
-
Animator, rigger, and software developer. Currently working at the Blender Institute as a developer on Blender's animation system.
Been using Blender since 1998, and worked on Big Buck Bunny and Sintel (two of Blender's open movie projects).
- Joined on
2003-03-21 -
-
I don't feel strongly one way or the other. Mainly just wanted to make sure it was on your radar, since it felt a little out of place with the surrounding code. Feel free to address it (or not)…
You have this marked is WIP, but the code looks good and it works as expected in testing. I also think it's a good change.
Fair enough. Since you e.g. changed the helper subclasses to mix-in classes I thought you were taking the opportunity to do some minor cleanups as well.
Done. I also added explicit just-in-case checks that the action is indeed considered empty/legacy/layered in the respective tests.
Good point. I think both cases should be tested, however: an empty action and an explicitly legacy action. So I'll add an additional test case for the explicitly legacy action (and rename this…
Basically looks good to me. Just a question and a take-it-or-leave-it suggestion.
Maybe this is a good opportunity to also update the code comments to use proper capitalization and punctuation?
I'm not super familiar with how things are supposed to work around this kind of stuff, but just double-checking:
No meaningful difference, just didn't find get_action() when I was looking for a function that did that. Thanks!
The rearrangement of the logic here appears to just change what situations get misleading error messages. Now if autoexec fails, invalid expressions that don't otherwise require Python also get displayed as not working due to "Python restricted for security", which isn't really the cause. Example file attached.