I read on one of Alek's posts that a script should use if statements to check methods are executing successfully. I have the following banking code:

getBank().open();
sleep(random(600, 1200));
if (!getInventory().contains("Knife")) {
    getBank().withdraw("Knife", 1);
}
getBank().depositAllExcept("Knife", "Arrow shafts");
getBank().withdrawAll(getLogType());
getBank().close();

Would it be good practice to use an if statement for every method call here? I was unsure because it would create a very large section of nested if statements ie. if() {

if() {

if() {

if() {

}  }  }  }

On an unrelated note I know that it might be better to use a conditional sleep after the getBank().open() call, but when I use a conditional sleep that sleeps until the bank widget is visible I believe that it gives the script un-human like speed - any comments or advice for this would be greatly appreciated

Tom

Yes it would be good practice. I would write your code like this:

Have an if that checks whether the bank is open, if it's not then you proceed to open it then use else ifs to check other conditions that will trigger the corresponding event.

if (!getBank.isOpen()){
	if (getBank().open()){
    	sleep(random(600, 1200));
    }
}
else if (!getInventory().contains("Knife")){
 	if (getBank().withdraw("Knife", 1){
    	//some sleep condition
    }
}
//etc...

 

I think it's fine for you to use regular random sleep to mimic human behavior instead of conditional sleep after opening bank but it might just make your script slower.

Quote

On an unrelated note I know that it might be better to use a conditional sleep after the getBank().open() call, but when I use a conditional sleep that sleeps until the bank widget is visible I believe that it gives the script un-human like speed - any comments or advice for this would be greatly appreciated

You could just do a short sleep after the conditional sleep finished. This would be more "human-like". At least in this case it would solve your problem

2 hours ago, R3G3N said:

Yes it would be good practice. I would write your code like this:

Have an if that checks whether the bank is open, if it's not then you proceed to open it then use else ifs to check other conditions that will trigger the corresponding event.

 

I think it's fine for you to use regular random sleep to mimic human behavior instead of conditional sleep after opening bank but it might just make your script slower.

I've put this together, what do you think? 

if (!getBank().isOpen()){
    if (getBank().open()){
        sleep(random(300, 600));
    }
}
else if (!getInventory().contains("Knife")){
    if (getBank().withdraw("Knife", 1)){
        sleep(random(300, 600));
    }
}
else if (getBank().depositAllExcept("Knife", "Arrow shafts")) {
    sleep(random(300, 600));
}
else if (getBank().withdrawAll(getLogType())) {
    sleep(random(300, 600));
    if (getBank().close()) {
        Timing.waitCondition(() -> !getBank().isOpen(), 5000);
    }
}

Notice how the first and second ifs have a condition that needs to be triggered before the action happens. I would put !getInventory().isEmptyExcept("Knife", "Arrow shafts")) for the third if.

6 hours ago, Tommm39 said:

I've put this together, what do you think? 

if (!getBank().isOpen()){
    if (getBank().open()){
        sleep(random(300, 600));
    }
}
else if (!getInventory().contains("Knife")){
    if (getBank().withdraw("Knife", 1)){
        sleep(random(300, 600));
    }
}
else if (getBank().depositAllExcept("Knife", "Arrow shafts")) {
    sleep(random(300, 600));
}
else if (getBank().withdrawAll(getLogType())) {
    sleep(random(300, 600));
    if (getBank().close()) {
        Timing.waitCondition(() -> !getBank().isOpen(), 5000);
    }
}

okay, so let start with those sleep timers, doing a sleep thats random between 300ms and 600ms is pointless, the idea of using if statements to check if a action is completed is so you can use conditional sleeps so the script can flow nicely, static sleep should only be used in very niche places.

first sleep should be condition  bank.isOpen()
second should be inventory.contains("Knife")
third should be inventory.isEmptyExcept("Knife, "Arrow shafts")
fourth should be inventory.contains(getLogType())

watch it run how you have it, then do the change to that ^ and just watch how different the script will act 

Edited April 27, 20187 yr by GPSwap

Thank you both very much for your input - I've put this together and I think it's a lot better now! 

if (!getBank().isOpen()) {
        if (getBank().open()) {
            Timing.waitCondition(() -> getBank().isOpen(), 5000);
        }
    }

    if (!getInventory().contains("Knife")) {
        if (getBank().withdraw("Knife", 1)) {
            Timing.waitCondition(() -> getInventory().contains("Knife"), 5000);
        }
    }

    if (!getInventory().isEmptyExcept("Knife")) {
        if (getBank().depositAllExcept("Knife")) {
            Timing.waitCondition(() -> getInventory().onlyContains("Knife"), 5000);
        }
    }

    if (getBank().contains(getLogType())) {
       if (getBank().withdrawAll(getLogType())) {
           Timing.waitCondition(() -> getInventory().contains(getLogType()), 5000);
       }
    }

    if (getBank().close()) {
        Timing.waitCondition(() -> !getBank().isOpen(), 5000);
    }
}

 

48 minutes ago, Tommm39 said:

Thank you both very much for your input - I've put this together and I think it's a lot better now! 

if (!getBank().isOpen()) {
        if (getBank().open()) {
            Timing.waitCondition(() -> getBank().isOpen(), 5000);
        }
    }

    if (!getInventory().contains("Knife")) {
        if (getBank().withdraw("Knife", 1)) {
            Timing.waitCondition(() -> getInventory().contains("Knife"), 5000);
        }
    }

    if (!getInventory().isEmptyExcept("Knife")) {
        if (getBank().depositAllExcept("Knife")) {
            Timing.waitCondition(() -> getInventory().onlyContains("Knife"), 5000);
        }
    }

    if (getBank().contains(getLogType())) {
       if (getBank().withdrawAll(getLogType())) {
           Timing.waitCondition(() -> getInventory().contains(getLogType()), 5000);
       }
    }

    if (getBank().close()) {
        Timing.waitCondition(() -> !getBank().isOpen(), 5000);
    }
}

That looks nicer and I'm sure it runs better, but when doing banking its always good to put a check to make sure its in your bank and stop the script if it isn't so that your bot doesn't get stuck in a loop trying to withdraw a item you don't have (code example below), also I would put your deposit before you withdraw your knife incase your invent is full and you don't have a knife or it will loop the same thing repeatedly unable to get a knife.

if (!getInventory().isEmptyExcept("Knife")) {
	if(getBank().contains("Knife")){
		if (getBank().depositAllExcept("Knife")) {
			Timing.waitCondition(() -> getInventory().onlyContains("Knife"), 5000);
		}
	}else{
		log("No knife is found stopping script");
		stop(true);
	}
}

Edited April 27, 20187 yr by GPSwap

As for sleeps, I usually do a conditional sleep and then a small regular sleep afterwards

1 hour ago, Tom said:

As for sleeps, I usually do a conditional sleep and then a small regular sleep afterwards

How long do you set that small regular sleep for?

7 minutes ago, Antonio Kala said:

How long do you set that small regular sleep for?

Generally between 100-500 milliseconds