first bot ive written (Ash Picker)

[bikemano 0]

so this is my first bot ive written, more of just messing around with codes and figuring out how things work, the idea is on start it goes to the ge, then looks for ashes to pick up, and banks when the inventory is full... not the fastest, or most optimized... and there's a couple bugs.

Any recommendations would be helpful

 

Quote

import org.dreambot.api.methods.Calculations;
import org.dreambot.api.methods.item.GroundItems;
import org.dreambot.api.methods.map.Area;
import org.dreambot.api.script.AbstractScript;
import org.dreambot.api.script.ScriptManifest;
import org.dreambot.api.script.Category;
import org.dreambot.api.wrappers.items.GroundItem;


@ScriptManifest(author = "Bikemano", name = "Ash Picker", version = 1.0, description = "Picks up Ash", category = Category.MONEYMAKING)
public class main extends AbstractScript {

    Area area = new Area(3143, 3513, 3186, 3468);


    public void onStart() {
        log("starting Script");
        if (!area.contains(getLocalPlayer())); {
        getWalking().walk(area.getRandomTile());}
    }

    public void onExit() {

    }

    @Override
    public int onLoop() {
       if (getInventory().isFull()) {
           log("banking");
           getBank().open();
           sleep(Calculations.random(500, 1000));
           getBank().depositAllItems();
           sleep(Calculations.random(500, 750));
           getBank().close();
       }
       else {
           log("Looking");
           GroundItem ashes = getGroundItems().closest("Ashes");
           if (ashes != null) {
               if (ashes.interact("Take")){
                   log("Picking up");
                       sleepUntil(() -> !ashes.exists(),15000);
                   }
               }
           }



        return Calculations.random(1000, 2000);
    }
}

 

[TheCloakdOne 389]

looks good dude, i would suggest changing the sleeps in your bank method to a sleepUntil, should make it feel a bit more responsive. Might be worth moving around the GE area to ensure your not missing any ashes off screen?

 

sleepUntil(() -> getBank.isOpen(), 1200); //Wait for bank to open
sleepUntil(() -> getInventory.isEmpty(), 1200); //Wait for all items to be deposited (careful of bank space/unbankable items with this one)
sleepUntil(() -> !getBank.isOpen(), 1200); //Wait for bank to close

 

[bikemano 0]

thanks, and yeah the clicking off screen is one of the problems i had, also clicking out of the area

[TheCloakdOne 389]

Ah for that use something like (im a bit lit so it might not compile):

 

Item ashes = getGroundItems().closest("Ashes");
if(ashes == null) {
	return 600;
}

//If its not on the screen then make it so
if (!ashes.isOnScreen()) {
			//If its far away, walk closer
			if(ashes.getDistance(getLocalPlayer().getTile()) > 8) {
				getWalking.walkTo(ashes.getTile().getArea(5));
				return 600;
			}

			//Check its not on the screen and rotate to it
            getCamera().rotateToTile(ashes.getTile());
            sleepUntil(ashes::isOnScreen, 600);
}

if(!ashes.Interact("Take")) {
	log("Unable to pick up ashes");
	return 600;
}

//Ashes picked up

 

[Stoned 52]

 if (!area.contains(getLocalPlayer())); {
        getWalking().walk(area.getRandomTile());}
    }

The above is incorrect, it should be:

 if (!area.contains(getLocalPlayer())) {
        getWalking().walk(area.getRandomTile());
		}
    }

Also you should declare your variables globally then rewrite them as needed, rather than declare them each loop as this is inefficient:

before onLoop():

GroundItem ashes;

Then change: 

GroundItem ashes = getGroundItems().closest("Ashes");

To this:

ashes = getGroundItems().closest("Ashes");

You could also call a getWalking().walk(area.getRandomTile()) as an else statement after the ashes null check. Then add a sleepUntil character is standing still, and it will mean it will run around to scan the GE if it cannot find any ashes.

getWalking().walk(area.getRandomTile()));
sleepUntil(() -> getLocalPlayer().isStandingStill(), Calculations.random(6500, 9000);