Need some help with my script

[Lychees]

Hello Guys,

Im working on a script that makes tuna potatoes from cooked ingredients in the bank 

It basically works like this:

 

Inventory contains 9 Bowl, 9 Cooked sweetcorn, 9 Tuna and a Knife

Combines items to 9 Corn and tuna

Bank to get 9 Pat of butter and 9 Baked potato

Combines items to get Potato with butter and after that makes the Tuna potatoes

And leaves the inventory with 9 Bowl and 9 Tuna Potatoes

 

But I can’t get the banking to work I’ve tried this:

 case BANK:
    	if (!getBank().isOpen()) {
    	    getBank().open();
    	} else {
    		// Inventory 9 Tuna and corn, 9 Pat of butter, 9 Baked Potato and 1 knife inventory ready for making Potato with butter and Tuna Potato.
    		if (this.inventory.getAmount(new String[] { "Tuna and corn" }) == 9) {
        		sleep(random(333, 666));
        	    getBank().depositAllExcept(new String[] { "Knife", "Tuna and corn" });
        	    	sleep(random(300, 800));
        	    	getBank().withdraw("Pat of butter", 9);
        	    	sleep(random(300, 800));
        	    	getBank().withdraw("Baked Potato", 9);
        	    	sleep(random(300, 800));
        	    	if (this.inventory.getAmount(new String[] { "Pat of butter", "Baked Potato" }) == 18){
        	    	getBank().close();
        	    	sleep(random(300, 800));
        	    	return Main.random(300, 800);
    	 		}}
    		// Inventory 9 Bowl, 9 Tuna, 9 Cooked Sweedcorn and 1 knife start and finish of inventory.
    		if (this.inventory.getAmount(new String[] { "Bowl" }) == 9) {
        		sleep(random(333, 666));
        	    getBank().depositAllExcept(new String[] { "Knife", "Bowl" });
        	    	sleep(random(300, 800));
        	    	getBank().withdraw("Tuna", 9);
        	    	sleep(random(300, 800));
        	    	getBank().withdraw("Cooked Sweedcorn", 9);
        	    	sleep(random(300, 800));
        	    	if (this.inventory.getAmount(new String[] { "Tuna", "Cooked Sweedcorn" }) == 18){
        	    	getBank().close();
        	    	sleep(random(300, 800));
        	    	return Main.random(300, 800);
    	 		}}
    	}
        break;

Some help would be appreciated ;)

[Apaec]

Having a look at your code, you appear to be making a very common mistake with beginners - not considering that OSRS is a live game. Issues such as latency fluctuations can mean any given line isn't guaranteed to execute, even though it might seem like most of the time it is successful. To account for this, you have to make all of your code conditional. This means nesting your code with checks to make sure it does one interaction at a time per loop, and at no point in your code should any line rely on another to successfully execute.

I know you have got the opening correct, however the following example is just to illustrate a point:

For example, consider the code:

getBank().open();
getBank().depositAll();

What if the first line fails to successfully open the bank? Then the second line will cause a horrible error to be thrown in the logger, and your script will fail.

The key too a good script is reliability - here's a better way of structuring the above code (and you will need to do this for all of your withdrawing):

if (getBank().isOpen() {
	if (!getInventory().isEmpty() {
		getBank().depositAll();
	}
} else { 
	getBank().open();
}

Of course you would probably need to add conditional sleeps in there, where appropriate, to avoid spam-clicking.

Secondly, you're using a huge amount of static sleeps. BAD!! Make them conditional (to prevent spam-clicking) or remove them altogether - they would not be allowed on the SDN anyway!

Good luck!

[HeyImJamie]

What Apaec said, also why do you define String arrays for things that aren't even an array  

[Lychees]

1 hour ago, Apaec said:

Having a look at your code, you appear to be making a very common mistake with beginners - not considering that OSRS is a live game. Issues such as latency fluctuations can mean any given line isn't guaranteed to execute, even though it might seem like most of the time it is successful. To account for this, you have to make all of your code conditional. This means nesting your code with checks to make sure it does one interaction at a time per loop, and at no point in your code should any line rely on another to successfully execute.

I know you have got the opening correct, however the following example is just to illustrate a point:

For example, consider the code:

getBank().open();
getBank().depositAll();

What if the first line fails to successfully open the bank? Then the second line will cause a horrible error to be thrown in the logger, and your script will fail.

The key too a good script is reliability - here's a better way of structuring the above code (and you will need to do this for all of your withdrawing):

if (getBank().isOpen() {
	if (!getInventory().isEmpty() {
		getBank().depositAll();
	}
} else { 
	getBank().open();
}

Of course you would probably need to add conditional sleeps in there, where appropriate, to avoid spam-clicking.

Secondly, you're using a huge amount of static sleeps. BAD!! Make them conditional (to prevent spam-clicking) or remove them altogether - they would not be allowed on the SDN anyway!

Good luck!

Thank you, I understand it better now and it runs much cleaner with conditional sleeps i did this

    case BANK:
    	if (getBank().isOpen()) {
    		  if (getBank().contains("Bowl") && (getBank().getAmount("Bowl") >= 9)) {
    		    getBank().withdraw("Bowl", 9);
    		    new ConditionalSleep(3000, 500) {
    		    	   public boolean condition() throws InterruptedException {
    		    		   return getInventory().contains("Bowl");
						}
					}.sleep();
				}}
    		  else {
    		  getBank().open();
    		  new ConditionalSleep(3000, 500) {
		    	   public boolean condition() throws InterruptedException {
		    		   return  (getBank().isOpen());
					}
				}.sleep();
			}
    
        break;

56 minutes ago, HeyImJamie said:

What Apaec said, also why do you define String arrays for things that aren't even an array  

Thought that would be handy for all the items but never continued with the idea. Sorry for the mess

[Apaec]

2 minutes ago, Lychees said:

Thank you, I understand it better now and it runs much cleaner with conditional sleeps i did this

    case BANK:
    	if (getBank().isOpen()) {
    		  if (getBank().contains("Bowl") && (getBank().getAmount("Bowl") >= 9)) {
    		    getBank().withdraw("Bowl", 9);
    		    new ConditionalSleep(3000, 500) {
    		    	   public boolean condition() throws InterruptedException {
    		    		   return getInventory().contains("Bowl");
						}
					}.sleep();
				}}
    		  else {
    		  getBank().open();
    		  new ConditionalSleep(3000, 500) {
		    	   public boolean condition() throws InterruptedException {
		    		   return  (getBank().isOpen());
					}
				}.sleep();
			}
    
        break;

 

 

Thought that would be handy for all the items but never continued with the idea. Sorry for the mess

 

Perhaps try writing your own withdrawing method, which takes a string item and int amount, and maybe some other parameters such as booleans for noted, stackable etc. Then this method would do different things based on the bank status, e.g if you already have a certain number and more, it would deposit instead of withdraw, if you don't have any it would stop, etc. Might be a good project for you to work on!

[Lychees]

1 hour ago, Apaec said:

Perhaps try writing your own withdrawing method, which takes a string item and int amount, and maybe some other parameters such as booleans for noted, stackable etc. Then this method would do different things based on the bank status, e.g if you already have a certain number and more, it would deposit instead of withdraw, if you don't have any it would stop, etc. Might be a good project for you to work on!

That would be great for this script but I have no idea where to start.

Maybe make a list of items in inventory and in the bank put them in a string with the amount

Subtract the items from each other and check them like this.

if Subtractitems == amountneeded great! return

if Subtractitems > = amountneeded deposit

if Subtractitems <  = amountneeded withdraw

i think i can use a Arraylist with some kind of Iterator

is this is a step in the right direction?

[Apaec]

5 minutes ago, Lychees said:

That would be great for this script but I have no idea where to start.

 

Maybe make a list of items in inventory and in the bank put them in a string with the amount

 

Subtract the items from each other and check them like this.

if Subtractitems == amountneeded great! return

 

if Subtractitems > = amountneeded deposit

if Subtractitems <  = amountneeded withdraw

i think i can use a Arraylist with some kind of Iterator

is this is a step in the right direction?

 

Looks good, but you don't need any lists! 

A method would just take in the parameters and perform the checks on them. You know the amount of an item by getInventory().getAmount() - make sure you have some though, otherwise this will throw an error.

This should get you started, wrote it in reply box so apologies if there are any errors!

private boolean withdrawItem(String itemName, int amount, boolean noted, boolean stackable) {
  if (!stackable && amount>28) return false; // illegal argument
  if (getBank().isOpen()) { // make sure bank is open
    int currentAmount = getInventory().contains(itemName) ? getInventory().getAmoumt(itemName) : 0;
    int amountNeeded = amount - currentAmount;
    if (amountNeeded > 0) {
      // Withdraw some!
    } else if (amountNeeded < 0) {
      // Deposit some!
    } else return true; // We already have the right amount!
  } else return false;
}