First script feedback

[Chicken Wing]

Hey, I've been trying to get familiar with the api and wrote a simple fletching script, it would be great if some more experienced scripters could give some feedback 

package dev;

import org.osbot.rs07.api.model.Entity;
import org.osbot.rs07.script.Script;
import org.osbot.rs07.script.ScriptManifest;
import org.osbot.rs07.utility.ConditionalSleep;

import util.SmartKeyboard;

import java.awt.*;

@ScriptManifest(author = "You", info = "My first script", name = "Fletcher", version = 0.1, logo = "")
public class LogCutter extends Script {
	
	private boolean _hasKnife = false;
	private final static int LOG_ID = 1515;
	private final static int BOW_u_ID = 66;

	@Override
	public void onStart() {
		if(inventory.contains(946)){
			_hasKnife = true;
		}
		log("Script started");
		getMouse().setSpeed(1);
		
	}

	private enum State {
		WITHDRAW_LOGS, CUT_LOGS, WAIT, BANK_BOWS, WITHDRAW_KNIFE, RECOVER_UNEXPECTED_STOP
	};

	private State getState() {
		if(!inventory.contains(946)){
			return State.WITHDRAW_KNIFE;
		}
		
		if(_hasKnife && inventory.isEmptyExcept(946)){
			return State.WITHDRAW_LOGS;
		}
		
		if(_hasKnife && inventory.getAmount(1515) == 27){
			return State.CUT_LOGS;
		}
		
		if(_hasKnife && inventory.getAmount(66) == 27){
			return State.BANK_BOWS;
		}
		
		if(_hasKnife && inventory.contains(LOG_ID) && !myPlayer().isAnimating()){
			return State.RECOVER_UNEXPECTED_STOP;
		}
		
		return State.WAIT;
		
		

	}

	@Override
	public int onLoop() throws InterruptedException {
		switch (getState()) {
		case BANK_BOWS:
			openBank();
			depositBows(BOW_u_ID);
			break;
			
		case CUT_LOGS:
			cutLongBows(LOG_ID);
			break;
			
		case WAIT:
			break;
			
		case WITHDRAW_KNIFE:
			openBank();
			withdrawKnife();
			break;
			
		case WITHDRAW_LOGS:
			openBank();
			withdrawLogs(LOG_ID);
			break;
			
		case RECOVER_UNEXPECTED_STOP:
			cutLongBows(LOG_ID);
			break;
			
			
		}

		return random(200, 300);
	}

	// private methods -------------------------------------

	private void withdrawLogs(int id) throws InterruptedException {
		if (this.bank.isOpen() && bank.contains(id)) {
			bank.withdrawAll(id);
		}
		new ConditionalSleep(5000) {

			@Override
			public boolean condition() throws InterruptedException {
				return inventory.getAmount(id) == 27;
			}
		}.sleep();
		bank.close();
		new ConditionalSleep(5000) {

			@Override
			public boolean condition() throws InterruptedException {
				return !bank.isOpen();
			}
		}.sleep();
		sleep(random(290, 380));
		
	}

	private void withdrawKnife() throws InterruptedException {
		if (this.bank.isOpen() && bank.contains(946)) {
			bank.withdraw(946, 1);
		}
		new ConditionalSleep(5000) {

			@Override
			public boolean condition() throws InterruptedException {
				return inventory.getAmount(946) == 1;
			}
		}.sleep();
		_hasKnife = true;
		sleep(random(290, 380));
		
	}

	private void openBank() throws InterruptedException {
		if (!this.bank.isOpen()) {
			Entity bankBooth = objects.closest("Bank booth");
			if (bankBooth != null) {
				bankBooth.interact("Bank");
			}
			new ConditionalSleep(5000) {

				@Override
				public boolean condition() throws InterruptedException {
					return bank.isOpen();
				}
			}.sleep();
		}
		sleep(random(280, 390));
		
	}
	

	private void depositBows(int id) throws InterruptedException {
		if (this.bank.isOpen() && bank.contains(id)) {
			bank.depositAll(id);
		}
		new ConditionalSleep(5000) {

			@Override
			public boolean condition() throws InterruptedException {
				return inventory.getAmount(id) == 0;
			}
		}.sleep();
		sleep(random(290, 380));
		
	}
	
	private void cutLongBows(int id) throws InterruptedException {
		if(inventory.contains(id)){
			inventory.getItem(946).interact("Use");
			new ConditionalSleep(5000) {

				@Override
				public boolean condition() throws InterruptedException {
					return inventory.isItemSelected();
				}
			}.sleep();
			
			inventory.interact("Use", id);
			
			new ConditionalSleep(5000) {

				@Override
				public boolean condition() throws InterruptedException {
					return dialogues.isPendingOption();
				}
			}.sleep();
			
			sleep(random(280, 390));
			widgets.get(304, 10).interact("Make X");
			sleep(random(400, 700));
			new SmartKeyboard(this, 65).typeString("33", true, false);
			sleep(random(280, 390));
			getMouse().moveOutsideScreen();
			new ConditionalSleep(100000) {

				@Override
				public boolean condition() throws InterruptedException {
					return inventory.getAmount(BOW_u_ID) == 27;
				}
			}.sleep();
			sleep(random(280, 390));
		}
		
	}
	
	//  ---------------------------------------------------------------------------

	@Override
	public void onExit() {
		log("Script stopped");
	}

	@Override
	public void onPaint(Graphics2D g) {

	}

}

[Bobrocket]

new SmartKeyboard(this, 65)

 

A few tips would be to use names and not IDs, as well as checking visibility of widgets before you interact with them. Looks solid otherwise, as long as it works!

 

EDIT: you may want to keep the SmartKeyboard object global instead of making a new one every time.

Edited August 20, 2015 by Bobrocket

[Chicken Wing]

new SmartKeyboard(this, 65)

 

A few tips would be to use names and not IDs, as well as checking visibility of widgets before you interact with them. Looks solid otherwise, as long as it works!

 

 

I was having some problems with the default keyboard so I used your one, its very nice . And yeah thats an oopsy for not checking if its visible. Also if I add paint, and maybe an option to input the string of the log and some other stuff, would this be SDN worthy? (free ofc)

[Bobrocket]

I was having some problems with the default keyboard so I used your one, its very nice . And yeah thats an oopsy for not checking if its visible. Also if I add paint, and maybe an option to input the string of the log and some other stuff, would this be SDN worthy? (free ofc)

 

I don't see why not

[Tom]

As Bob has said, using ID's can cause problems in the future as they can potentially change, you should also lower the amount of states you have,

WITHDRAW_KNIFE, and WITHDRAW_LOGS dont really need to be seperate, you could have one state called WITHDRAW and have a few conditionals in there.