TaskScript still checks accept() method of lower priority TaskNode, after accepting a higher priority TaskNode

[wefwefwef222 0]

This is just a minor performance issue...

My code:

package bots.test;

import org.dreambot.api.script.Category;
import org.dreambot.api.script.ScriptManifest;
import org.dreambot.api.script.impl.TaskScript;

@ScriptManifest(category = Category.CONSTRUCTION, name = "AAAAAAAAAAAAAA", author = "me", version = 1.0)
public class Test extends TaskScript {
    @Override
    public void onStart() {
        addNodes(new TestTask2(), new TestTask1());
    }
}

 

package bots.test;

import org.dreambot.api.script.TaskNode;

public class TestTask1 extends TaskNode {
    @Override
    public int priority() {
        return 1;
    }

    @Override
    public boolean accept() {
        log("task 1 accept");
        return true;
    }

    @Override
    public int execute() {
        return 5000;
    }
}

 

package bots.test;

import org.dreambot.api.script.TaskNode;

public class TestTask2 extends TaskNode {
    @Override
    public int priority() {
        return 2;
    }

    @Override
    public boolean accept() {
        log("task 2 accept");
        return true;
    }

    @Override
    public int execute() {
        return 5000;
    }
}

The subsequent logs:

• 5:47:42 PM: [SCRIPT] task 2 accept
• 5:47:42 PM: [SCRIPT] task 1 accept

 

I don't believe that the loop() method for TaskScript should still be checking the accept() methods of lower-priority-than-already-accepted TaskNodes. For example, because TestTask2 has a priority of 2, and was accepted, the accept() method of a test task with a lower priority (in this example TestTask1) shouldn't even be executed, because it's not a priority. This will save some CPU usage, since integer comparisons are easy, but someone could have complex accept() code. Hopefully I'm making sense. Thanks!

[Pandemic 2711]

Unfortunately this is necessary as the priority method isn't a constant, so we couldn't presort the tasks to exit on the first available task

The performance implications of this design are minor, and if this is really a concern you're free to extend AbstractScript and create your own task based implementation

[wefwefwef222 0]

1 hour ago, Pandemic said:

Unfortunately this is necessary as the priority method isn't a constant, so we couldn't presort the tasks to exit on the first available task

The performance implications of this design are minor, and if this is really a concern you're free to extend AbstractScript and create your own task based implementation

I'm not saying exit on the first accepted. I'm saying some logic like this (obviously I'm sure the logic behind the scenes is a little more complex):

    ArrayList<TaskNode> nodeList = new ArrayList<>(); // Populated on start...
    public void onLoop() {
        TaskNode bestAccepted = null;
        for (TaskNode taskNode : nodeList) {
            if (bestAccepted == null || (taskNode.priority() > bestAccepted.priority() && taskNode.accept())) {
                bestAccepted = taskNode;
            }
        }
        sleep(bestAccepted.execute());
    }

Idk, just a thought... and I get what you're saying about me writing my own, but using the DB built-in library classes is so convenient/clean I figured I would bring it up. The logic above would be checking if it's even worth it to check the .accept() method at all, saving some (sure, minor) CPU time.

[Pandemic 2711]

1 hour ago, wefwefwef222 said:

I'm not saying exit on the first accepted. I'm saying some logic like this (obviously I'm sure the logic behind the scenes is a little more complex):

    ArrayList<TaskNode> nodeList = new ArrayList<>(); // Populated on start...
    public void onLoop() {
        TaskNode bestAccepted = null;
        for (TaskNode taskNode : nodeList) {
            if (bestAccepted == null || (taskNode.priority() > bestAccepted.priority() && taskNode.accept())) {
                bestAccepted = taskNode;
            }
        }
        sleep(bestAccepted.execute());
    }

Idk, just a thought... and I get what you're saying about me writing my own, but using the DB built-in library classes is so convenient/clean I figured I would bring it up. The logic above would be checking if it's even worth it to check the .accept() method at all, saving some (sure, minor) CPU time.

Oh alright I misunderstood, I'll take a look at our implementation and see if that's something I can fix up easily

Ended up rewriting it, it should work as you've described starting in the next client release, thanks for pointing out the bug.

[wefwefwef222 0]

18 hours ago, Pandemic said:

Oh alright I misunderstood, I'll take a look at our implementation and see if that's something I can fix up easily

Ended up rewriting it, it should work as you've described starting in the next client release, thanks for pointing out the bug.

Nice, thanks!