Jump to content

Welcome to DreamBot!

Download for Free

Supercharge Your Bots

Run unlimited bots today using DreamBot's Covert Mode and
stay more protected.

Upgrade Now
Frequently Asked Questions
  • Are you not able to open the client? Make sure you have Java installed
  • Still not working? Try downloading and running JarFix
  • Help! My bot doesn't do anything! Enable fresh start in client settings and restart the client
  • How to purchase with PayPal/OSRS gold? You can purchase vouchers from other users
  • Try asking for help in the chatbox
osrs gold
Bottinghub.com

Interested in advertising your business? Reach out today!

mshackertr2

Constructive Criticism on my Woodcutting Bot

Recommended Posts

package main;

import org.dreambot.api.methods.Calculations;
import org.dreambot.api.methods.map.Area;
import org.dreambot.api.script.AbstractScript;
import org.dreambot.api.script.Category;
import org.dreambot.api.script.ScriptManifest;
import org.dreambot.api.wrappers.interactive.GameObject;
import java.awt.*;

/**
 * Created by Jarod on 3/6/2020.
 */
@ScriptManifest(category = Category.WOODCUTTING, name = "Basic", author = "Jarod", version = 1.0)
public class Main extends AbstractScript {

    Area bankArea = new Area(1589, 3475, 1593, 3476);
    Area treeArea = new Area(1577, 3480, 1581, 3493);

    @Override
    public void onStart() {
        log("Hi");

    }

    @Override
    public int onLoop() {

        if (!getInventory().isFull()) {
            if (treeArea.contains(getLocalPlayer())) {
                chopTree("Magic tree");
            } else {
                if (getWalking().walk(treeArea.getRandomTile())) {
                    sleep(Calculations.random(3000, 5500));
                    if (treeArea != null) {
                        log("treeArea Chest is not null.");
                    } else {
                        log("treeArea Chest is null.");
                    }
                }
            }
        }

        if (getInventory().isFull()) {
            if (bankArea.contains(getLocalPlayer())) {
                bank();
            } else {
                if (getWalking().walk(bankArea.getRandomTile())) {
                    sleep(Calculations.random(3000, 6000));
                    if (bankArea != null) {
                        log("BankArea Chest is not null.");
                    } else {
                        log("BankArea Chest is null.");
                    }
                }
            }
        }

        return 600;
    }

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

    @Override
    public void onPaint(Graphics graphics) {

    }

    private void chopTree(String treeName) {
        GameObject tree = getGameObjects()
                .closest(gameObject -> gameObject != null && gameObject.getName().equals(treeName));
        if (tree != null && tree.interact("Chop down")) {
            int countLog = getInventory().count("Logs");
            sleepUntil(() -> getInventory().count("Logs") > countLog, 12000);
        }
    }

    private void bank() {
        GameObject chest = getGameObjects().closest(c -> c != null && c.getName().contains("Bank chest") && c.hasAction("Use"));
        if (!getBank().isOpen()) {
            if (chest != null) {
                if (chest.interact("Use")) {
                    sleepUntil(() -> getBank().isOpen(), 2000);
                }
            }
        } else {
           if (getBank().depositAll(item -> item != null && item.getName().contains("Magic logs"))) {
                sleepUntil(() -> !getInventory().contains(item -> item != null && item.getName().contains("Magic logs")), 2000);
            }
        }

    }
}

Share this post


Link to post
Share on other sites

ok I will take a shot at it

if (!getInventory().isFull()) {
	//code
}

if(getInventory().isFull()){
	//code
}

You could just have else there

if (treeArea != null) {
	log("treeArea Chest is not null.");
} else {
	log("treeArea Chest is null.");
}

treeArea is your own variable that you define at the top, how could it be null?

if (getWalking().walk(treeArea.getRandomTile()))

you don't need to get a new random tile every time you call the walk function

@Override
public void onPaint(Graphics graphics) {

}

Why are you posting an empty function?

sleepUntil(() -> getInventory().count("Logs") > countLog, 12000);

if somebody else gets the tree before you the bot will just stand there until the sleepUntil times out

also sleepUntil is a performance hog

GameObject chest = getGameObjects().closest(c -> c != null && c.getName().contains("Bank chest") && c.hasAction("Use"));

Your code is written to only handle chests, why? There are 4 types of banks as you can see here

https://dreambot.org/javadocs/org/dreambot/api/methods/container/impl/bank/BankType.html

so let DreamBot handle this for you and write something like

BankLocation anyTypeOfBank = getBank().getClosestBankLocation();
getBank().open(anyTypeOfBank);

that way it will open the nearest bank regardless of the type

if (chest.interact("Use")) {
	sleepUntil(() -> getBank().isOpen(), 2000);
}
sleepUntil(() -> !getInventory().contains(item -> item != null && item.getName().contains("Magic logs")), 2000);

You use sleepUntil to check if the bank is open and also to check if the logs were already deposited. That's pretty expensive way of doing it.

while(!getBank().isOpen()){
	sleep(500);
}

This is essentially the same thing but without the performance hit, although it really only matters if you intend to run many bots.

Share this post


Link to post
Share on other sites

first remove that bankarea( we can reach it by applying the web walker )

i'd focus on seperating task1- chopTree & task2 -  walk from node1 - by state1 - haveLogs()

Switch(!haveLogs())  //State1 condition

then it's Task validation logic: 

Case true:
	if(treeArea.contains(getLocalPlayer()))  //TaskNode1 validation ; doubles as TaskNode 2's validation
		chopTree("Magic tree")   //TaskNode1 execution
    else getWalking().walk(treeArea);  //TaskNode2 execution
Case false:           

	BankLocation anyTypeOfBank = getBank().getClosestBankLocation(); getBank().open(anyTypeOfBank); //Node1 Execution

 

 

Edited by DefCon

Share this post


Link to post
Share on other sites
BankLocation anyTypeOfBank = getBank().getClosestBankLocation(); getBank().open(anyTypeOfBank); //Node1 Execution

 

Careful with this line if you are botting around Varrock, under certain cases it will try and bank at the cooks guild even if you dont have the cooking level. Ideally whitelist a set of approved banks and only let it check against them:

 

    List<BankLocation> approvedBanks = Arrays.asList(BankLocation.VARROCK_EAST, BankLocation.VARROCK_WEST, BankLocation.GRAND_EXCHANGE);

    //Walk to bank
    BankLocation closestBank = this.getClosestBank();
    getBank().open(closestBank)
      
    //Get closest bank
    public static BankLocation getClosestBank() {
        double shortestDistance = 90000;
        BankLocation closestBank = null;

        for (BankLocation bank : approvedBanks) {
            if (bank.getCenter().distance(getLocalPlayer().getTile()) < shortestDistance) {
                shortestDistance = bank.getCenter().distance(getLocalPlayer().getTile());
                closestBank = bank;
            }
        }

        //If nothing is closest, go to first bank in list
        if (closestBank == null) {
            closestBank = approvedBanks.get(0);
        }

        log("Closest bank is: " + closestBank + " - shortestDistance away");
        return closestBank;
    }

 

Share this post


Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.


×
×
  • Create New...