0
Fixed in Alpha

Collapsing nodes into a Super Unit fails

Chris Smith 3 months ago • updated 2 months ago 15

Here is my node tree when created in a different node graph (the wires only show because of the keys I hold for my cropped screen grab):




Then After selecting it all and selecting the new collapse button and saving as a macro asset:





Console:

Bolt Version:
2.0.0.a4
Unity Version:
2018.3.11
.NET Version:
Bolt 2

Note: I noticed my screen grab was before I wired up the wheel controller to the grounded var but it didn't change or fix anything after I did.

[If I understand correctly what you are trying to do]

When you create a macro through a collapse, it creates an input output for you. That's why he gives an error, he does not expect you to do the work for him. Collapse is designed to break a large graph into small pieces quickly.

If you already have an input output in the nested graph, you can convert it to macro through the graph inspector. Or, if the graph is outside the nested unit, you can create an empty nested and copy-paste your units.

Yes, great point.  However, I did try doing a copy paste to a new graph last night and got the same results.

Today, based on your note I was going to erase my input and output to try the collapse again, except my node tree wasn't even saved correctly by Bolt and loaded as a single node with errors and no nodes at all inside it.

So, I think there is maybe still something "buggish" going on.


I may also add that assuming you were correct about me trying to collapse while I already have input/output nodes, I may suggest to Bolt team that there be an allowance of this behavior.  


Meaning if one already has the input and output when a collapse is requested that it see that the input/output already exists then simply moves that to a new macro instead of trying to create new input/output nodes and erroring.



Under Review

Hi Chris,

Thanks for the report. Can you use the graph upload tool on your parent graph (before collapsing) so I can test it myself?

Hi Lazlo,

I will need to recreate it over the weekend.  When this project was loaded the next day all of the nodes above were collapsed into an empty super unit shell with no ports.


P.S. Have had Bolt a couple of weeks now and I'm obsessed with it. Although I've coded for years, Bolt is the answer for me as the most enjoyable way to create in Unity whether you know code or not. Excited for Bolt's future progress.

Cannot Reproduce

Hi Chris,

Seeing as there are no full stack traces or a graph to reproduce from, I'll have to close this as CR for now. Feel free to comment again whenever you've created & uploaded a reproduction case.

I do want to add though, not sure if you saw the comments above.  But for the collapse function, if it doesn't already I want to suggest that if we made an input and output node in advance, that when we collapse, that it doesn't error because you already made an input and output node.  That the collapse can see you already did the work and do a direct fold.  Make sense?

Working on Fix

Ah sure, I can make sure it doesn't throw an error -- or just prevents you from doing it.

Because if you already have IO units, why are you collapsing at all? Your graph is a macro already.

I did it in a situation where I was inside a state in a state machine.  Had a bunch of nodes I had been working on I wanted to group in a super unit.  So, I started thinking about what ins and outs I would want , added them in and started forming the super unit before I collapsed.  If it errors if you make the in/out manually then it will take discipline to remember to collapse first THEN mess with the ins and outs.

Kind of like in nodal video compositing where you are working in a master graph and realize you have too much complexity on a branch and want to group them into a sub-unit or group while still being inside the master graph.

Right, but there is a problem with your approach, so it would make sense to enforce creating the inputs in a separate graph. The problem is that creating ports on IO nodes will also create ports at the graph level for the current graph, so you'd be left with phantom ports that you're not actually using. It's not going to crash or anything, but from a design perspective, it's not ideal.

I see. Like you said then, maybe have your code just ignore any I/O added and add new ones inside the collapse without error.  Then it's up to the user to remember not to waste their time messing with IO in advance, but there is no error involved.

Would it be too complex to at least copy the work done from a pre-IO inside of the superunit, while still leaving the "external" IO as external?

To answer very honestly, yes, it would be very complex. The collapse system is already an extremely complex process that tries to infer how to best create the IO ports from the available elements, so trying to have it "prefer" existing IO (but also create new IO if needed) would add another layer of potential points of failure.

I do agree there is an issue with the inability to copy/paste or reuse existing IO units. This is however a larger design problem, and while it should definitely be addressed somewhere down the road, I'll just focus on fixing the actual error here for the time being.

Sorry Lazlo, been out of town working.  I will try and reproduce in the future and see what happens.

Fixed in Alpha

Collapsing will now be disabled when the selection contains IO units starting in Alpha 5.

A very nice solution.  Thanks, Lazlo!