Skip to content
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

I add a bool property in inputs node #99

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

huiyao8761380
Copy link

Whether the object is created at the world origin,at the cursor by default.

Whether the object is created at the world origin,at the cursor by default.
Whether the object is created at the world origin,at the cursor by default.
@huiyao8761380
Copy link
Author

QQ截图20200101122532

Copy link
Owner

@aachman98 aachman98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding the same property in all the child classes, you can add it once in the parent class "ScInputNode" (see .../sorcar/nodes/_base/node_input.py)
Since this affects all the input nodes, add the socket in pre_execute() method of the parent class as well.

I have no idea in pre_execute()
Copy link
Author

@huiyao8761380 huiyao8761380 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea in pre_execute() in thit night

Copy link
Author

@huiyao8761380 huiyao8761380 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It work!

@aachman98 aachman98 added enhancement Request for new feature and removed enhancement Request for new feature labels Mar 5, 2020
Copy link
Owner

@aachman98 aachman98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! I've mentioned a few minor changes. Make the modifications as you find the time. Will merge this by tomorrow.
Thanks for your contribution to Sorcar!

EDIT: Sorry for the late reply...I was busy for the last couple of months.


class ScInputNode(ScNode):
in_name: StringProperty(default="Object", update=ScNode.update_value)
in_WorldOrigin: BoolProperty(default=False, update=ScNode.update_value)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in_world_origin
(Just for consistency in variable names)

# in_uv: BoolProperty(default=True, update=ScNode.update_value)
out_mesh: PointerProperty(type=bpy.types.Object)

def init(self, context):
self.node_executable = True
super().init(context)
self.inputs.new("ScNodeSocketString", "Name").init("in_name")
self.inputs.new("ScNodeSocketBool", "World Origin").init("in_WorldOrigin",True)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep this property hidden by default because each input node has its own set of sockets visible by default (just to slightly increase UX)

nodes/_base/node_input.py Outdated Show resolved Hide resolved
@aachman98 aachman98 added the scheduled Feature to be released soon label Mar 6, 2020
@portnov
Copy link

portnov commented Mar 17, 2020

Just a general remark. For such parameters, I'd suggest to use EnumProperty instead of BoolProperty, just for better UX. If you do this as a checkbox / toggle button named "Create blablabla", it will not be obvious to user what will it do if the checkbox is disabled. With EnumProperty, you can create a property named "Create object" with two possible values: "at origin", "at cursor".

@huiyao8761380
Copy link
Author

Just a general remark. For such parameters, I'd suggest to use EnumProperty instead of BoolProperty, just for better UX. If you do this as a checkbox / toggle button named "Create blablabla", it will not be obvious to user what will it do if the checkbox is disabled. With EnumProperty, you can create a property named "Create object" with two possible values: "at origin", "at cursor".

yeah,I‘m learning dynamic enum property

@aachman98
Copy link
Owner

aachman98 commented Mar 18, 2020

This won’t require dynamic EnumProperty though. We only need 2 options in the drop down at the moment. It will also be easier to add more later.

@Durman
Copy link

Durman commented Mar 26, 2020

@portnov Or name can be more clear like Center to origin or Origin to 3D cursor.

@aachman98 aachman98 added question Further information is requested and removed scheduled Feature to be released soon labels May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants