Repeat Task until Condition is met

[MiniDigger]

It would be nice to have repeating tasks in the chain. something like this:

newChain().repeat(c-> c.repeat(c->c.delay(10, TimeUnit.SECONDS).sync(this::updateSigns),6).async(this::updateDB),-1)

would do something like this

while(true){
   for(int i = 0;i<6;i++){
       wait(10, second)
       (sync) updateSigns();
   }
   (async)updateDB();
}

(repeat would take a lambda that does stuff with a new chain that will be executed every time repeat is called) Don't really know if the way I outlined that api is a good thing to do something like that, but I think the general concept of repeating some parts of a chain n (or -1 for endless) times.

[aikar]

This would have to look like there's 2 things really being asked for here.

1) the ability to call something like .chain((c) -> { }) that allows you to insert tasks into the chain as part of a callback, which would have to execute IMMEDIATELY to build the chain.

So that design isn't good, it would need to be:

otherChain = TaskChain.newChain()/*some other steps and don't call execute*/;

TaskChain.newChain().import(otherChain);

So TC can read the tasks out of that and clone them into this one;

Then the 2nd request is, a way to say "run this sequence of tasks X times"

I feel like the repeating part is unnecessary as it can be calculated easily by the caller as:

TaskChain chain = TaskChain.newChain();
for (int i = 0; i < 6; i++) {
    chain.import(otherChain); // or could be chain.sync(this.updateDB); to make this template not even needed.
    chain.delay(10, TimeUnit.SECONDS);
}
chain.async(this::updateDB).execute();
chain.execute();

So yeah i don't see repeating as a beneficial API. But using another chain as a template, maybe so.

[aikar]

Oh I just noticed the infinite loop outer layer.

That's not something i want to do with TC, as TC needs the ability to shut down, and there's no clear way to define how to shut that down.

Thats something easy to just wrap in Bukkits repeating task outer wrapper, then TC control the inner actions.

[t7seven7t]

I disagree with ignoring the infinite loop use case. I think it could be useful for creating tasks that loop a number of times before moving onto other tasks. I've created a fork which I think achieves a reasonable compromise for repeating functions. Let me know if you think it's worth creating a PR for: https://github.com/t7seven7t/TaskChain/tree/Repeat

Here's an example of how it might be useful:

newChain()
        .<Integer>syncUntil(r -> ++r, r -> r > 10, 1, TimeUnit.SECONDS ) // increments up til 10
        .sync(r -> r * 5) // multiply by 5 => 50
        .<Integer>syncUntil(r -> r - 2, r -> r < 30, 1, TimeUnit.SECONDS) // reduce by 2 until <30
        .execute();

Previously a similar task chain would require a lot more lines of code and this even allows arbitrarily changing end conditions.

Regarding your comment about shutting down, the other tasks don't really have a clear way defined for shutting them down either mid-task. They kind of just terminate and throw an error presumably.

[aikar]

Hmm, I do like how you pulled this off, and I can see us landing something like that.

Go ahead and open a PR, but I won't be able to review/deal with it until a few weeks, unless i find time here and there.

But please ensure there's a version of each method with "Game Units" too (w/o the TimeUnit)

But i still need to study the change a bit more before full sign off, but overall looks good and clean and i think that can fit with TC's design since it does account for shutdown.

[aikar]

also @t7seven7t as for other tasks shutdown, the shutdown process does currently block until each task is finished, and then it "runs to completion"

So I would say make sure that when the shutdown flag gets triggered for this, that it doesnt abort the chain. I think when the chain shuts down, the untilIf should get one more call, to enable the caller to force flush anything they need to before process before the chain proceeds.

After the untilIf returns (they need to flush in that method on shutdown), then the chain should continue as the current shutdown code does.

The shutdown process stops all thread switching and delays, and runs every task to end in 1 operation.

It may be more applicable for us to add a TaskChain.isShutDown() that does return currentChain.factory.shutdown, and document in until methods that they MUST check if the chain is shutting down in the if method and act accordingly.

[DenWav]

@aikar I think a repeating chain works as long as the repetition is based on a BooleanSupplier. Sure, if you wanted a

while (true) {}

Then you could just do .repeat(() -> true), but that's just no different than while (true) {}. It allows the task to check some condition for whether it should continue or not, allowing it to shutdown.

[t7seven7t]

Okay I've added some new methods for game ticks and javadocs to my branch #https://github.com/t7seven7t/TaskChain/commit/f717e7073aa5f884be4e07d01cee75a3485f319a.

I think regarding the shutdown there are few issues I can see. At the moment when async delay tasks are aborted they prevent the rest of the chain from completing because AbortChainExceptions are thrown which then abort the whole chain rather than running every task to completion without delays. Sync tasks will depend on the GameInterface implementation.

Also I think I've found potential concurrency issues in my code that I'll need to fix later. When a syncUntil task runs it may sleep on an async thread and then never puts it back on the main thread. Same thing for currentUntil.