Possible error in Sequence tick

[michieletto]

Hi,
Thanks for the great library!
I've had a look at the code in the devel branch, and entered the Sequence class method tick.
At line 561, you yield node.
Then, the code checks if the node is the child itself and the node status is SUCCESS.
Then, after some operations, you yield again and return.

I might be wrong, but I think the yield at line 561 is somehow in the wrong place.
Maybe it should be moved after the checks?
It's otherwise hard to reach those checks with the current version, right?

If I'm wrong, please can someone better explain the idea behind the code?

Thanks.
Best.
Stefano

[michieletto]

I am also wondering if the checks at line 542 and line 545 are correct.

The current version is:

        elif self.memory and common.Status.RUNNING:
            assert self.current_child is not None  # should never be true, help mypy out
            index = self.children.index(self.current_child)
        elif not self.memory and common.Status.RUNNING:
            self.current_child = self.children[0] if self.children else None

but I suppose it should be:

        elif self.memory and self.status == common.Status.RUNNING:
            assert self.current_child is not None  # should never be true, help mypy out
            index = self.children.index(self.current_child)
        elif not self.memory and self.status == common.Status.RUNNING:
            self.current_child = self.children[0] if self.children else None

or even better:

        elif self.memory:
            assert self.current_child is not None  # should never be true, help mypy out
            index = self.children.index(self.current_child)
        else:
            self.current_child = self.children[0] if self.children else None

since as commented at line 549 these conditions should cover all variations

[sea-bass]

Not sure about the first comment, but your second one for sure! The only reason that worked is because, as you pointed out, the check didn't matter since the first if condition already checks whether your status is RUNNING.

Let me throw that into #481