Parallel node does not work correctly on first run

NodeCanvas Forums Support Parallel node does not work correctly on first run

Viewing 12 posts - 1 through 12 (of 12 total)
  • Author
    Posts
  • #17505
    gotenxds
    Participant

    I have a Parallel node that branches out to two paths, I set it to reset on first failure, for some reason it resets on first success causing the running action to stop and execute again, BUT, only for the first time, here is a gif showing it:
    Notice that I cause the first path to return success, the entire tree resets, I do it again, it does not.

    Attachments:
    You must be logged in to view attached files.
    #17511
    gotenxds
    Participant

    @Gavalakis
    I debugged the code of the parallel node, it seems that you are not clearing the finishedConnections variable which causes you to count the same connection twice on two different runs of OnExecute

    As you can see in the attached image right after the loop that counts the finished connections you have 2 connections marked as finished, but when inspecting the outConnections you can clearly see that one of them is still with status=running

    So, this node thinks both of it’s children are done (you can also see in the gif that that is not the case) and then calls
    ResetRunning in the check for finishedConnectionsCount == outConnections.Count

    For what ever reason, either the node starts off with one connection counted as finished, or there is another bug with clearing the the finished connections, in either case you can see the frustrating result in the gif above.

    Attachments:
    You must be logged in to view attached files.
    #17542
    Gavalakis
    Keymaster

    Hello. Sorry for the late reply and thank you for the information! I am really trying to reproduce this issue with the Parallel node but I honestly can’t :-/ Can you please post the code of your custom tasks used in the gif (at least the ones related like Is Talking, Set Moving and Follow Path) out of curiosity? Just to double check, are you using the latest version of NodeCanvas and the Parallel code OnReset method being like this:

    Last but not least, are you able to somehow reproduce this, or does this only happen in this specific tree?
    Please let me know (meanwhile I will double-check the parallel code again). Thank you.

    Join us on Discord: https://discord.gg/97q2Rjh

    #17548
    gotenxds
    Participant

    Hya Gavalakis, glad to see you’re alive 😉

    Sure, here is the code for the custom tasks:

    isTalking just checks the current value on another class, I know I can do this via the checkBoolean task but for now I prefer doing it like so:

    SetMoving is also very simple:

    FollowPath is a bit more complex and uses the A* package and generally it just sets the next point in a path for the npc to follow.

    I am using the latest version (3.29), and it does look like this:

    I will try and replicate the issue again on a different tree.

    #17549
    gotenxds
    Participant

    Gavalakis, I took me a few hours but I was able to create a minimal graph that does reproduce this issue using only native nodecanvas components,
    here is a gif of the issue and an explanation on how to reproduce it.

    So, this is a bit complicated and I did not dive into the graph code so I have no idea why this is happening but,
    To reproduce this you need
    1. A parallel root
    2. A selector leading to a DYNAMIC conditional node that is set to return failure on first run, I’l call this condition A
    3. A sequencer that leads from the parallel root and is waiting until a different condition, We will call this one condition B

    To reproduce this, it is very important that the conditional node (A) will first return failure on the first time.
    Then, switch it’s condition (A) so it returns true.
    Now that the A node is engaged (In this case I just wait for ever) switch condition B to be true too.
    This will cause a graph reset (only on the first time!)

    Unity_X7QBQA5DUi

    Attachments:
    You must be logged in to view attached files.
    #17573
    Gavalakis
    Keymaster

    Hello again. There is a misunderstanding of how the Parallel node works unfortunately; In the example you posted above the Parallel node works as it should, but I will do my best to explain why.

    – So, when the Parallel is set to “Repeat”, it will repeat its finished children/branches until the policy is met, or until all children have had a chance to finish at least once. (at which point it will return a status according to the policy set and in our case the tree reset since the Parallel here is the root node).

    So what is happening here is:

    1) The first time the tree runs, the right branch is finished as soon as the Wait 1 Second action completes. However the left branch is still not finished since there is a Wait Until node.

    2) When “mySecondBoolean” is set to True and the Conditional node goes Running, the right branch remains finished and the left branch remains un-finished.

    3) When “myBoolean” is set to True (therefore triggering the Wait Until node), the left branch finally becomes finished.
    Since now both the left and the right branches have had a chance to complete/finish, the Parallel node returns and the tree resets.

    4) We are now on a new tree cycle. Both branches are now un-finished. The left branch is un-finished because of the Wait Until node still waiting, but also the right branch is un-finished since the Wait 100000 seconds is running long.

    5) When we set “myBoolean” to True now, the left branch is getting finished, however the right branch remains un-finished since the Wait 100000 seconds is still running. Therefore the Parallel does not return and the tree not reset since the condition “both children have had a chance to finish at least once” is NOT met.

    Here is a gif (replication similar to your test tree) in which I have also added debug information on the connections of the Parallel node (which is probably a good idea to have anyway) which will hopefully better help understand the above:
    ParallelRepeat

    I have also attached an mp4 video so it is easier to scroll through time.
    ParallelRepeat

    You can also easily add the debug info on the connections I have now added by adding this code in Parallel.cs node:

    This will certainly also help you debug and understand what is happening in your other first tree that you posted.

    I did my best to explain what is happening and why the Parallel node works as it should in the end, but please let me know if you need any further elaboration. Thanks.

    Join us on Discord: https://discord.gg/97q2Rjh

    Attachments:
    You must be logged in to view attached files.
    #17576
    gotenxds
    Participant

    Hya Gavalakis, thanks for the answer, The behavior you are describing is not what is documented in the docs

    image_2024-03-04_151038050

    I also dont think it makes much sense, if the node policy is to only be finished on first failure than it should only finish on first failure, what you are describing is on first failure or success, which has its own policy, so this kinda makes both policies moot when “repeat” is selected.

    Regardless, if you are not willing to fix, can you suggest a better way to achieve this?

    For now I’ve added a “repeat forever” decorator over the sequencer which prevents the child from ever returning and thus prevents this, it feels like a workaround rather then an actual correct way.

    I strongly urge you to rethink if this makes sense.
    It seems to me like if the node says it will keep running until the first failure then it should do that.

    Attachments:
    You must be logged in to view attached files.
    #17578
    Gavalakis
    Keymaster

    Hello again. Indeed the web documentation does not mention this even though the “Repeat” tooltip in the editor does (that is typically more up-to-date and I will have to update the web doc to reflect that).

    ParallelRepeatTolltip

    In any case, I can’t really just change the behaviour of the Repeat option now because that would be a breaking change for existing graphs of other users since that is the behaviour of the Parallel “Repeat” for a long time now, but I can gladly add an extra option to handle what you want to achieve. So can you please explain what is the goal here, meaning what would you like to happen in the test graph we have created above?
    Do note that in this test graph, the policy of the Parallel (First Failure) is never met since none of its child ever return Failure. If we set the Policy to First Success on the other hand, then it will work like this:

    ParallelRepeatFirstSuccess

    Let me know what specifically you want to happen in that test graph and I will let you know how to ahcieve it, or add an extra option to the Parallel if not possible already.

    Join us on Discord: https://discord.gg/97q2Rjh

    Attachments:
    You must be logged in to view attached files.
    #17581
    gotenxds
    Participant

    In my case I dont really need to entire graph to ever reset, which is way I set it to on first failure, my reset point is the switch node (represented by the dynamic node)

    The idea here is to allow running additional events in the graph without interrupting the rest of the running graph, hence using the parallel, this is the only way I found of simulating events that don’t change the current state of the graph, if this is an anti pattern and there is a better way to run multiple branches at the same time in case of events let me know.

    In this case I would want the parallel node to act like a game loop or an infinite while loop

    #17585
    Gavalakis
    Keymaster

    If you want a Parallel root and have its children repeat forever, may I then suggest to use a Repeat “forever” decorator at each child of the Parallel like in the image below? In this case the “Repeat” option in the Parallel has no effect and the Policy is also pretty much irrelevant since the decorators repeat their child nodes forever.
    ParallelRepeatForever

    Let me know if this is the behaviour you are after.

    Join us on Discord: https://discord.gg/97q2Rjh

    Attachments:
    You must be logged in to view attached files.
    #17595
    gotenxds
    Participant

    That is what I’ve been doing as a workaround (I’ve said that in a previous comment here)
    Is that really a good way of implementing this? I’m worried about performance issues (from reading other posts here) and it seems that this would still be running extra checks on the main root node, considering this is going to be a large tree that I am going to run this on about 100~ agents I’m worried it will add up.

    #17601
    Gavalakis
    Keymaster

    Hello again,
    Yes, this is a fine way of doing this. This is what the Repeat nodes are for. The extra logic the Parallel runs with this setup is negligible performance-wise. I could potentially also add a policy/option in Parallel for Repeat, to either work as it currently does (default) or be possible to replicate the above behaviour without the need of using Repeaters for convenience.

    Join us on Discord: https://discord.gg/97q2Rjh

Viewing 12 posts - 1 through 12 (of 12 total)
  • You must be logged in to reply to this topic.