fluid-hierarchical-task-network: Domain.FindPlan(...) rejects valid new plan

Describe the bug Domain.FindPlan(...) rejects valid new plan, because the MethodTraversalRecord equals the LastMTR, even though the found plan differs from the last found plan.

To Reproduce Define a domain like this (where OperatorA and OperatorB are both operators that do NOT finish instantly):

.Select("Test Select")
    .Action("Test Action A")
        .Condition("Can choose A", ctx => ctx.HasState(TestWorldState.CanChooseA))
        .SetOperator(new OperatorA())
    .End()

    .Action("Test Action B")
        .Condition("Can not choose A", ctx => !ctx.HasState(TestWorldState.CanChooseA))
        .SetOperator(new OperatorB())
    .End()
.End()

Then set TestWorldState.CanChooseA to true and Tick() the planer. Observe (as expected) that a plan containing only “Test Action A” is found. Then (while the existing plan is not yet finished) set TestWorldState.CanChooseA to false and Tick() the planer again. Observe that the plan containing only “Test Action B” is initially found during the decomposition but is then discarded by this part of the code:

// If this MTR equals the last MTR, then we need to double check whether we ended up
// just finding the exact same plan. During decomposition each compound task can't check
// for equality, only for less than, so this case needs to be treated after the fact.
var isMTRsEqual = ctx.MethodTraversalRecord.Count == ctx.LastMTR.Count;
if (isMTRsEqual)
{
    for (var i = 0; i < ctx.MethodTraversalRecord.Count; i++)
        if (ctx.MethodTraversalRecord[i] < ctx.LastMTR[i])
        {
            isMTRsEqual = false;
            break;
        }

    if (isMTRsEqual)
    {
        plan = null;
        status = DecompositionStatus.Rejected;
    }
}

because both the MethodTraversalRecord and the LastMTR are equal and contain only the single entry 0, even though the new plan (containing only “Test Action B”) is different from the old plan (containing only “Test Action A”).

Expected behavior The new plan should not have been rejected for being equal to the old plan because it is clearly different.

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 40 (21 by maintainers)

Most upvoted comments

Ah, I found the conceptual difference: In my test it is the other way around:

Initially the WorldState is set in a way that the “better”/first Action is chosen, then the WorldState changes so that the first Action is no longer allowed and the “worse”/second Action should be chosen instead.

If you change your test like this, you can see it for yourself:

public void FindPlanIfWorldStateChangeAndOperatorIsContinuous_ExpectedBehavior()
{
    var ctx = new MyContext();
    ctx.Init();

    var planner = new Planner<MyContext>();
    var domain = new Domain<MyContext>("Test");
    var select = new Selector() { Name = "Test Select" };

    var actionA = new PrimitiveTask() { Name = "Test Action A" };
    actionA.AddCondition(new FuncCondition<MyContext>("Can choose A", context => context.GetState(MyWorldState.HasA) == 0));
    actionA.SetOperator(new MyOperator());
    var actionB = new PrimitiveTask() { Name = "Test Action B" };
    actionB.AddCondition(new FuncCondition<MyContext>("Can not choose A", context => context.GetState(MyWorldState.HasA) == 1));
    actionB.SetOperator(new MyOperator());

    domain.Add(domain.Root, select);
    domain.Add(select, actionA);
    domain.Add(select, actionB);

    Queue<ITask> plan;
    ITask currentTask;

    planner.Tick(domain, ctx, false);
    plan = planner.GetPlan();
    currentTask = planner.GetCurrentTask();
    Assert.IsTrue(plan != null);
    Assert.IsTrue(plan.Count == 0);
    Assert.IsTrue(currentTask.Name == "Test Action A");
    Assert.IsTrue(ctx.MethodTraversalRecord.Count == 2);
    Assert.IsTrue(ctx.MethodTraversalRecord[0] == 0);
    Assert.IsTrue(ctx.MethodTraversalRecord[1] == 0);

    // When we change the condition to Done = true, the first plan should no longer be allowed, we should find the second plan instead!
    ctx.SetState(MyWorldState.HasA, true, EffectType.Permanent);

    planner.Tick(domain, ctx, true);
    plan = planner.GetPlan();
    currentTask = planner.GetCurrentTask();
    Assert.IsTrue(plan != null);
    Assert.IsTrue(plan.Count == 0);
    Assert.IsTrue(currentTask.Name == "Test Action B");
    Assert.IsTrue(ctx.MethodTraversalRecord.Count == 2);
    Assert.IsTrue(ctx.MethodTraversalRecord[0] == 0);
    Assert.IsTrue(ctx.MethodTraversalRecord[1] == 1);
}

Emelion1 on Dec 20, 2022

Hmm. Your test passes but my test still fails… I will investigate what the remaining difference is.

Emelion1 on Dec 20, 2022

I will have a look.

Emelion1 on Dec 20, 2022

Here, I modified your test to be able to reproduce the problem without needing my extensions:

private class MyOperator : IOperator
{
    public TaskStatus Update(IContext ctx)
    {
        return TaskStatus.Continue;
    }

    public void Stop(IContext ctx)
    {
    }
}

[TestMethod]
public void FindPlanIfSelectorFindBetterPrimaryTaskMTRChangeSuccessfully_ExpectedBehavior()
{
    var ctx = new MyContext();
    ctx.Init();

    var planner = new Planner<MyContext>();
    var domain = new Domain<MyContext>("Test");
    var select = new Selector() { Name = "Test Select" };

    var actionA = new PrimitiveTask() { Name = "Test Action A" };
    actionA.AddCondition(new FuncCondition<MyContext>("Can choose A", context => context.Done == true));
    actionA.SetOperator(new MyOperator());
    var actionB = new PrimitiveTask() { Name = "Test Action B" };
    actionB.AddCondition(new FuncCondition<MyContext>("Can not choose A", context => context.Done == false));
    actionB.SetOperator(new MyOperator());

    domain.Add(domain.Root, select);
    domain.Add(select, actionA);
    domain.Add(select, actionB);

    Queue<ITask> plan;
    ITask currentTask;

    planner.Tick(domain, ctx, false);
    plan = planner.GetPlan();
    currentTask = planner.GetCurrentTask();
    Assert.IsTrue(plan != null);
    Assert.IsTrue(plan.Count == 0);
    Assert.IsTrue(currentTask.Name == "Test Action B");
    Assert.IsTrue(ctx.MethodTraversalRecord.Count == 2);
    Assert.IsTrue(ctx.MethodTraversalRecord[0] == 0);
    Assert.IsTrue(ctx.MethodTraversalRecord[1] == 1);

    // When we change the condition to Done = true, we should now be able to find a better plan!
    ctx.Done = true;

    planner.Tick(domain, ctx, true);
    plan = planner.GetPlan();
    currentTask = planner.GetCurrentTask();
    Assert.IsTrue(plan != null);
    Assert.IsTrue(plan.Count == 0);
    Assert.IsTrue(currentTask.Name == "Test Action A");
    Assert.IsTrue(ctx.MethodTraversalRecord.Count == 2);
    Assert.IsTrue(ctx.MethodTraversalRecord[0] == 0);
    Assert.IsTrue(ctx.MethodTraversalRecord[1] == 0);
}

Emelion1 on Dec 20, 2022