Jump to content
Frequently Asked Questions
  • Are you not able to open the client? Try following our getting started guide
  • 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/Crypto gold? You can purchase vouchers from other users
  • My first script - code reveiw


    Theorems

    Recommended Posts

    Hello scripters,


     


    I am halfway through my first java course so I figured I had enough basic knowledge to create a script. I decided to make a sheep shearer with the goal of making it very efficient by conserving a certain amount of run energy for when it reaches the sheep and by trying to optimize re-shear time (it is not that efficient yet). 


     


    I wanted to post my code and see if some of the more experienced scripters would give me feedback so I don't develop bad habits early on, or if I am using the wrong methods for something to let me know. Any feedback is greatly appreciated.


     


    for running: start with shears in inventory near lumbridge.


     


    edit: improved shearing interaction - it actually seems to be fairly efficient now




    import org.dreambot.api.methods.Calculations;
    import org.dreambot.api.methods.map.Area;
    import org.dreambot.api.methods.map.Tile;
    import org.dreambot.api.script.AbstractScript;
    import org.dreambot.api.script.ScriptManifest;
    import org.dreambot.api.wrappers.interactive.GameObject;
    import org.dreambot.api.wrappers.interactive.NPC;
    import org.dreambot.api.wrappers.interactive.Player;
    import org.dreambot.api.wrappers.items.Item;
    import org.dreambot.api.script.Category;
    import org.dreambot.api.methods.container.impl.bank.BankType;
    import org.dreambot.api.methods.container.impl.bank.Bank;

    import java.awt.*;
    import java.util.Arrays;
    import java.util.List;


    @ScriptManifest(author = "Theorems", category = Category.MONEYMAKING, name = "Theorems", version = 1.0, description = "Shears sheep at lumbridge")

    public class Driver extends AbstractScript
    {
    /*
    * States:
    * 1 - shear sheep
    * 2 - bank wool
    */
    private int state = 1;
    private int wool = 0;
    int timeout = 0;
    int inventoryRemaining;

    private Area R_lumbridgeTower = new Area(3206, 3227, 3212, 3228);
    private Area Bank = new Area(3207, 3220, 3210, 3218);
    private Area sheepPen = new Area(3212, 3258, 3193, 3276);
    private Area topFloor = new Area(3205, 3228, 3207, 3225);

    public void onStart()
    {
    //log("Theorem's sheeps has started");
    inventoryRemaining = getInventory().getEmptySlots();
    getWalking().setRunThreshold(100);
    }

    @Override
    public int onLoop()
    {
    timeout = 0;
    if (state == 1)
    {
    shear();
    }
    else if (state == 0)
    {
    bank();
    }
    // return value is small, but there are other sleeps throughout the script
    return Calculations.random(200, 600);
    }

    @Override
    public void onPaint(Graphics g)
    {
    g.setFont(new Font("Arial", Font.BOLD, 14));
    g.setColor(Color.GREEN);
    g.drawString("Wool Deposited: " + wool, 25, 40);

    }

    private void bank()
    {
    // Conserve run energy while banking
    //run gets turned on when at 100% but the bot saves 40-60% for sheeps
    if (getWalking().isRunEnabled())
    {
    if (getWalking().getRunEnergy() < Calculations.random(40, 60))
    getWalking().toggleRun();

    }
    // --STATE-- full inventory, ground-level, not in tower --STATE--
    if (getInventory().isFull() && getLocalPlayer().getZ() == 0
    && !R_lumbridgeTower.contains(getLocalPlayer()))
    {
    walkDelay();
    getWalking().walkExact(R_lumbridgeTower.getRandomTile());

    // --STATE-- full inventory, ground-level, in tower --STATE--
    }
    else if (getInventory().isFull() && getLocalPlayer().getZ() == 0)
    {
    //climb first stairs
    GameObject stairs = getGameObjects().closest(16671);
    do
    {
    stairs.interact("Climb-up");
    sleep(Calculations.random(500, 1100));
    }
    while (getLocalPlayer().getZ() == 0);

    // --STATE-- full inventory, second floor --STATE--
    }
    else if (getInventory().isFull() && getLocalPlayer().getZ() == 1)
    {
    //climb second stairs
    GameObject stairs = getGameObjects().closest(16672);
    do
    {
    sleep(Calculations.random(100, 500));
    stairs.interact("Climb-up");
    sleep(Calculations.random(500, 1100));
    }
    while (getLocalPlayer().getZ() == 1);

    // --STATE-- full inventory, third floor --STATE--
    }
    else if (getInventory().isFull() && getLocalPlayer().getZ() == 2)
    {
    //open the bank and deposit the wool
    getBank().open();
    do
    {
    sleep(Calculations.random(3000, 5400));
    getBank().open();
    }
    while (!getBank().isOpen());
    if (getBank().depositAllExcept(1735))
    {
    wool += 27;
    }

    // --STATE-- not full inventory, third floor --STATE--
    }
    else if (!getInventory().isFull() && getLocalPlayer().getZ() == 2)
    {
    getWalking().walk(topFloor.getRandomTile());
    sleep(Calculations.random(1200, 2400));

    GameObject stairs = getGameObjects().closest(16673);
    do
    {
    sleep(Calculations.random(100, 500));
    stairs.interact("Climb-down");
    sleep(Calculations.random(300, 1600));
    }
    while (getLocalPlayer().getZ() == 2);
    sleep(Calculations.random(500, 1100));
    }

    // --STATE-- not full inventory, second floor --STATE--
    else if (!getInventory().isFull() && getLocalPlayer().getZ() == 1)
    {
    GameObject stairs = getGameObjects().closest(16672);
    do
    {
    sleep(Calculations.random(100, 500));
    stairs.interact("Climb-down");
    sleep(Calculations.random(300, 1600));
    }
    while (getLocalPlayer().getZ() == 1);
    sleep(Calculations.random(500, 1100));
    }
    // --STATE-- not full inventory, ground-level --STATE--
    else if (!getInventory().isFull() && getLocalPlayer().getZ() == 0)
    {
    state = 1;
    }
    }


    private void shear()
    {
    if (sheepPen.contains(getLocalPlayer()))
    {
    // Check if we even have shears
    if (getInventory().contains(item -> item.getName().contains("Shears")))
    {
    inventoryRemaining = getInventory().getEmptySlots();

    // Switch to the banking state if our inventory is full
    if (getInventory().isFull())
    {
    state = 0;

    }
    else
    {

    // If we are walking, turn on run to get wool faster
    if (!getWalking().isRunEnabled())
    {
    if (getWalking().getRunEnergy() > Calculations.random(15, 25))
    getWalking().toggleRun();

    }

    // Get the closest sheep
    NPC sheep = getNpcs()
    .closest((s) -> s != null && s.getID() != 731
    && Arrays.asList(s.getActions()).contains("Shear")
    && sheepPen.contains(s));


    //shear the sheep if it exists
    if (sheep != null)
    {

    // if not already busy shearing or walking
    //if (!getLocalPlayer().isInteracting(sheep))
    //{
    sheep.interact("Shear");
    if (getWalking().isRunEnabled())
    {
    timeout = Calculations.random(2000, 3500);
    }
    else
    {
    timeout = Calculations.random(3000, 4500);
    }
    sleepUntil(() -> !getLocalPlayer().isMoving(), timeout);

    // }
    }

    }
    }
    }
    else
    {
    if (getInventory().isFull())
    {
    state = 0;
    }
    else
    {
    walkToSheep();
    }
    }
    }

    private void walkToSheep()
    {
    log("walk to sheepos");
    do
    {
    walkDelay();
    getWalking().walk(sheepPen.getRandomTile());
    }
    while (!sheepPen.contains(getLocalPlayer()));
    }

    private void walkDelay()
    {
    // this method is for sleeping between clicks while running to a location
    // the timeout is set lower for walking.

    if (getWalking().isRunEnabled())
    {
    timeout = Calculations.random(4000, 6500);
    sleepUntil(() -> !getLocalPlayer().isMoving() || getLocalPlayer()
    .distance(getClient().getDestination()) < Calculations.random(2, 8),
    timeout);
    }
    else
    {
    timeout = Calculations.random(6000, 10000);
    sleepUntil(() -> !getLocalPlayer().isMoving() || getLocalPlayer()
    .distance(getClient().getDestination()) < Calculations.random(2, 8),
    timeout);
    }
    }


    }


    Link to comment
    Share on other sites

    A really good job for a first attempt.

     

    Some things you can work on to simplify your logic and prevent bugs when your scripts get more complicated:

     

    Its good practice to only have one interact per loop.  Avoid using while loops to perform the entire process of walking to bank for example.  While this less efficient in an ideal scenario, it is worth it to avoid bugs and for maintainability.  

     

    Don't store a hard state, instead determine the state of your script every loop so it can be started/restarted at any point.  This prevents bugs.  

     

    Your walk to bank logic can be replaced by calling getBank.openClosest() in a loop.  Same with the walk to sheep.  

        @Override
        public int onLoop() {
            if(!getInventory().isFull()){
                if(SHEEP_AREA.contains(getLocalPlayer())){
                    getNpcs().closest(npc -> npc != null && npc.hasAction("Shear")).interact("Shear");
                }else if(getBank().isOpen()){//sometimes the bank can get stuck open at this location
                    getBank().close();
                }else {
                    getWalking().walk(SHEEP_AREA.getRandomTile());
                }
            }else if(getBank().isOpen()){
                getBank().depositAllExcept(SHEARS);
            }else {
                getBank().open(BankLocation.LUMBRIDGE);
            }
            return 1000;
        }
    

    Something like this would be a good place to start.  

     

    Its great to see new promising scripters!

    Link to comment
    Share on other sites

    A really good job for a first attempt.

     

    Some things you can work on to simplify your logic and prevent bugs when your scripts get more complicated:

     

    Its good practice to only have one interact per loop.  Avoid using while loops to perform the entire process of walking to bank for example.  While this less efficient in an ideal scenario, it is worth it to avoid bugs and for maintainability.  

     

    Don't store a hard state, instead determine the state of your script every loop so it can be started/restarted at any point.  This prevents bugs.  

     

    Your walk to bank logic can be replaced by calling getBank.openClosest() in a loop.  Same with the walk to sheep.  

        @Override
        public int onLoop() {
            if(!getInventory().isFull()){
                if(SHEEP_AREA.contains(getLocalPlayer())){
                    getNpcs().closest(npc -> npc != null && npc.hasAction("Shear")).interact("Shear");
                }else if(getBank().isOpen()){//sometimes the bank can get stuck open at this location
                    getBank().close();
                }else {
                    getWalking().walk(SHEEP_AREA.getRandomTile());
                }
            }else if(getBank().isOpen()){
                getBank().depositAllExcept(SHEARS);
            }else {
                getBank().open(BankLocation.LUMBRIDGE);
            }
            return 1000;
        }
    

    Something like this would be a good place to start.  

     

    Its great to see new promising scripters!

    Thanks for the feedback! Yeah, those while loops are a pain because I was realizing if I pause/stop the script when it is banking it doesn't shut down properly and keeps trying to run in a glitchy way. Although it can be initially started at any phase without glitches.

    Link to comment
    Share on other sites

    You should follow convention and put { on the same line as the while, if, do, etc. Also everything inside a loop or if should be indented.

    Link to comment
    Share on other sites

    You should follow convention and put { on the same line as the while, if, do, etc. Also everything inside a loop or if should be indented.

    That's the formatter used in my intro to programming course. I didn't realize it did not follow convention but I guess it would be good to use a formatter that does.

    Link to comment
    Share on other sites

    That's the formatter used in my intro to programming course. I didn't realize it did not follow convention but I guess it would be good to use a formatter that does.

    Using curly braces on new lines is C/C++ convention. If you're using IntelliJ it can auto format your code by pressing Ctrl + Alt + L or on a mac with cmd + option + L

    Link to comment
    Share on other sites

    Archived

    This topic is now archived and is closed to further replies.

    ×
    ×
    • Create New...

    Important Information

    We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.