baritone icon indicating copy to clipboard operation
baritone copied to clipboard

IExecutionControl API class

Open crosby-moe opened this issue 2 years ago • 3 comments

  • Adds the convenience method IBaritone.createExecutionControl() which returns an IExecutionControl for creating a Baritone process who's sole purpose is to pause/resume Baritone.
  • Replaces the IBaritoneProcess in ExecutionControlCommands with an IExecutionControl, simplifying the class greatly.

The goal being to encourage 3rd party API users to employ proper methods for pausing/resuming Baritone as opposed to using the command system.

Pausing is simplified from:

private static boolean paused = false;

static {
    BaritoneAPI.getProvider().getPrimaryBaritone().getPathingControlManager().registerProcess(new IBaritoneProcess() {
        @Override
        public boolean isActive() {
            return paused;
        }
    
        @Override
        public PathingCommand onTick(boolean b, boolean b1) {
            BaritoneAPI.getProvider().getPrimaryBaritone().getInputOverrideHandler().clearAllKeys();
            return new PathingCommand(null, PathingCommandType.REQUEST_PAUSE);
        }
    
        @Override
        public boolean isTemporary() {
            return true;
       }

        @Override
        public void onLostControl() {}

        @Override
        public double priority() {
            return 0d;
        }

        @Override
        public String displayName0() {
            return "Pause Controls";
        }
    });
}

public static void pauseBaritone() {
    paused = true;
}

to

private static final IExecutionControl EXECUTION_CONTROL = BaritoneAPI.getProvider().getPrimaryBaritone().createExecutionControl("Pause Controls", 0d);

public static void pauseBaritone() {
    EXECUTION_CONTROL.pause();
}

crosby-moe avatar Aug 30 '23 23:08 crosby-moe

W

machiecodes avatar Aug 30 '23 23:08 machiecodes

I was debating whether I should make IExecutionControl extend IBaritoneProcess since it's, at its core, just a wrapper and would maybe enable more functinality, but it would also create confusion since IExecutionControl.paused() would have the same return value as IBaritoneProcess.isActive().

I could also just make a getter in IExecutionControl for the underlying IBaritoneProcess I guess...

pls give input

crosby-moe avatar Aug 30 '23 23:08 crosby-moe

I get your point about simplifying basic Baritone integration, but I'm not sure whether creating a new type of component just for this purpose is the way to go. I think providing just a concrete implementation of IBaritoneProcess is enough and the factory method for the one-call setup should be a static method on that class rather than an instance method on Baritone and it should do the registration after the constructor is done.

I'd also suggest choosing a different name since when I first read the title of this pr I though you were working on #3565. Maybe something like SimplePauserProcess.

ZacSharp avatar Sep 06 '23 22:09 ZacSharp