Yellow-Dog-Man / Resonite-Issues

Issue repository for Resonite.
https://resonite.com
134 stars 2 forks source link

Node: Duplicate Slot with multi exclude #2753

Open MahSandwich opened 1 month ago

MahSandwich commented 1 month ago

Is your feature request related to a problem? Please describe.

I would like to be able to duplicate a hierarchy on an avatar but exclude certain child hierarchies such as grab anchors and tool shelf anchors as this sometimes results in breaking references or duplicating items that should not be duplicated.

Describe the solution you'd like

A Duplicate node wich can have multiple slot inputs for excludes (expandable like the add_multi node and similars).

Describe alternatives you've considered

a Destroy node which finds and removes the hierarchies immediately after duplication but this does not solve broken references or work for separate objects that also got duplicated in the process. This otherwise generally works for non-avatars or if nothing is held, equipped, worn, etc.

Additional Context

This may be similar to the grab instancer with excludes but this is not grab instancing. My intention for this is visual effect but could have additional use.

Requesters

Hikari Akimori (hikari_akimori)

JackTheFoxOtter commented 1 month ago

This is a good idea, and in line with the excluded sub-hierarchies for grab instancers, but will probably have to wait for collections to be implemented.

stiefeljackal commented 1 month ago

This is a good idea, and in line with the excluded sub-hierarchies for grab instancers, but will probably have to wait for collections to be implemented.

Please explain how this needs to wait for collections.

As it is currently written in the issue, the proposed idea is something that would function like a Duplicate node but well be “expandable like the add_multi node and similars” to exclude one or more slots.

I believe the author of the issue is not suggesting inputs to explicitly exclude only those slots, but to exclude the root slot hooked into the input so that the children slots are implicitly excluded. It would be similar to how explicitly marking a slot non-persistent will implicitly mark the children slots as non-persistent as well.

Frooxius commented 1 month ago

While we could make an node that has expandable number of inputs, we'd handle this one through collections instead.

General rationale is that nodes that have dynamic number of inputs are more "fixed" and limited, because you need to know the exact number ahead of time and add appropriate number of inputs.

This works okay for operations like addition, because often you have scenarios where you need to add a few numbers together, because it fits a formula, but a bit less for something like this, where you might be dynamically finding/fetching the objects to exclude and the list length can vary.

This follows also how the internal API's for this are implemented, which will generally accept a list of objects to exclude from the operation.

MahSandwich commented 1 month ago

This is a good idea, and in line with the excluded sub-hierarchies for grab instancers, but will probably have to wait for collections to be implemented.

Please explain how this needs to wait for collections.

As it is currently written in the issue, the proposed idea is something that would function like a Duplicate node but well be “expandable like the add_multi node and similars” to exclude one or more slots.

I believe the author of the issue is not suggesting inputs to explicitly exclude only those slots, but to exclude the root slot hooked into the input so that the children slots are implicitly excluded. It would be similar to how explicitly marking a slot non-persistent will implicitly mark the children slots as non-persistent as well.

Correct. Excluding a slot would imply all child slots are also excluded.

stiefeljackal commented 1 month ago

@Frooxius

General rationale is that nodes that have dynamic number of inputs are more "fixed" and limited, because you need to know the exact number ahead of time and add appropriate number of inputs.

And this would fit for this Duplicate Slot node scenario where you are excluding the slots that are known. Taking the example from this issue, we know the root slots of the grab spheres and the tool shelves (two to four depending), so one can just plug in the two to four known slots into the n number of inputs. This is probably a slightly extreme case since you may need four inputs. However, I can see most cases where the user just needs to exclude the root slot that contains the slots that shouldn’t be duplicated with the slot.

This works okay for operations like addition, because often you have scenarios where you need to add a few numbers together, because it fits a formula, but a bit less for something like this, where you might be dynamically finding/fetching the objects to exclude and the list length can vary.

There is no mentioned of dynamically finding or fetching objects in this issue. The issue indicated excluding a known slot and implicitly excluding the children under it. If one wants to exclude slots based on a certain filter criteria, then that would be a different node altogether.

This follows also how the internal API's for this are implemented, which will generally accept a list of objects to exclude from the operation.

If you make this node expandable, wouldn’t that be a List behind-the-scenes anyway?

shiftyscales commented 1 month ago

Blocked by #572 as Frooxius described above.

Frooxius commented 1 month ago

And this would fit for this Duplicate Slot node scenario where you are excluding the slots that are known. Taking the example from this issue, we know the root slots of the grab spheres and the tool shelves (two to four depending), so one can just plug in the two to four known slots into the n number of inputs. This is probably a slightly extreme case since you may need four inputs. However, I can see most cases where the user just needs to exclude the root slot that contains the slots that shouldn’t be duplicated with the slot.

My point isn't that there aren't any cases where a fixed number of inputs wouldn't fit, but more that it's not general enough.

Implementation that uses a variable number of inputs only works for usecases where the number of items to exclude is known, but doesn't work for dynamic.

Implementation that uses dynamic works for both use-cases. Therefore we'd implement it this way.

There is no mentioned of dynamically finding or fetching objects in this issue. The issue indicated excluding a known slot and implicitly excluding the children under it. If one wants to exclude slots based on a certain filter criteria, then that would be a different node altogether.

We will usually look for a way to generalize people's requests to get most out of the work we put it. Rather than implementing a request verbatim that only fits the specific use-case user requested, we will often make decisions to implement things in a way that covers significantly more use-cases and provides more flexibility, even if that's not what the user exactly originally requested.

If you make this node expandable, wouldn’t that be a List behind-the-scenes anyway?

It's quite different actually. Nodes with variable number of inputs change the structure of the node itself. The number of inputs you feed in decided at coding time - you can't change it dynamically - doing so causes the whole node setup to be re-built.

A node that accepts collection as input can process collection of any size - the number of inputs is part of data itself.

stiefeljackal commented 1 month ago

@Frooxius I completely understand that you want to generalize this as much as you can, and I agree with you on that aspect. However, I do feel that the original request was general enough as is since there is a difference between something that is of a fixed size and something that is of a dynamic size. Although approaches for dynamic-sized lists can fit for fixed-sized lists, it also affects optimization and provides a very slight overhead. That’s why arrays with a fixed size is still a thing in programming and still provided as an option. Unless the collections implementation will have fixed-size collections that will have optimization benefits, I feel that making this entirely dynamic is being overly generalized by a very slight margin. Anyway, you have made your decision; I am not going to comment more on this issue unless prompted to.

Frooxius commented 1 month ago

@stiefeljackal I disagree on that aspect, this approach is not generic enough for me.

For the optimization aspect, it's important to consider how much it would actually speed things up in this particular scenario to see if it's worth the increased engineering cost.

In this particular case, duplication process itself is heavy enough in comparison, that I don't see this saving any measurable amount of time on the node. If the performance increase you get is something like 0.00001 % (just a tiny number for illustrative purposes), it's just not worth the extra engineering & maintenance effort.

If it was something that's part of very tight loop and the dynamic/fixed execution was significant part of execution time, then it'd be a different story, but it doesn't apply here.

stiefeljackal commented 1 month ago

@Frooxius I’m not talking about time, but mainly the data side of it. You need slightly more for dynamic lists compared to something like fixed arrays. However, this is just splitting hairs at this point.

At this point, let’s just agree to disagree. I don’t want this issue to go off topic.