Add Patrol Command and Goal

[rycbar0]

The new goal type allows to have baritone path to multiple points of interest in succession. There is also a new command to use with the new goal type. The waypoint command got a update to add a waypoint to a patrol route.

[ZacSharp]

I know (at least now, 27 minutes after submitting my previous review. Please forgive me) this pr is a draft, but since this is a pretty fundamental structural problem with this approach to patrolling I'll write it anyway.
If you want a changing goal you need some process to run the logic, both because goals are assumed to be stateless and because making them depend on game state like this can cause threading and performance problems.

[wagyourtail]

Fix completed event status for arriving with goto. Add a process that wraps a command queue that sends the next when it gets the completed event from the current child process. That sound about right for what the structure should be @ZacSharp

[ZacSharp]

from the current child process

Can you elaborate on this a bit more please?
Apart from me being confused by the term "child process" appearing in the context of Baritone processes this sounds about right though.
I also like listening for the completion event instead of checking isInGoal like CustomGoalProcess does it (because the latter sometimes causes premature cancellation, which also is the sole reason for the event not working after #goto <coords>).

[wagyourtail]

so i phrased it in a way to implement it wider with the long ask for task queue. for this we would have to have a new term child process for this new task aggregator process in order to have it wrap the process that's performing the current task in order to know when it. And patrol would be an example of something the task aggregator could do by queueing multiple goto's

[leijurv]

I don't think this is the right approach either. Commands and processes aren't the right abstraction to create a state machine. It would need to be a new class I think. Processes aren't meant to contain each other and they don't reasonably have duration, and commands having duration is also weird. A command isn't a flexible enough interface to create a good state machine, you want to be able to pass in arbitrary classes (such as other states) rather than just strings. Setting a wait in CustomGoalProcess breaks down the abstraction in an icky way.

Let me give an example from an internal spawnmason project, which uses a state machine to implement gringotts.

Each state can return either a new state, or a PathingCommand.

The wait state would have this as equivalent:

    public static class DelayState implements State {

        private final State then;
        private int ticks;

        // 0 = do it immediately
        // 1 = do nothing this tick, but do it next tick
        // 2 = do nothing this tick nor next, do it after that
        public DelayState(State then, int ticks) {
            this.ticks = ticks;
            this.then = then;
        }

        @Override
        public StateOrPath update() {
            ticks--;
            return ticks < 0 ? new StateOrPath(then) : STATIONARY;
        }
    }

Here are some more examples of states:

private static final StateOrPath STATIONARY = new StateOrPath(new PathingCommand(null, PathingCommandType.REQUEST_PAUSE));

...

    public class AwaitingEnderPearlThrowConfirmation implements State {

        int ticks;
        boolean gotCooldown;
        boolean gotEntity;

        @Override
        public StateOrPath update() {
            if (getPackets(SPacketCooldown.class).anyMatch(packet -> packet.getItem() instanceof ItemEnderPearl)) {
                gotCooldown = true;
                logDirect("Got ender pearl SPacketCooldown");
            }
            if (getPackets(SPacketSpawnObject.class).anyMatch(packet -> packet.getType() == 65)) {
                gotEntity = true;
                logDirect("Got ender pearl SPacketSpawnObject");
            }
            if (gotCooldown && gotEntity) {
                logDirect("Throw confirmed");
                return new StateOrPath(new SortState(true));
            }
            if (++ticks > 600) {
                logDirect("Retrying throw");
                return new StateOrPath(new SetPearlState());
            }
            return STATIONARY;
        }
    }

...

    public class GotoEnderChestState implements State {

        final EnderChestInteractionState onceOpened;
        final List<BlockPos> echests = new CalculationContext(baritone).worldData.getCachedWorld().getLocationsOf("ender_chest", Integer.MAX_VALUE, ctx.playerFeet().x, ctx.playerFeet().z, 2);

        public GotoEnderChestState(EnderChestInteractionState onceOpened) {
            this.onceOpened = onceOpened;
        }

        @Override
        public StateOrPath update() {
            Goal goal = new GoalComposite(echests.stream().map(GoalGetToBlock::new).toArray(Goal[]::new));
            if (goal.isInGoal(ctx.playerFeet()) || goal.isInGoal(baritone.getPathingBehavior().pathStart())) {
                for (BlockPos maybe : echests) {
                    Optional<Rotation> rot = RotationUtils.reachable(ctx, maybe, false);
                    if (rot.isPresent()) {
                        baritone.getLookBehavior().updateTarget(rot.get(), true);
                        if (ctx.isLookingAt(maybe)) {
                            return rightClickThen(new AwaitingEnderChestGuiOpenConfirmation(onceOpened, this, maybe));
                        }
                        return STATIONARY;
                    }
                }
                logDirect("I'm standing somewhere where I think I should be able to open a chest, but it doesn't seem to be reachable :(");
            }
            return new StateOrPath(new PathingCommand(goal, PathingCommandType.REVALIDATE_GOAL_AND_PATH));
        }
    }

In that last one you can see interesting behavior such as passing this as a state into another state's constructor for an easy and intuitive "retry on failure", as well as how composable it is, such as with rightClickThen, which returns STATIONARY until the player's motion is nearly zero, then right clicks, then goes to the next state. Instead of implementing this each time, rightClickThen makes it easy.

    private StateOrPath rightClickThen(State next) {
        Baritone.settings().remainWithExistingLookDirection.value = false;
        if (!stationaryIsh()) {
            return STATIONARY;
        }
        return new StateOrPath(new DelayState(() -> {
            baritone.getInputOverrideHandler().setInputForceState(Input.CLICK_RIGHT, true);
            return new StateOrPath(next);
        }, 1));
    }

Would be cool to add something like this to upstream baritone :)

The actual implementation is very simple:

public interface State {
    StateOrPath update();
}

public class StateOrPath {

    private final State state;
    private final PathingCommand path;

    public StateOrPath(final State state) {
        this.state = state;
        this.path = null;
    }

    public StateOrPath(final PathingCommand path) {
        this.state = null;
        this.path = path;
    }

    public Optional<State> getState() {
        return Optional.ofNullable(state);
    }

    public PathingCommand getPath() {
        return path;
    }

}

...

public class StateMachine {

    private State currentState;

    public StateMachine(final State initialState) {
        this.currentState = initialState;
    }

    public State getCurrentState() {
        return currentState;
    }

    public void setCurrentState(final State state) {
        this.currentState = state;
    }

    public PathingCommand update() {
        StateOrPath next = new StateOrPath(currentState);
        int iters = 0;
        do {
            Helper.HELPER.logDebug("State: " + next.getState().get().getClass());
            currentState = next.getState().get();
            if (++iters > 20) { // dont let this go for forever, no matter what happens
                break;
            }
            next = currentState.update();
        } while (next.getState().isPresent());
        // the state should have the pathing command ready now!
        return next.getPath();
    }
}

[ZacSharp]

A comment that ended up being my pen and paper for sorting my thoughts. Might contain inconsistencies and whatever. Not all of it reflects my current opinion anymore, but it would be a shame to just delete it. You don't need to read it.

I think we can do two things here

1. Go back to the original scope of this pr and do just a goal queue. There's already functionality to check whether a goal is reached so we can just make CustomGoalProcess have a queue of goals or create a new PatrolProcess for more complicated goal switching logic (like switching randomly, not "logic" as in "programmable") and get away with it.
2. Do more general task chaining. Without major efforts I'd do that as follows
   1. Add a temporary low priority process which ignores onLostControl and is notified by #cancel like BuilderProcess is notified by #resume.
   2. Whenever that process is ticked it pops an instruction from it's queue and runs it. An instruction would generally be a wrapper around a String which is executed as a normal command.
   3. The hacky wait command would be avoided by having certain subcommands of the #queue add command like #queue add wait which look like the normal #queue add command but in reality they add a special instruction token to the command queue. This would effectively create commands which can only be used inside the queue.
   4. This could easily be expanded to a very imperative DSL with subroutines etc. with CommandQueueProcess turning into a line wise interpreter
   5. We could also use this with arbitrary java code by making the instruction class/interface accessible
   6. The above points are struck through because I can't imagine them being nice to use. More like programming in Brainfuck.
   I personally don't having a higher priority process wait on lower priority processes to finish or abandon their current task (processes don't have duration, but the things they do do have a duration) would be that bad, however I see how it's not really optimal since it's very much on the line of trying to establish delegation without the appropriate structure.
   TLDR of this spoiler: I don't like some things

   My approach to establishing the required structure would probably have been creating some sort of Task which is essentially a short lived process made with delegation in mind, but that feels very similar to AltoClef and I don't like the way their tasks look and it would probably still suffer much of the same problems as the current processes (i.e. passing control back and forth in the presence of unsafe actions as mentioned in Nasty parts of Baritone #3565). I also would try my best to prevent FSMs and their variations like HFSMs, they are either powerless or spaghetti. Doing behavior trees would end up being the Task approach I already mentioned with some additional headache for making it a behavior tree and just slapping some onFinish at the current processes doesn't feel like a good idea.
   Two things I like

   We could use threading to turn long behaviors into synchronous functions, composing behavior just like normal code (I have a ready to use synchronizer class to be used for that exact purpose for whatever I want to automate next) and switching between concurrent tasks by switching between "parallel" threads. Waiting for things you started would become as simple as waiting for the call to return and even isSafeToCancel would become easier to do: things which aren't safe to cancel would just catch the CancellationException thrown by waitTick until they are done. Forceful cancellation without memory leak would be impossible though and shared control at least hard to do because the framework we'd have to build on is already restrictive.
   However what leijurv showed also looks simple to write and use and would allow for easy composition both in the "command queue" case and for the creation of general behavior. It is also easily combinable with some changes to input handling I'm currently working on (ask if you are interested, this wall of text is too big already) and would maybe also make the "cancel once safe" problem easier to solve by caching states which aren't safe to cancel so they can run instead of the normal logic in the next tick. The situation for combining inputs is about the same as it currently is. Given that leijurv also seems to have positive experience with it and I just have hypothetics or negative experience with other options I think it's the best option we have for a new execution scheme
   Doubts whether we even need any of the above

   For a working command queue we would still need a way to determine whether the action started by a command is done though and the current processes are already enough to do a goal queue, so unless we're going to offer a complete new script-esque interface to the functionality of the existing commands I don't see much use in any type of new execution system. Third parties can (and do) already roll their own execution schemes so unless it offers e.g. better interaction with other Baritone based features there's no reason for them to use the new state machine and unless some of the things in the previous sentence change there's no reason for Baritone to use the new system. Even for things like finally implementing branch mining it would be simpler to just use an in-process state machine.
   However none of the above actually helps anything for command queues because those inherently have to run commands and regardless of how we try to set up a new system to allow nesting and complex behavior, if it runs arbitrary commands it ends up controlling itself in some sense.
   And finally an insight

   What we actually need to change is not the execution of behavior, it's the execution of commands. If a command is run, it must not activate some process, instead it must create a state / subprocess which will be returned to the caller. In the case of the player running a command from chat `ExampleBaritoneControl` will add that subprocess as a temporary (as in "will eventually cease to exist") top level task or set the state as the current state of the state machine and in the case of some process / state running the command it will start ticking it until it's done / return it as the next state (possibly wrapped in an AndThen(state, this).

Also I hate markdown for it's poor nestability. Why can't I just have a list in a spoiler in a list and still use inline code?

The short version is: I don't think we need a new execution system for command queues but rather a means of executing commands "within a context", meaning that commands shouldn't go and set some global state to make Baritone do some thing. Instead they should return an object that, when executed by some means, makes Baritone do the thing. That way there's the clear caller -> callee relationship needed to have the command queue take care of it's children properly.

Based on the current process system it could look like this

// the object would be more than just a BooleanSupplier, but we don't need more functionality for this example
// also note that I ingored the fact that a command might not return a thing to execute (e.g. #help)
// in QueueCommand.java
    public BooleanSupplier run(String label, IArgsConsumer args) {
        Stack<BooleanSupplier> taskList = // create task list, e.g. with code like in createRunCommandTask
        return () -> {
                    if (taskList.peek().get()) {
                        taskList.pop();
                    }
                    return taskList.isEmpty();
            };
        }
    }
    private BooleanSupplier createRunCommandTask(String command) {
        Lazy<BooleanSupplier> action = new Lazy(() -> baritone.getCommandManager().execute(command));
        return () -> action.get().get();
    }
// in ExampleBaritoneControl.java
    public void onSendChatMessage(ChatEvent event) {
        // parse command
        BooleanSupplier action = command.execute(label, args);
        baritone.getExecutorProcess().setTask(action); // could also add, create a new process, whatever
    }

And with a CPS executor maybe like this

// note that I ingored the fact that a command might not return a thing to execute (e.g. #help)
// in QueueCommand.java
    public State run(String label, IArgsConsumer args, State andThen) {
        State state = // create task list, e.g. with RunCommandStates
        // we don't really need to decompose the list into a chain of states, we could also have a State continuously return to itself and run the next action
        return state;
    }
    private static class RunCommandState extends State {
        private final String command;

        public RunCommandState(String command, State andThen) {
            super(andThen);
            this.command = command;
        }

        // override annotation omitted because GitHub can't handle it
        StateOrPath update() {
            return new PathOrState(baritone.getCommandManager().execute(command, andThen));
        }
    }
// in ExampleBaritoneControl.java
    public void onSendChatMessage(ChatEvent event) {
        // parse command
        State state = command.execute(label, args, State.DONE);
        baritone.getStateExecutor().setCurrentState(state);
    }
// actually, we might not go for full CPS and instead have a ExecuteAndThen(first, second) state which
// runs its own state machine for the first state so we don't have to pass andThen literally everywhere
// the example leijurv provided wasn't pure CPS either

EDIT: Actually let's do the crazy thing and plug in IBaritoneProcess for the BooleanSupplier just to show it really works

// Don't think I'm suggesting to plug this into Baritone
// also note that I ingored the fact that a command might not return a thing to execute (e.g. #help)
// also note that I have omitted onLostControl, priority and isTemporary. They would be delegated mostly like onTick and displayName.
// in QueueCommand.java
    public BooleanSupplier run(String label, IArgsConsumer args) {
        Stack<IBaritoneProcess> taskList = // create task list, e.g. with code like in createRunCommandTask
        return new IBaritoneProcess() {
                public PathingCommand onTick(boolean calcFailed, boolean isSafeToCancel) {
                    while (!taskList.isEmpty() && !taskList.peek().isActive()) {
                        taskList.pop();
                    }
                    if (taskList.isEmpty()) {
                        return null;
                    }
                    return taskList.peek().onTick(calcFailed, isSafeToCancel);
                }
                public boolean isActive() {
                    return !taskList.isEmpty();
                }
                public String displayName() {
                    return "TaskSequence running " + taskList.peek().displayName();
                }
        }:
    }
    private IBaritoneProcess createRunCommandTask(String command) {
        Lazy<IBaritoneProcess> action = new Lazy(() -> baritone.getCommandManager().execute(command));
        return new IBaritoneProcess() {
                public PathingCommand onTick(boolean calcFailed, boolean isSafeToCancel) {
                    return action.get().onTick(calcFailed, isSafeToCancel);
                }
                public boolean isActive() {
                    return action.map(IBaritoneProcess::isActive).orElse(true); // avoid running the command before ticking
                }
                public String displayName() {
                    return " process from command " + command;
                }
        };
    }
// in ExampleBaritoneControl.java
    public void onSendChatMessage(ChatEvent event) {
        // parse command
        IBaritoneProcess action = command.execute(label, args);
        baritone.getExecutorProcess().setTask(action);
    }
// in ExecutorProcess.java
    private IBaritoneProcess child;
    public void setTask(IBaritoneProcess child) {
        this.child = child;
    }
    public PathingCommand onTick(boolean calcFailed, boolean isSafeToCancel) {
        return child.onTick(calcFailed, isSafeToCancel);
    }
    public boolean isActive() {
        return child != null && child.isActive();
    }

I don't think the state machine is needed for this pr, but I'd be happy about a pr introducing it (I could also write it if you want that).

[rycbar0]

Im not able to do this.