First Script - looking for advice, criticism, etc

[Sara]

Hey everyone

I decided to try my hand at learning how to write my own scripts, I've been consuming as many tutorials as I can (Thank you everyone who contributes to that section) and reviewing the API during lax points at work, figured I should finally start a project. I started this script this afternoon and am pretty happy with how it turned out, but was looking for any input you all have on how to make it better, where to go from here, any bad practices I may have done that I should avoid, best practices to do, etc.

Main class: 

Spoiler

package core;

import java.util.ArrayList;
import org.osbot.rs07.script.Script;
import org.osbot.rs07.script.ScriptManifest;
import task.BankCabbage;
import task.GetCabbage;
import task.Task;
import task.WalkToBank;
import task.WalkToCabbage;

@ScriptManifest(author = "Sara", info = "Script I'm using to learn Java", name = "SaraCabbage", version = 0, logo = "")

public class Main extends Script {
	ArrayList<Task> tasks = new ArrayList<Task>();
	@Override
	public void onStart() {
		tasks.add(new BankCabbage(this));
		tasks.add(new GetCabbage(this));
		tasks.add(new WalkToBank(this));
		tasks.add(new WalkToCabbage(this));
	}
	@Override
	public int onLoop() throws InterruptedException {
		tasks.forEach(tasks -> tasks.run());
		return 700;
	}
}

 

Task class: 

Spoiler

package task;

import org.osbot.rs07.script.MethodProvider;

public abstract class Task {
	protected MethodProvider api;
	public Task(MethodProvider api) {
		this.api = api;
	}
	public abstract boolean canProcess();
	public abstract void process();
	public void run() {
		if (canProcess())
			process();
	}
}

 

Sleep class: 

Spoiler

package util;

import org.osbot.rs07.utility.ConditionalSleep;

import java.util.function.BooleanSupplier;

public final class Sleep extends ConditionalSleep {

    private final BooleanSupplier condition;

    public Sleep(final BooleanSupplier condition, final int timeout) {
        super(timeout);
        this.condition = condition;
    }

    @Override
    public final boolean condition() throws InterruptedException {
        return condition.getAsBoolean();
    }

    public static boolean sleepUntil(final BooleanSupplier condition, final int timeout) {
        return new Sleep(condition, timeout).sleep();
    }
}

 

WalkToCabbage class: 

Spoiler

package task;

import org.osbot.rs07.api.map.Area;
import org.osbot.rs07.api.map.Position;
import org.osbot.rs07.event.WebWalkEvent;
import org.osbot.rs07.event.webwalk.PathPreferenceProfile;
import org.osbot.rs07.script.MethodProvider;

public class WalkToCabbage extends Task {
	
	private static final Area Cabbage_Area = new Area(3067, 3298, 3044, 3285);
	Position pos1 = new Position(3060, 3286, 0);

	public WalkToCabbage(MethodProvider api) {
		super(api);
	}

	@Override
	public boolean canProcess() {
		//If player inventory *IS NOT* full and player *IS NOT* at the cabbage patch, this evaluates to true
		if(!api.getInventory().isFull() && !Cabbage_Area.contains(api.myPlayer())) {
			return true;
		} else {
			return false;
		}
	}

	@Override
	public void process() {
		//If canProcess condition evaluates to true, do this
		api.log("Walking to Cabbage");
		WebWalkEvent webEvent = new WebWalkEvent(pos1);
		webEvent.useSimplePath();
		PathPreferenceProfile ppp = new PathPreferenceProfile();
		ppp.setAllowTeleports(false);
		webEvent.setPathPreferenceProfile(ppp);
		api.execute(webEvent);
	}
	
}

 

GetCabbage class: 

Spoiler

package task;

import org.osbot.rs07.api.map.Area;
import org.osbot.rs07.api.model.Entity;
import org.osbot.rs07.script.MethodProvider;

import util.Sleep;

public class GetCabbage extends Task {
	
	private static final int[] Cabbage_GroundID = { 1161 };
	private static final Area Cabbage_Area = new Area(3067, 3298, 3044, 3285);
	
	public GetCabbage(MethodProvider api) {
		super(api);
	}
	
	@Override
	public boolean canProcess() {
		//If player inventory *IS NOT* full and player *IS* at the cabbage patch, this evaluates to true
		if(!api.getInventory().isFull() && Cabbage_Area.contains(api.myPlayer())) {
			return true;
		} else {
			return false;
		}
	}
	
	@Override
	public void process() {
		//If canProcess condition evaluates to true, do this
		Entity cabbage = api.getObjects().closest(Cabbage_GroundID);
		if (!api.myPlayer().isMoving() && cabbage != null && cabbage.interact("Pick")) {
		    Sleep.sleepUntil(() -> api.myPlayer().isAnimating() || !cabbage.exists(), 5000);
		}
	}
}

 

WalkToBank class: 

Spoiler

package task;

import org.osbot.rs07.api.map.Area;
import org.osbot.rs07.api.map.Position;
import org.osbot.rs07.event.WebWalkEvent;
import org.osbot.rs07.event.webwalk.PathPreferenceProfile;
import org.osbot.rs07.script.MethodProvider;

public class WalkToBank extends Task {
	
	private static final Area Bank_Area = new Area(3088, 3246, 3097, 3240);
	Position pos1 = new Position(3095, 3241, 0);
	
	public WalkToBank(MethodProvider api) {
		super(api);
	}

	@Override
	public boolean canProcess() {
		//If player inventory *IS* full and player *IS NOT* at the bank, this evaluates to true
		if(api.getInventory().isFull() && !Bank_Area.contains(api.myPlayer())) {
			return true;
		} else {
			return false;
		}
	}

	@Override
	public void process() {
		//If canProcess condition evaluates to true, do this
		api.log("Walking to bank");
		WebWalkEvent webEvent = new WebWalkEvent(pos1);
		webEvent.useSimplePath();
		PathPreferenceProfile ppp = new PathPreferenceProfile();
		ppp.setAllowTeleports(false);
		webEvent.setPathPreferenceProfile(ppp);
		api.execute(webEvent);

	}
	
}

 

BankCabbage class: 

Spoiler

package task;

import org.osbot.rs07.api.map.Area;
import org.osbot.rs07.script.MethodProvider;

public class BankCabbage extends Task {
	
	private static final Area Bank_Area = new Area(3088, 3246, 3097, 3240);
	
	public BankCabbage(MethodProvider api) {
		super(api);
	}

	@Override
	public boolean canProcess() {
		//If player inventory *IS* full and player *IS* at bank, this evaluates to true
		if(api.getInventory().isFull() && Bank_Area.contains(api.myPlayer())) {
			return true;
		} else {
			return false;
		}
	}

	@Override
	public void process() {
		//If canProcess condition evaluates to true, do this
		if (!api.getBank().isOpen()) {
		    try {
				api.log("Depositing cabbages");
				api.getBank().open();
			} catch (InterruptedException e) {
				e.printStackTrace();
			}
		} else {
			api.log("Cabbages have been deposited");
		    api.getBank().depositAll();
		}
	}
}

 

 

Edited July 19, 2019 by Sara

[ammanizzo]

When can we test the scripts xD?

[Hybris]

Definitely looks great for a first script, the only thing I noticed is this:

@Override
	public boolean canProcess() {
		//If player inventory *IS NOT* full and player *IS NOT* at the cabbage patch, this evaluates to true
		if(!api.getInventory().isFull() && !Cabbage_Area.contains(api.myPlayer())) {
			return true;
		} else {
			return false;
		}
	}

 

You can write this way easier:

@Override
	public boolean canProcess() {
		//If player inventory *IS NOT* full and player *IS NOT* at the cabbage patch, this evaluates to true
		return !api.getInventory().isFull() && !Cabbage_Area.contains(api.myPlayer()));
	}

 

If you have any questions feel free to ask

[Sara]

3 hours ago, Hybris said:

Definitely looks great for a first script, the only thing I noticed is this:

@Override
	public boolean canProcess() {
		//If player inventory *IS NOT* full and player *IS NOT* at the cabbage patch, this evaluates to true
		if(!api.getInventory().isFull() && !Cabbage_Area.contains(api.myPlayer())) {
			return true;
		} else {
			return false;
		}
	}

 

You can write this way easier:

@Override
	public boolean canProcess() {
		//If player inventory *IS NOT* full and player *IS NOT* at the cabbage patch, this evaluates to true
		return !api.getInventory().isFull() && !Cabbage_Area.contains(api.myPlayer()));
	}

 

If you have any questions feel free to ask

Thanks for this!

I originally wrote it that way but when I was struggling with tasks (ended up being unrelated) I switched these to if/else in desperation to figure out what I was doing wrong . You’re 100% right it’s absolutely much simpler and aesthetically pleasing how you wrote, and I’ll be switching back to that moving forward.

 

Thanks again!

[Sara]

4 hours ago, ammanizzo said:

When can we test the scripts xD?

You should be able to compile it as is, I can see about uploading the script file though if you’re unable to do that? Let me know

[Ragboys is back]

Pretty good for a first script, good job

[ExtraBotz]

Nice work! You should add comments to your code to make it easier to read.

May I ask why you use specific positions for walking? Not that I know any better, but I typically use something like getWalking.webWalk(area); which automatically finds a random point within the area. I do this so that the walking stays unpredictable.

My personal belief is that after your script is ran 1, 100, 1000+ times then patterns start to develop if you use static information.

[Sara]

28 minutes ago, ExtraBotz said:

Nice work! You should add comments to your code to make it easier to read.

May I ask why you use specific positions for walking? Not that I know any better, but I typically use something like getWalking.webWalk(area); which automatically finds a random point within the area. I do this so that the walking stays unpredictable.

My personal belief is that after your script is ran 1, 100, 1000+ times then patterns start to develop if you use static information.

I actually didn't realize that you can use areas with webWalk, and since I already have the cabbage patch and bank configured as defined areas, that would simplify the code even further, I'll go ahead and give that a shot, thank you! (Edit: Implemented )

 

1 hour ago, Ragboys is back said:

Pretty good for a first script, good job

Thanks!

Edited July 19, 2019 by Sara

[Reminiscence]

I've honestly never seen such a simple script spread out so much, but at least you're getting the fundamentals down and the more difficult scripts should be easier to maintain.