[Script Help Please] Is there a better way to optimize this code?

Lol_marcus · May 7, 2020

Hi again all,

I'm working on a simple superglass maker. I've noticed that it's much more human-like and definitely more efficient to have it withdraw the giant seaweed by clicking it 3 times, rather than every time right click -> amount -> 3, and then the same thing for the buckets of sand. My script ended up like this, but is there a cleaner more practical way of doing this, or is the way I did it fine?

package core;

import org.osbot.rs07.api.map.constants.Banks;
import org.osbot.rs07.api.ui.Skill;
import org.osbot.rs07.api.ui.Spells;
import org.osbot.rs07.script.Script;
import org.osbot.rs07.script.ScriptManifest;
import org.osbot.rs07.utility.ConditionalSleep;

import java.awt.*;


@ScriptManifest(name = "Moltenglass", version = 1, author = "Marcus", logo = "", info = "")


public class Main extends Script {
	

    @Override
    public void onStart() throws InterruptedException {
        getExperienceTracker().start(Skill.MAGIC);
        getExperienceTracker().start(Skill.CRAFTING);
    }
    
    

    @Override
    public int onLoop() throws InterruptedException {

        if (canTan()) {
            clickTan();
        } else {
            bank();
        }
        return 700;
    }

    public boolean canTan() {
        return getEquipment().isWieldingWeaponThatContains("staff") &&
        		(inventory.contains("Giant seaweed") && inventory.getAmount("Giant seaweed") == 3) &&
        		(inventory.contains("Bucket of sand") && inventory.getAmount("Bucket of sand") == 18);
    }

    public void clickTan() {

        if (inventory.contains("Giant seaweed") && (inventory.contains("Bucket of sand"))) {
        	getMagic().castSpell(Spells.LunarSpells.SUPERGLASS_MAKE);
            new ConditionalSleep(1800, 100) {
                @Override
                public boolean condition() throws InterruptedException {
                    return getInventory().onlyContains("Molten glass");
                }
            }.sleep();
        }
    }
    
    
    
    private void bank() throws InterruptedException {
        if (!Banks.GRAND_EXCHANGE.contains(myPosition())) {
            getWalking().webWalk(Banks.GRAND_EXCHANGE);
        } else if (!getBank().isOpen()) {
            getBank().open();
        } else if (!getInventory().isEmptyExcept("Rune pouch")) {
            getBank().depositAll();
        } else if (getBank().contains("Giant seaweed")) {
            getBank().withdraw("Giant seaweed", 1);
            getBank().withdraw("Giant seaweed", 1);
            getBank().withdraw("Giant seaweed", 1);
            getBank().withdraw("Bucket of sand", 18);
            getBank().close();
        } else {
            stop(true);
        }
    }
    
    

    @Override
    public void onPaint(Graphics2D paint) {
        int mXp = getExperienceTracker().getGainedXP(Skill.MAGIC);
        int	cXp = getExperienceTracker().getGainedXP(Skill.CRAFTING);
        int mXpph = getExperienceTracker().getGainedXPPerHour(Skill.MAGIC);
        int cXpph = getExperienceTracker().getGainedXPPerHour(Skill.CRAFTING);



        super.onPaint(paint);
        paint.drawString("Magic XP: " + mXp, 387, 328);
        paint.drawString("Crafting XP: " + cXp, 387, 313);
        paint.drawString("Magic XP/PH: " + mXpph, 237, 328);
        paint.drawString("Crafting XP/PH: " + cXpph, 237, 313);
    }
}

Thanks for the input.

Edited May 7, 2020 by Lol_marcus

Geeseball · May 7, 2020

Quote


            getMouse().click(rX, rY, false);
            new ConditionalSleep(1800, 100) {
                @Override
                public boolean condition() throws InterruptedException {
                    return getInventory().onlyContains("Molten glass");
                }
            }.sleep();

I am not 100% experienced in spells and running them but try this for you spell:

getMagic().castSpell(Spells.LunarSpells.SUPERGLASS_MAKE);

And correct my if I'm wrong but just try this:

getBank().withdraw("Giant seaweed", 3);

Sorry for the messy replies and edit.

Edited May 7, 2020 by Geeseball

Lol_marcus · May 7, 2020

58 minutes ago, Geeseball said:
I am not 100% experienced in spells and running them but try this for you spell:
getMagic().castSpell(Spells.LunarSpells.SUPERGLASS_MAKE);
And correct my if I'm wrong but just try this:
getBank().withdraw("Giant seaweed", 3);
Sorry for the messy replies and edit.

I've edited the piece of code to have it cast the spell. Thanks

If I use withdraw("Giant seaweed", 3); it takes at least 2-3 seconds longer per bank iteration to get the items. Because it has to keep changing the amount it's withdrawing for the seaweed and for the buckets of sand. If I have it the way it is now it'll click 3 times on the seaweed and then use the withdraw X option to get 18 buckets of sand, which is much faster. Did that make any sense?

Edited May 7, 2020 by Lol_marcus

Foulwerp · May 8, 2020

First off putting everything into methods then just calling that method is a terrible way to do this while it works it isn't pretty. I would just put it all into your loop directly. Typically you want to do only 1 action a loop. With the banking you could make it return from the loop after every withdraw but I cba to do that. Remember when you return from the loops that's how long it will wait in milliseconds before going back into the loop so you'll have to tweak the returns how you want I made everything 1 second.

   @Override
    public int onLoop() throws InterruptedException {
       if (!Banks.GRAND_EXCHANGE.contains(myPlayer())) {
            walking.webWalk(Banks.GRAND_EXCHANGE);
            return 1000;
        }
        if (inventory.getAmount("Giant seaweed") == 3 && inventory.getAmount("Bucket of sand") == 18) {
            if (bank.isOpen()) {
                bank.close();
                return 1000;
            }
            getMagic().castSpell(Spells.LunarSpells.SUPERGLASS_MAKE);
            new ConditionalSleep(1000, 100) {
                @Override
                public boolean condition() {
                    return getInventory().contains("Molten glass");
                }
            }.sleep();
            return 1000;
        }
        if (bank.isOpen()) {
            if (inventory.contains("Molten glass")) {
                bank.depositAll();
                return 1000;
            }
            if (bank.getAmount("Giant seaweed") < 3 || bank.getAmount("Bucket of sand") < 18) {
                bank.close();
                stop();
            }
            bank.withdraw("Giant seaweed", 1);
            bank.withdraw("Giant seaweed", 1);
            bank.withdraw("Giant seaweed", 1);
            bank.withdraw("Bucket of sand", 18);
            return 1000;
        }
        bank.open();
        return 1000;
    }

Lol_marcus · May 8, 2020

33 minutes ago, Foulwerp said:

First off putting everything into methods then just calling that method is a terrible way to do this while it works it isn't pretty. I would just put it all into your loop directly. Typically you want to do only 1 action a loop. With the banking you could make it return from the loop after every withdraw but I cba to do that. Remember when you return from the loops that's how long it will wait in milliseconds before going back into the loop so you'll have to tweak the returns how you want I made everything 1 second.


   @Override
    public int onLoop() throws InterruptedException {
       if (!Banks.GRAND_EXCHANGE.contains(myPlayer())) {
            walking.webWalk(Banks.GRAND_EXCHANGE);
            return 1000;
        }
        if (inventory.getAmount("Giant seaweed") == 3 && inventory.getAmount("Bucket of sand") == 18) {
            if (bank.isOpen()) {
                bank.close();
                return 1000;
            }
            getMagic().castSpell(Spells.LunarSpells.SUPERGLASS_MAKE);
            new ConditionalSleep(1000, 100) {
                @Override
                public boolean condition() {
                    return getInventory().contains("Molten glass");
                }
            }.sleep();
            return 1000;
        }
        if (bank.isOpen()) {
            if (inventory.contains("Molten glass")) {
                bank.depositAll();
                return 1000;
            }
            if (bank.getAmount("Giant seaweed") < 3 || bank.getAmount("Bucket of sand") < 18) {
                bank.close();
                stop();
            }
            bank.withdraw("Giant seaweed", 1);
            bank.withdraw("Giant seaweed", 1);
            bank.withdraw("Giant seaweed", 1);
            bank.withdraw("Bucket of sand", 18);
            return 1000;
        }
        bank.open();
        return 1000;
    }

That makes perfect sense, and the way you've written it out is great for visual learners like myself. Thanks for the input.

Geeseball · May 8, 2020

14 hours ago, Lol_marcus said:

I've edited the piece of code to have it cast the spell. Thanks

If I use withdraw("Giant seaweed", 3); it takes at least 2-3 seconds longer per bank iteration to get the items. Because it has to keep changing the amount it's withdrawing for the seaweed and for the buckets of sand. If I have it the way it is now it'll click 3 times on the seaweed and then use the withdraw X option to get 18 buckets of sand, which is much faster. Did that make any sense?

Ah, sorry I misunderstood you. I would leave the banking of Seaweed and Buckets of Sands like this.

In addition, I do not agree with @Foulwerp In a small script like this it makes little difference whether you put everything in the loop or create separate methods, the overview remains there. However, in larger scripts creating methods is much more organized, and therefore creating methods is certainly not a “terrible way” in my opinion. You can always use methods for your consistency, be it for small or large scripts.

Next, try to incorporate more randomness into your scripts. Add a Thread.sleep(random(500,100)) or a return(random(500,1000) instead of a fixed number. In reality, a person does not always click on the next action exactly after 1 second.

If you have any more questions don’t hesitate to send me a message, I don’t mind helping you!

Edited May 8, 2020 by Geeseball

Ace99 · May 8, 2020

17 hours ago, Lol_marcus said:

new ConditionalSleep(1800, 100) { @Override public boolean condition() throws InterruptedException { return getInventory().onlyContains("Molten glass"); } }.sleep();

If you have a rune pouch in your inv for this (like your banking section makes it seem) your sleep when clickTan() is never met as your inv will never only contain molten glass. Also to that same degree in your canTan() you don't check for the rune pouch. There is also a check for "if you have runes to cast x spell" in the api. Might want to use that too. Finally I think it is good practice to method everything and just use the methods in the main loop. Organized and clear... well would be clearer if you didn't reuse method names from a tanner lol!

Oh and to answer your main questions, look into making a custom Interaction Event. I've only used them with interacting with the inv but it may work with a bank. This way you don't loop 3 times for 3 seaweed clicks, the event will click all 3 times, at one time.

Lol_marcus · May 8, 2020

39 minutes ago, Ace99 said:

If you have a rune pouch in your inv for this (like your banking section makes it seem) your sleep when clickTan() is never met as your inv will never only contain molten glass. Also to that same degree in your canTan() you don't check for the rune pouch. There is also a check for "if you have runes to cast x spell" in the api. Might want to use that too. Finally I think it is good practice to method everything and just use the methods in the main loop. Organized and clear... well would be clearer if you didn't reuse method names from a tanner lol!

Oh and to answer your main questions, look into making a custom Interaction Event. I've only used them with interacting with the inv but it may work with a bank. This way you don't loop 3 times for 3 seaweed clicks, the event will click all 3 times, at one time.

Ah! I didn't notice I kept the same method names from my previous script! ^^ I had made a lunar spell hide tanner beforehand.

The custom interaction sounds like a great idea! If it's not too much of a hassle, would you mind giving me a snippet of your custom inventory interaction event? If not I'll look through other scripts and have a gander with the API. Thanks for the input!

4 hours ago, Geeseball said:

Ah, sorry I misunderstood you. I would leave the banking of Seaweed and Buckets of Sands like this.

In addition, I do not agree with @Foulwerp In a small script like this it makes little difference whether you put everything in the loop or create separate methods, the overview remains there. However, in larger scripts creating methods is much more organized, and therefore creating methods is certainly not a “terrible way” in my opinion. You can always use methods for your consistency, be it for small or large scripts.

Next, try to incorporate more randomness into your scripts. Add a Thread.sleep(random(500,100)) or a return(random(500,1000) instead of a fixed number. In reality, a person does not always click on the next action exactly after 1 second.

If you have any more questions don’t hesitate to send me a message, I don’t mind helping you!

I had originally left sleep(random(x,y))'s throughout the script, but I felt it made it messy, and removed them. I'll take a look into implementing it again, but with cleaner code. Thanks for the input and the offer to help, that's very kind of you!

Foulwerp · May 8, 2020

I guess I could of wrote my response a little more elegantly.

He needs to learn how to utilize the loop and returning from the loop before he starts trying all these other things. He is using voids, doing everything and then just returning a default 700 after every action. Where if he puts the code in the loop he can tweak the returns and randomize them after each action. Does an action need to wait x millis before going back into the loop ect. The way he was doing it with voids ect works sure but he would be better suited at his level sticking to the loop till he learns how it works and gets more comfortable with the loop. Once he has a firm grasp of the logic and how it work he can move forward onto more complex things.

While we could drop him head first in to task based scripts and push him to learn it before hes really ready and he could succeed and do well. The logic on those is the same as if you were to just put it in the loop itself and he probably would have less headaches with trying to figure out the java side of it as he has a very basic understanding of java.

Lol_marcus · May 8, 2020

1 hour ago, Foulwerp said:

I guess I could of wrote my response a little more elegantly.

He needs to learn how to utilize the loop and returning from the loop before he starts trying all these other things. He is using voids, doing everything and then just returning a default 700 after every action. Where if he puts the code in the loop he can tweak the returns and randomize them after each action. Does an action need to wait x millis before going back into the loop ect. The way he was doing it with voids ect works sure but he would be better suited at his level sticking to the loop till he learns how it works and gets more comfortable with the loop. Once he has a firm grasp of the logic and how it work he can move forward onto more complex things.

While we could drop him head first in to task based scripts and push him to learn it before hes really ready and he could succeed and do well. The logic on those is the same as if you were to just put it in the loop itself and he probably would have less headaches with trying to figure out the java side of it as he has a very basic understanding of java.

I believe myself to be a very fast learner as well as having some knowledge of other programming languages. I might not know a whole lot about JAVA, but I'm familiar with structuring code and implementing logic/flow. I'd excel more if I learned the actual way scripts are made and structured rather than learning basic "Hello world!" things, if that makes sense? I know how to do everything you've mentioned in regards to randomizing the actions. I had it originally in my script, but decided to remove it for the sake of XP. With all the randomizing I had on the script I was losing circa 20k xp/ph between magic and crafting, and for a throwaway account, the increase in ban potential outweighed the xp and gp /ph loss.

Foulwerp · May 8, 2020

Sorry if I misunderstood your other posts say you are new with java but didn't mention prior experience.

Having perfect xp rates for long or sometimes even short periods of time is a red flag for your account. I personally take random afk breaks in my scripts to break up the pattern and perfectness of the script and sacrifice a lot of xp to that to try to ensure longevity of the account. It seems to work for me, or I may just be lucky, but I have made scripts with perfect xp rates and those accounts didn't last nearly as long as accounts that sacrifice some xp.

I understand you are on a throwaway account but even sacrificing some xp to ensure that your account live long enough to produce for you will greatly help and you can get more produced albeit slower but if they last longer they can produce more for you. I don't know what you are doing with the accounts seems like producing gold from the scripts.

Edited May 8, 2020 by Foulwerp

memelord123 · May 9, 2020

On 5/8/2020 at 1:26 AM, Lol_marcus said:


    private void bank() throws InterruptedException {
        if (!Banks.GRAND_EXCHANGE.contains(myPosition())) {
            getWalking().webWalk(Banks.GRAND_EXCHANGE);
        } else if (!getBank().isOpen()) {
            getBank().open();
        } else if (!getInventory().isEmptyExcept("Rune pouch")) {
            getBank().depositAll();
        } else if (getBank().contains("Giant seaweed")) {
            getBank().withdraw("Giant seaweed", 1);
            getBank().withdraw("Giant seaweed", 1);
            getBank().withdraw("Giant seaweed", 1);
            getBank().withdraw("Bucket of sand", 18);
            getBank().close();
        } else {
            stop(true);
        }
    }

If you change the else if statements into if statements, your code will be able to run multiple statements one after the other (during the same onLoop()) instead of running one if else and breaking out and waiting for the next iteration. You might want to add sleeps if you do that though but it will still make it quicker than sleeping 700ms before iterating again. Also you might want to add small random sleeps between the seaweed withdrawals:

else if (getBank().contains("Giant seaweed")) {
            if (getBank().withdraw("Giant seaweed", 1)) {
            	sleep(random(5,15);
            } if (getBank().withdraw("Giant seaweed", 1)) {
            	sleep(random(5,15);
            } if (getBank().withdraw("Giant seaweed", 1)) {
            	sleep(random(5,15);
            } getBank().withdraw("Bucket of sand", 18);
            getBank().close();

And a randomised onLoop() return value, eg: return random(600, 800);

Lol_marcus · May 9, 2020

1 hour ago, memelord123 said:
If you change the else if statements into if statements, your code will be able to run multiple statements one after the other (during the same onLoop()) instead of running one if else and breaking out and waiting for the next iteration. You might want to add sleeps if you do that though but it will still make it quicker than sleeping 700ms before iterating again. Also you might want to add small random sleeps between the seaweed withdrawals:
else if (getBank().contains("Giant seaweed")) {
            if (getBank().withdraw("Giant seaweed", 1)) {
            	sleep(random(5,15);
            } if (getBank().withdraw("Giant seaweed", 1)) {
            	sleep(random(5,15);
            } if (getBank().withdraw("Giant seaweed", 1)) {
            	sleep(random(5,15);
            } getBank().withdraw("Bucket of sand", 18);
            getBank().close();
And a randomised onLoop() return value, eg: return random(600, 800);

Thanks for the input. I'll try them out and see if it makes the script better.

I had random sleeps between withdrawing the seaweed, but the clicks were too slow to be human-like. When you're getting 3 of the same thing out of the bank it's literally split seconds between the clicks. With the sleep I noticed it would click some-what slow.

Any idea why?

Nbacon · May 10, 2020

Hi marcus

I think this would solve alot of your problems. Just look at the script I see little errors that do unintended things. If you do it like this , you dont have to do alot of chained if. I found the those to slow down bots.(but not by alot)

Quote

public void onStart() throws InterruptedException {
getExperienceTracker().start(Skill.MAGIC);

getExperienceTracker().start(Skill.CRAFTING);
if(getMagic().getCurrentBook() != Magic.Book.LUNAR){
//swap
}
if (!Banks.GRAND_EXCHANGE.contains(myPosition())) {
getWalking().webWalk(Banks.GRAND_EXCHANGE);
}

if(!getEquipment().isWieldingWeaponThatContains("staff")){
//get the staff and put it on
}

if(!getEquipment().isWieldingWeaponThatContains("Rune pouch")){
//get the rune pouch and check it
}

}
public int onLoop() throws InterruptedException {

int loop_time =random(700,900);;
if (getBank().isOpen()){
if (getInventory().contains("Molten glass")){
getBank().depositAllExcept("Rune pouch");
if (bank.getAmount("Giant seaweed") < 3 || bank.getAmount("Bucket of sand") < 18) {
bank.close();
stop();
}

}else if (getInventory().getItem("Giant seaweed").getAmount()<3){
getBank().withdraw("Giant seaweed", 1);
loop_time =random(5,50);
}else if(!getInventory().contains("Bucket of sand")){
getBank().withdraw("Bucket of sand", 18);
loop_time =random(5,50);
}else {
getBank().close();
}

}else {

if ((inventory.contains("Giant seaweed")&&(inventory.contains("Bucket of sand") ) ){

clickTan();
}else {

getBank().open();
}

}

return loop_time;
}

Edited May 10, 2020 by Nbacon
I cant spell

Sign In

[Script Help Please] Is there a better way to optimize this code?

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation

Recently Browsing 0 members