Yo, I just finished my power mining script.
Looking for advice on how I can make it better.
I want advice focused on how I can do things better, however additions for the script are appreciated too!

I'm not using the banking and world hop part of the script.
I initially wanted to, then decided not to.
I just left it there in case I change my mind.

Thanks again .

import org.osbot.rs07.script.Script;
import org.osbot.rs07.script.ScriptManifest;
import org.osbot.rs07.api.filter.AreaFilter;
import org.osbot.rs07.api.map.Area;
import org.osbot.rs07.api.map.constants.Banks;
import org.osbot.rs07.utility.ConditionalSleep;

@ScriptManifest(author = "Lexhanatin", name = "AlKharidMiner", info = "Just an empty script :(", version = 0.1, logo = "")
public final class AlKharidMiner extends Script  {
	Area miningArea = new Area(
		    new int[][]{
		        { 3296, 3312 },
		        { 3295, 3312 },
		        { 3294, 3311 },
		        { 3293, 3310 },
		        { 3294, 3309 },
		        { 3295, 3308 },
		        { 3296, 3308 }
		    }
		);
	
    @Override
    public final int onLoop() throws InterruptedException {
    	
    	if (canMine()) {
    		if (!isMining()) {
    			mine();
    		}
    	}
    	
    	else if (inventory.isFull()) {
    		dropAll();
    	}
        return random(150, 200);
    }
    
    private void mine() {
    	
    	if (!miningArea.contains(myPosition())) {
    		getWalking().webWalk(miningArea);
    	}
    	
    	// Not used
    	/*
    	else if (isAreaOccupied()) {
    		worldSwitch();
    	}
    	*/
    	
    	else if (mineRock()) {
    		new ConditionalSleep(800, 1500) {
    			@Override
    			public boolean condition() throws InterruptedException {
    				return isMining();
    			}
    		}.sleep();
    	}
    	else if (isMining()) {
    		new ConditionalSleep(800, 1500) {
    			@Override
    			public boolean condition() throws InterruptedException {
    				return !isMining();
    			}
    		}.sleep();
    	}
    }
    
    private boolean canMine() {
    	
    	return getInventory().contains("Rune pickaxe") || equipment.isWieldingWeapon("Rune pickaxe") && !inventory.isFull();
    }
    
 // Not used
    private boolean isAreaOccupied() {
    	return getPlayers().filter(new AreaFilter<>(miningArea)).size() >= 2;
    }
    
    // Not used
    public boolean worldSwitch() {
    	return worlds.hopToF2PWorld();
    }
    
    private boolean isMining() {
    	return myPlayer().isAnimating();
    }
    
    private boolean mineRock() {
    	//ID 7488, 7455 (2)
    	return getObjects().closest(miningArea,7488, 7455).interact("Mine");
    }
    
    private void dropAll() throws InterruptedException {
    	if (inventory.isFull()) {
    		inventory.dropAllExcept("Rune pickaxe");
    	}
    }
    
    // Not used
    private void bank() throws InterruptedException {
    	
    	if (!Banks.DUEL_ARENA.contains(myPosition())) {
    		getWalking().webWalk(Banks.DUEL_ARENA);
    	}
    	
    	else if (!getBank().isOpen()) {
    		getBank().open();
    	}
    	
    	else if (!getInventory().isEmptyExcept("Rune pickaxe") || equipment.isWieldingWeapon("Rune pickaxe")) {
    		getBank().depositAll();
    	}
    	
    	else if (getBank().contains("Rune pickaxe")) {
    		getBank().withdrawAll("Rune pickaxe");
    		sleep(random(200, 400));
    	}
    }
}

 

going to use it right now and send a proggy your way

 

Sometimes you use getInventory() and sometimes you use inventory. It looks very odd, and kind of unclean IMO.

I like that you used methods such as canMine() for cleanliness but for some like worldSwitch() is it really needed?

Read over https://osbot.org/api/org/osbot/rs07/utility/ConditionalSleep.html#ConditionalSleep-int-int- again.

In Java, constants like your Area should be static and final and capitalized like

final static Area MINING_AREA = new Area(new int[][] { { 3296, 3312 }, { 3295, 3312 }, { 3294, 3311 }, { 3293, 3310 }, { 3294, 3309 }, { 3295, 3308 }, { 3296, 3308 } });

You could also make it private.

Pretty decent otherwise.

Hey, thats pretty good 

 

Il try get a proggy brb

use conditional sleeps

Use color for the rocks instead of ID

3 minutes ago, Funfight said:

how do you do that? getColor?

read the API

On 30/06/2017 at 6:27 PM, Funfight said:

how do you do that? getColor?

        Entity rock = script.getObjects().closest(
                obj -> obj.getName().equals("Rocks") && obj.getDefinition().getModifiedModelColors()[0] == colour
        );