-
Notifications
You must be signed in to change notification settings - Fork 1
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
Custom Edit Context Views #4
Conversation
Adds custom edit context views and closes Freedom-of-Form-Foundation#7 The views added are "Armature Mode", "Skeleton Mode", "Muscle Mode", and "Skin Mode" Additionally, there's a "Smart Object Mode" which functions as "Bone Edit Context" would but can be run on any object, not just bones. Smart Object Mode will make visible any selected objects, plus any objects in any geometry nodes present on these selected objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the Pull Request! ^.=.^ I've done a review. The code looks good, and the design is sound. The only code logic change I propose is in ARF_OT_Smart_Object_Mode.
There are a few style issues that should be addressed too, and then it can be merged. A bit nitpicky, but that keeps the code clean of course. I realize that the Lathreas/user-interface also doesn't yet pass the style review, so consider this style guide to be necessary for when it all gets merged into main. We might as well get that done now, so here it goes!
- Make sure your comments don't exceed the 88 character line limit. Feel free to split them into multiple lines if necessary.
- Make sure to end comments with a period (.) or colon (:) .
- Please add (simple) docstrings to the classes. No need to be fancy, for example adding something like
"""Sets the ARF mode to Skeleton Mode."""
(for the skeleton mode operator class) is enough for most of the classes here. - Please add a module docstring to explain what the file is for.
Thanks again for your work!
scripts/ui/__init__.py
Outdated
|
||
def register_all(): | ||
bone_properties.register() | ||
mode_selection.register() | ||
#object_type_selection.register() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this commented code before merging (registering object_type_selection
should go in a different PR).
scripts/ui/__init__.py
Outdated
print("Registered scripts.ui") | ||
|
||
|
||
def unregister_all(): | ||
bone_properties.unregister() | ||
mode_selection.unregister() | ||
#object_type_selection.unregister() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this commented code before merging (registering object_type_selection
should go in a different PR).
scripts/ui/mode_selection.py
Outdated
for modifier in selection.modifiers: | ||
if modifier.type != 'NODES': | ||
continue | ||
# Disabled geom node modifiers sometimes do not contain a node_group | ||
if (modifier.node_group == None): | ||
continue | ||
|
||
for input in modifier.node_group.inputs: | ||
if not (isinstance(input, bpy.types.NodeSocketInterfaceObject)): | ||
continue | ||
|
||
# For some reason input.default_value is a copy of the object and not the actual | ||
# object in the scene, so we need to get the actual object in the scene. | ||
inputobj = bpy.context.scene.objects.get(input.default_value.name) | ||
inputobj.hide_set(False) | ||
# We also need to set every parent to also be visible | ||
parent = inputobj.parent | ||
while (parent != None): | ||
parent.hide_set(False) | ||
parent = parent.parent | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see you are looping over the modifier list. Sadly that has the effect of displaying the objects that directly modify the bones, which may not be the objects that the user can edit (instead being intermediary objects). These objects can be rather large and clutter the viewport, due to their mathematical requirement of being large, so it's best if they stay hidden. The preferred way to display the correct objects is to loop over selection.ARF.bone_properties.geometry_constraints
and then getting constraint_object
from that. e.g.:
for selection in selected:
for geometry_constraint in selection.ARF.bone_properties.geometry_constraints:
geometry_constraint.constraint_object.hide_set(False)
Let me send you a .blend file where an object has those geometry_constraints properly filled in so that you can test the code!
as explicitly set in the ARF menu rather than trying to intuit them from the geometry nodes. Also adds asserts for ARF existing, removes any code going over line 80, and adds doc strings to the top of the file.
I believe the newest commit should resolve all of the issues that you brought up. I tested with the startup.blend you sent me and it did seem to work as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
LGTM.
Adds custom edit context views and closes Freedom-of-Form-Foundation#7
The views added are "Armature Mode", "Skeleton Mode", "Muscle Mode", and "Skin Mode"
Additionally, there's a "Smart Object Mode" which functions as "Bone Edit Context" would but can be run on any object, not just bones.
Smart Object Mode will make visible any selected objects, plus any objects in any geometry nodes present on these selected objects.