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 8 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 Gambling

Interested in advertising your business? Reach out today!

Download the DreamBot client today!
luminlumin

llWoodCutter [Progressive] 1 - 60

Recommended Posts

You may want to change how you have the get area and axes does not really make sense to have them public if you never use them and just have them copied and pasted in each of your classes. 

Edited by Towlie

Share this post


Link to post
Share on other sites

You may want to change how you have the get area and axes does not really make sense to have them public if you never use them and just have them copied and pasted in each of your classes. 

 

Ok, I will do that and upload the fix soon.

Share this post


Link to post
Share on other sites

        if (getBank().isOpen()) {
            getBank().depositAllExcept(pickAxeToUse());
            withdrawAxe();
            sleepUntil(() -> getInventory().contains(pickAxeToUse()), 3000);
            getBank().close();
        } else {
            getBank().open();
            if (getBank().isOpen()) {
                getBank().depositAllExcept(pickAxeToUse());
                withdrawAxe();
                getBank().close();
            }
        }

:thinking:

    public Area getWoodcuttingArea() {
        if (getSkills().getRealLevel(Skill.WOODCUTTING) < 16) {
            return TREE_AREA;
        } else if (getSkills().getRealLevel(Skill.WOODCUTTING) < 31) {
            return OAK_AREA;
        } else if (getSkills().getRealLevel(Skill.WOODCUTTING) < 61) {
            return WILLOW_AREA;
        } else {
            return YEW_AREA;
        }
    }

    public String getTreeToCut() {
        if (getSkills().getRealLevel(Skill.WOODCUTTING) < 16) {
            return "Tree";
        } else if (getSkills().getRealLevel(Skill.WOODCUTTING) < 31) {
            return "Oak";
        } else if (getSkills().getRealLevel(Skill.WOODCUTTING) < 61) {
            return "Willow";
        } else {
            return "Yew";
        }
    }

bruh...

These values should be stored in an enum ordered from yew -> normal that holds the name, lvl, and area.  Then you can just have a method like this in the enum: 

public enum Tree {
YEW("Yew", 60, new Area()), /*OAK WILLOW*/ COMMON("Tree", 1, new Area()),;

private String name;
private int lvl;
private Area area;

public static Tree getTreeForLvl(int lvl) {
for (Tree tree : Tree.values()) {
if (lvl >= tree.getLvl()) {
return tree;
}
}

return COMMON;
}

Tree(String name, int lvl, Area area){
this.name = name;
this.lvl = lvl;
this.area = area;
}

public String getName() {
return name;
}
public int getLvl(){
return lvl;
}
public Area getArea() {
return area;
}
}

the same could be applied to axes.  

 

 

Overall a good job for a beginner script.  A lot of your logic is unnecessary and can be simplified.  For example, the nodes GoToTrees and Chop can be combined as well as GoToBank and Bank.  Your bank method is overly complicated.  Try to keep interaction to 1 action per loop.  

 

example:

 

https://gist.github.com/dQw4w9WgXcQ/d9673975c6ac9a3649f45ad58dda4b5a

 

 

 

You should make your own tasknode implementation that can handle more than 1 node depth.  

You may want to change how you have the get area and axes does not really make sense to have them public if you never use them and just have them copied and pasted in each of your classes. 

 

theres no reason these methods need to be private

Edited by rickk

Share this post


Link to post
Share on other sites
        if (getBank().isOpen()) {
            getBank().depositAllExcept(pickAxeToUse());
            withdrawAxe();
            sleepUntil(() -> getInventory().contains(pickAxeToUse()), 3000);
            getBank().close();
        } else {
            getBank().open();
            if (getBank().isOpen()) {
                getBank().depositAllExcept(pickAxeToUse());
                withdrawAxe();
                getBank().close();
            }
        }

:thinking:

    public Area getWoodcuttingArea() {
        if (getSkills().getRealLevel(Skill.WOODCUTTING) < 16) {
            return TREE_AREA;
        } else if (getSkills().getRealLevel(Skill.WOODCUTTING) < 31) {
            return OAK_AREA;
        } else if (getSkills().getRealLevel(Skill.WOODCUTTING) < 61) {
            return WILLOW_AREA;
        } else {
            return YEW_AREA;
        }
    }

    public String getTreeToCut() {
        if (getSkills().getRealLevel(Skill.WOODCUTTING) < 16) {
            return "Tree";
        } else if (getSkills().getRealLevel(Skill.WOODCUTTING) < 31) {
            return "Oak";
        } else if (getSkills().getRealLevel(Skill.WOODCUTTING) < 61) {
            return "Willow";
        } else {
            return "Yew";
        }
    }

bruh...

These values should be stored in an enum ordered from yew -> normal that holds the name, lvl, and area.  Then you can just have a method like this in the enum: 

public enum Tree {
YEW("Yew", 60, new Area()), /*OAK WILLOW*/ COMMON("Tree", 1, new Area()),;

private String name;
private int lvl;
private Area area;

public static Tree getTreeForLvl(int lvl) {
for (Tree tree : Tree.values()) {
if (lvl >= tree.getLvl()) {
return tree;
}
}

return COMMON;
}

Tree(String name, int lvl, Area area){
this.name = name;
this.lvl = lvl;
this.area = area;
}

public String getName() {
return name;
}
public int getLvl(){
return lvl;
}
public Area getArea() {
return area;
}
}

the same could be applied to axes.  

 

 

Overall a good job for a beginner script.  A lot of your logic is unnecessary and can be simplified.  For example, the nodes GoToTrees and Chop can be combined as well as GoToBank and Bank.  Your bank method is overly complicated.  Try to keep interaction to 1 action per loop.  

 

example:

 

https://gist.github.com/dQw4w9WgXcQ/d9673975c6ac9a3649f45ad58dda4b5a

 

 

 

You should make your own tasknode implementation that can handle more than 1 node depth.  

 

theres no reason these methods need to be private

 

Ehh, guess I worded it kind of weird, I was trying to say either make them private (unnecessary, but would need to use a getter) or just use the public aspect of them.

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...