Pick certain amount of items

[bumzag]

I know this is a very simple concept but for some reason I'm struggling. I'm making a script to complete Goblin Diplomacy and I need to pick exactly 3 redberries. I know there's something inherently wrong with it, but here's my for loop thus far

		if (!(getInventory().getAmount("Redberries") == 3))
		{
			walk(redberriesArea);
		
			RS2Object redberries = getObjects().closest("Redberry bush");
			
			for (int i=0; i<4; i++)
			{
				int berryCount = i;
				redberries.interact("Pick-from");
				Sleep.sleepUntil(() -> getInventory().getAmount("Redberries") == berryCount + 1, 5000);
			}
		}

What am I doing wrong? It won't pick exactly 3, it'll pick one and then move on with the script, which makes the character run to a different area. Sometimes it'll pick 1, I think it's picked 2 before too.

Edited September 2, 2020 by bumzag

[BravoTaco]

1 hour ago, bumzag said:

I know this is a very simple concept but for some reason I'm struggling. I'm making a script to complete Goblin Diplomacy and I need to pick exactly 3 redberries. I know there's something inherently wrong with it, but here's my for loop thus far

		if (!(getInventory().getAmount("Redberries") == 3))
		{
			walk(redberriesArea);
		
			RS2Object redberries = getObjects().closest("Redberry bush");
			
			for (int i=0; i<4; i++)
			{
				int berryCount = i;
				redberries.interact("Pick-from");
				Sleep.sleepUntil(() -> getInventory().getAmount("Redberries") == berryCount + 1, 5000);
			}
		}

What am I doing wrong? It won't pick exactly 3, it'll pick one and then move on with the script, which makes the character run to a different area. Sometimes it'll pick 1, I think it's picked 2 before too.

You don't have to inverse the bool value in the first if statement just use != operator instead of ==, it will make it easier to read.

Ex. - if (getInventory.getAmount("Redberries") != 3)

You should also remove the for loop and allow the script to re-loop to re-trigger this action. I have found that scripts are much easier to debug/maintain if there is only 1 action happening per loop.

When interacting with something you should wrap it in an if statement so you will only activate your sleep call if the interaction was successful.

Ex. - if (redberries.interact("Pick-from"))

Now for the problem of it not picking the required amount, that is down to the current logic of the script.

If we take a look at this line

RS2Object redberries = getObjects().closest("Redberry bush");

And than look at how you are using that variable

for (int i=0; i<4; i++) {
  int berryCount = i;
  redberries.interact("Pick-from");
  Sleep.sleepUntil(() -> getInventory().getAmount("Redberries") == berryCount + 1, 5000);
}

You can see that in the loop you are not updating the redberries variable to another available bush, so you are trying to pick berries from the same bush even though you have already picked from there.

To fix that you would simply move where you set the variable to inside the loop, but again I don't recommend using for/while loops in scripts except for certain circumstances such as different dropping methods. Also you should always do a null check before using the variable.  With the for loop, if i = 0 than, you would do i < 3, to perform an action 3 times, since i is starting at 0. No need to set berrycount == i, just use i.

for (int i = 0; i < 3; i++)
{
  RS2Object redberries = getObjects().closest("Redberry bush");
  if (redberries != null) {
    if (redberries.interact("Pick-from")) {
       Sleep.sleepUntil(() -> getInventory().getAmount("Redberries") == (i + 1), 5000);
    }
  }
}

Let me know if you have any questions  

Edited September 2, 2020 by BravoTaco

[bumzag]

1 hour ago, BravoTaco said:

You don't have to inverse the bool value in the first if statement just use != operator instead of ==, it will make it easier to read.

Ex. - if (getInventory.getAmount("Redberries") != 3)

You should also remove the for loop and allow the script to re-loop to re-trigger this action. I have found that scripts are much easier to debug/maintain if there is only 1 action happening per loop.

When interacting with something you should wrap it in an if statement so you will only activate your sleep call if the interaction was successful.

Ex. - if (redberries.interact("Pick-from"))

Now for the problem of it not picking the required amount, that is down to the current logic of the script.

If we take a look at this line

RS2Object redberries = getObjects().closest("Redberry bush");

And than look at how you are using that variable

for (int i=0; i<4; i++) {
  int berryCount = i;
  redberries.interact("Pick-from");
  Sleep.sleepUntil(() -> getInventory().getAmount("Redberries") == berryCount + 1, 5000);
}

You can see that in the loop you are not updating the redberries variable to another available bush, so you are trying to pick berries from the same bush even though you have already picked from there.

To fix that you would simply move where you set the variable to inside the loop, but again I don't recommend using for/while loops in scripts except for certain circumstances such as different dropping methods. Also you should always do a null check before using the variable.  With the for loop, if i = 0 than, you would do i < 3, to perform an action 3 times, since i is starting at 0. No need to set berrycount == i, just use i.

for (int i = 0; i < 3; i++)
{
  RS2Object redberries = getObjects().closest("Redberry bush");
  if (redberries != null) {
    if (redberries.interact("Pick-from")) {
       Sleep.sleepUntil(() -> getInventory().getAmount("Redberries") == (i + 1), 5000);
    }
  }
}

Let me know if you have any questions  

This is why I love this site. Thank you for the detailed answer.

The only problem is, with your solution, it still tries picking from the bush even if there's no berries and it still increments the for loop, so I eventually run away. Is there a way to monitor the chat for the error it spits out when there's nothing left on the bush?

[Nbacon]

21 minutes ago, bumzag said:

?

The Item id changes as you pick barries[ 23630 is no redberrys and 23627 for Cadava]

Edited September 2, 2020 by Nbacon

[FuryShark]

50 minutes ago, Nbacon said:

The Item id changes as you pick barries[ 23630 is no redberrys and 23627 for Cadava]

adding to this how i did it

RS2Object bush = getObjects().closest(o -> o.getName().endsWith("bush") && o.getModelIds().length > 1);

[bumzag]

5 minutes ago, FuryShark said:

adding to this how i did it

RS2Object bush = getObjects().closest(o -> o.getName().endsWith("bush") && o.getModelIds().length > 1);

Can you expand on this? Will it also target Cadava bushes?

[FuryShark]

8 hours ago, bumzag said:

Can you expand on this? Will it also target Cadava bushes?

It will pick both bushes when theres a berry available. Just change to suit your needs  

[BravoTaco]

9 hours ago, bumzag said:

Can you expand on this? Will it also target Cadava bushes?

RS2Object bush = getObjects().closest(o -> o.getName().endsWith("bush") && o.getModelIds().length > 1);

Essentially what this is doing is applying a filter, using lambda expression. It will only grab the Object that passes the conditional check.

This can also look like this.

Filter<RS2Object> bushFilter = new Filter<RS2Object>() {
    @Override
    public boolean match(RS2Object rs2Object) {
        return rs2Object.getModelIds().length > 1 && rs2Object.getName().toLowerCase().endsWith("bush");
    }
};
RS2Object bush = getObjects().closest(bushFilter);

With the above code I first create a Filter of type RS2Object, than pass it to the getObjects().closest() method.

You can also cache this filter as a variable inside the class, so you won't have to declare it every time before using it. That would look something like this.

public class OSBotScript extends Script {

    private final Filter<RS2Object> bushFilter = new Filter<RS2Object>() {
        @Override
        public boolean match(RS2Object rs2Object) {
            return rs2Object.getModelIds().length > 1 && rs2Object.getName().toLowerCase().endsWith("bush");
        }
    };

With all that, lets break down what exactly is happening with the checks.

First the getModelIds() call, when the bush has no berries on it the modelIds array will return a a length of 1 since it is only showing the bush model. But once there are berries on the bush, it will than have more than one modelId stored inside the array so the length of it will be greater than 1.

The second call is just simply checking the name to see if the RS2Object does end with bush, you can change this to the exact name instead, that would look like this.

private final Filter<RS2Object> bushFilter = new Filter<RS2Object>() {
    @Override
    public boolean match(RS2Object rs2Object) {
        return rs2Object.getModelIds().length > 1 && rs2Object.getName().equalsIgnoreCase("Redberry bush");
    }
};

If both conditions are true than it will return that RS2Object.

[bumzag]

1 hour ago, BravoTaco said:

RS2Object bush = getObjects().closest(o -> o.getName().endsWith("bush") && o.getModelIds().length > 1);

Essentially what this is doing is applying a filter, using lambda expression. It will only grab the Object that passes the conditional check.

This can also look like this.

Filter<RS2Object> bushFilter = new Filter<RS2Object>() {
    @Override
    public boolean match(RS2Object rs2Object) {
        return rs2Object.getModelIds().length > 1 && rs2Object.getName().toLowerCase().endsWith("bush");
    }
};
RS2Object bush = getObjects().closest(bushFilter);

With the above code I first create a Filter of type RS2Object, than pass it to the getObjects().closest() method.

You can also cache this filter as a variable inside the class, so you won't have to declare it every time before using it. That would look something like this.

public class OSBotScript extends Script {

    private final Filter<RS2Object> bushFilter = new Filter<RS2Object>() {
        @Override
        public boolean match(RS2Object rs2Object) {
            return rs2Object.getModelIds().length > 1 && rs2Object.getName().toLowerCase().endsWith("bush");
        }
    };

With all that, lets break down what exactly is happening with the checks.

First the getModelIds() call, when the bush has no berries on it the modelIds array will return a a length of 1 since it is only showing the bush model. But once there are berries on the bush, it will than have more than one modelId stored inside the array so the length of it will be greater than 1.

The second call is just simply checking the name to see if the RS2Object does end with bush, you can change this to the exact name instead, that would look like this.

private final Filter<RS2Object> bushFilter = new Filter<RS2Object>() {
    @Override
    public boolean match(RS2Object rs2Object) {
        return rs2Object.getModelIds().length > 1 && rs2Object.getName().equalsIgnoreCase("Redberry bush");
    }
};

If both conditions are true than it will return that RS2Object.

That all makes perfect. On a different note however, wouldn't it be possible to simply select the nearest bush by the desired ID? For example if the bush's ID changes, couldn't I then just reselect the other bush based on it's ID?

So basically

if (!(getObjects().closest(modelID)) == desiredID)

I know thats not proper syntax but along those lines

Edited September 2, 2020 by bumzag

[BravoTaco]

21 minutes ago, bumzag said:

That all makes perfect. On a different note however, wouldn't it be possible to simply select the nearest bush the desired ID? For example if the bush's ID changes, couldn't I then just reselect the other bush based on it's ID?

So basically

if (!(getObjects().closest(modelID)) == desiredID)

I know thats not proper syntax but along those lines

Not exactly.

When the berries are picked from the bush, the bush's ID will stay the same but the children model ID of the berries are removed from the modelIDs array.

It is possible to select a bush with an ID, you would just have to create a method that will loop through the bush's child IDs and if it contains the ID than return that bush an example of that would look like this.

private RS2Object getObjectFromChildModelID(Predicate<RS2Object> filter, int childID) {
    Object[] objects = getObjects().getAll().stream().filter(filter).toArray();
    for (Object o : objects) {
        if (o != null) {
            RS2Object rs2Object = (RS2Object) o;
            if (rs2Object != null && rs2Object.getModelIds() != null) {
                for (int id : rs2Object.getModelIds()) {
                    if (id == childID)
                        return rs2Object;
                }
            }
        }
    }
    return null;
}

I have not tested the above code, but lets talk about what I am doing.

First I declare an Array of Objects that contains all objects that matches the given filter.

Than I loop through all those Objects, null check them, and cast them to an RS2Object, I have to do this since when I was grabbing the objects they were stored as an Object and not an RS2Object.

After casting it to an RS2Object, I can loop through the modelIDs and check if any of the children ID's match the given ID.

If a match is found that it will return that object else it will return null.

Also this code will not always return the closest bush, it will return whichever bush passes its condition check first.

The code in action.

Predicate<RS2Object> bushFilter = new Predicate<RS2Object>() {
    @Override
    public boolean test(RS2Object rs2Object) {
        return rs2Object.getName().equalsIgnoreCase("Redberry bush");
    }
};

RS2Object bush = getObjectFromChildModelID(bushFilter, -1);

if (bush != null && bush.interact("Pick-from")) {
    // other code here.
}

First I declare a Predicate of type RS2Object for the filter. A Predicate is essentially the same thing as a Filter, but is Java's version of it.

Than I try to set the bush variable using the above code snippet. The -1 would be whatever the ID of the berries is, you would have to do some investigating to find that.

Lastly, I null check and than interact with the bush.

Edited September 2, 2020 by BravoTaco

[bumzag]

10 hours ago, BravoTaco said:

Not exactly.

When the berries are picked from the bush, the bush's ID will stay the same but the children model ID of the berries are removed from the modelIDs array.

It is possible to select a bush with an ID, you would just have to create a method that will loop through the bush's child IDs and if it contains the ID than return that bush an example of that would look like this.

private RS2Object getObjectFromChildModelID(Predicate<RS2Object> filter, int childID) {
    Object[] objects = getObjects().getAll().stream().filter(filter).toArray();
    for (Object o : objects) {
        if (o != null) {
            RS2Object rs2Object = (RS2Object) o;
            if (rs2Object != null && rs2Object.getModelIds() != null) {
                for (int id : rs2Object.getModelIds()) {
                    if (id == childID)
                        return rs2Object;
                }
            }
        }
    }
    return null;
}

I have not tested the above code, but lets talk about what I am doing.

First I declare an Array of Objects that contains all objects that matches the given filter.

Than I loop through all those Objects, null check them, and cast them to an RS2Object, I have to do this since when I was grabbing the objects they were stored as an Object and not an RS2Object.

After casting it to an RS2Object, I can loop through the modelIDs and check if any of the children ID's match the given ID.

If a match is found that it will return that object else it will return null.

Also this code will not always return the closest bush, it will return whichever bush passes its condition check first.

The code in action.

Predicate<RS2Object> bushFilter = new Predicate<RS2Object>() {
    @Override
    public boolean test(RS2Object rs2Object) {
        return rs2Object.getName().equalsIgnoreCase("Redberry bush");
    }
};

RS2Object bush = getObjectFromChildModelID(bushFilter, -1);

if (bush != null && bush.interact("Pick-from")) {
    // other code here.
}

First I declare a Predicate of type RS2Object for the filter. A Predicate is essentially the same thing as a Filter, but is Java's version of it.

Than I try to set the bush variable using the above code snippet. The -1 would be whatever the ID of the berries is, you would have to do some investigating to find that.

Lastly, I null check and than interact with the bush.

After this question I promise I'll stop beating a dead horse:

What I did was create an object using the RS2Object method and outputted the ID

    RS2Object redberries = getObjects().closest("Redberry bush");

    log(redberries.getId());

I fully comprehend your point about checking the length of the modelID array, but with the code I posted, the log shows "23628" when there are berries, and then "23629" when there are no berries. Knowing this, couldn't I simply redeclare the bush variable to an object with an ID of 23628 instead of 23629?

Nvm, the solution by FuryShark truly is far cleaner and more sufficient. BravoTaco, thank you again for the detailed explanation. You're an excellent teacher.

 

For anyone that stumbles on this thread weeks/months/years from now, here is my code and I implemented FuryShark's solution

		if (!(getInventory().getAmount("Redberries") >= 3))
		{
			walk(redberryArea);
			
			RS2Object redberryBush = getObjects().closest(o -> o.getName().endsWith("Redberry bush") && o.getModelIds().length > 1);
			if (redberryBush.exists())
				if (redberryBush.isVisible())
					if (redberryBush != null)
					{
						int redberryCount = Math.toIntExact(getInventory().getAmount("Redberries"));
						redberryBush.interact("Pick-from");
						Sleep.sleepUntil(() -> getInventory().getAmount("Redberries") > redberryCount, 5000);
					}
			
		}

 

Edited September 3, 2020 by bumzag

[BravoTaco]

17 hours ago, bumzag said:

After this question I promise I'll stop beating a dead horse:

What I did was create an object using the RS2Object method and outputted the ID

    RS2Object redberries = getObjects().closest("Redberry bush");

    log(redberries.getId());

I fully comprehend your point about checking the length of the modelID array, but with the code I posted, the log shows "23628" when there are berries, and then "23629" when there are no berries. Knowing this, couldn't I simply redeclare the bush variable to an object with an ID of 23628 instead of 23629?

Nvm, the solution by FuryShark truly is far cleaner and more sufficient. BravoTaco, thank you again for the detailed explanation. You're an excellent teacher.

 

For anyone that stumbles on this thread weeks/months/years from now, here is my code and I implemented FuryShark's solution

		if (!(getInventory().getAmount("Redberries") >= 3))
		{
			walk(redberryArea);
			
			RS2Object redberryBush = getObjects().closest(o -> o.getName().endsWith("Redberry bush") && o.getModelIds().length > 1);
			if (redberryBush.exists())
				if (redberryBush.isVisible())
					if (redberryBush != null)
					{
						int redberryCount = Math.toIntExact(getInventory().getAmount("Redberries"));
						redberryBush.interact("Pick-from");
						Sleep.sleepUntil(() -> getInventory().getAmount("Redberries") > redberryCount, 5000);
					}
			
		}

 

Np. Glad ya got it all figured out, lmk if you have any other questions.