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

Revert: SvRecursiveNode mixin (#3954) #4334

Merged
merged 1 commit into from
Sep 13, 2021

Conversation

Durman
Copy link
Collaborator

@Durman Durman commented Sep 9, 2021

If data with vertices was connected to mask socket it changed shape fo all output data.

Preflight checklist

  • Code changes complete.

new version of the node had to be created
@vicdoval
Copy link
Collaborator

vicdoval commented Sep 9, 2021

Why would you connect vertices data to the mask input? Is makes sense if you input a mask with a level 3 (as the vertices lists) the matching of data changes the output compared to if you input the usual level 2 mask

@Durman
Copy link
Collaborator Author

Durman commented Sep 9, 2021

I used mask node for filtering vertices data. The node does not provide another way.
2021-09-09_16-33-19
I'm not against new functionality but it should extend existing functionality or create next version of a node if it changes existing behaviour.

@vicdoval
Copy link
Collaborator

vicdoval commented Sep 9, 2021

I used mask node for filtering vertices data. The node does not provide another way.
2021-09-09_16-33-19
I'm not against new functionality but it should extend existing functionality or create next version of a node if it changes existing behaviour.

This behaviour look to me more like a bug than a feature (what vectors are a True mask (1,1,1), (0,2,1)...?)

@Durman
Copy link
Collaborator Author

Durman commented Sep 9, 2021

Hm.. it doesn't look like a bug to me. Any way it's useful functionality and there is no any alternative to it. I used it in many layouts and I guess that other users may do this too. My opinion is that this changes should cause another node version to be created.

@Durman Durman added this to the Sverchok v.1.0 milestone Sep 10, 2021
@vicdoval
Copy link
Collaborator

But in which cases the vector in interpreted as false value? Only when is (0,0,0)?

@Durman
Copy link
Collaborator Author

Durman commented Sep 11, 2021

I guess never.

@Durman Durman merged commit 2d6a61b into master Sep 13, 2021
@Durman Durman deleted the revert_last_changes_merge_by_distance branch September 13, 2021 04:12
@vicdoval
Copy link
Collaborator

This pr may break layouts made after the SvRecursiveNode mixin to avoid breaking layouts (with a weird feature use) that existed previously. I think it should be reversed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants