Help with looting

[mrgeorge]

import org.osbot.rs07.api.NPCS;
import org.osbot.rs07.api.Walking;
import org.osbot.rs07.api.model.GroundItem;
import org.osbot.rs07.script.Script;
import org.osbot.rs07.script.ScriptManifest;

@ScriptManifest(author = "myName", info = "Kills chickens in Fally.", name = "ChickSlayer", version = 0.1, logo = "")
public class Main extends Script {

	public void onStart() {

	}

	public void onExit() {

	}

	public int onLoop() throws InterruptedException {
		NPCS npc = getNpcs();
		GroundItem arrows = groundItems.closest("Bronze arrow");
		Walking walk = new Walking();
		if (npc.closest("Chicken").isVisible() && !(myPlayer().isUnderAttack())) {
			attackNPC(npc);
			pickUpArrows(arrows, walk);
			sleep(1500);
		} else {
			camera.toEntity(npc.closest("Chicken"));
		}
		return 1500;
	}

	private void pickUpArrows(GroundItem arrows, Walking walk) throws InterruptedException {
		if (!myPlayer().isAnimating() && !myPlayer().isUnderAttack() && !myPlayer().isMoving()) {
			if (!arrows.isOnScreen()) {
				camera.toEntity(arrows);
				if (!arrows.isOnScreen()) {
					walk.walk(arrows);
				}
			}
			arrows.interact("Take");
		}

	}

	private void attackNPC(NPCS npc) throws InterruptedException {
		if (npc.closest("Chicken").isAttackable() && npc.closest("Chicken").exists() && !(myPlayer().isUnderAttack())
				&& !(npc.closest("Chicken").isUnderAttack())) {
			npc.closest("Chicken").interact("Attack");
			sleep(1500);
		}
	}
}

The code seems to compile and the script does run 75% of my logic. However I want to pick up the arrows right after the kill. And it is unable to do that. The script does in fact pick up the arrows but not right after the chicken is dead. It kills maybe 2 or 3 chickens then decides to pick up the arrows from the ground.  It also picks up arrows while its in combat. Please look in the picture below.

[Isolate]

30 minutes ago, mrgeorge said:

 

Ya ok so, important things to note, loop will run all code in order and not wait unless you tell it to
so knowing this important things to note here
> Attack chicken
> collect arrows
are straight after eachother, the code will not wait for the chicken to die before collecting arrows, it'll run collect arrows straight after it's attacked the npc
and since the first check of collect arrows is no combat, no animating, no moving
it'll not run in nearly any case if the attack function worked
recommend considering the loop is a loop
After reading the code a little after answering this question the way you're handling this whole situation is scary. to me
I reccomend looking into filters, and not declaring things you don't need to
an example
 

@Override
    public int onLoop() throws InterruptedException {
        //Mmk so we're going to look for a chicken then store it as freeChicken
        NPC freeChicken = getNpcs().closest(new Filter<NPC>() {
            //to do this we want to check multiple things since this is a combat related search
            @Override
            public boolean match(NPC npc) {
                       //firstly you get the obvious, check if the npcs name is Chicken because we hate chickens.
                return npc.getName().equalsIgnoreCase("Chicken") &&
                        //we gon check if the chickens health is > 0 because when a chicken is doing the dying animation technically it's not fighting..?
                         npc.getHealthPercent() >=0 &&
                        //then we check if this chicken is under attack, because we don't want non of that shit
                        !npc.isUnderAttack() &&
                        //next check is kinda a double whammy of "just in case"
                        // we check if the chicken isn't interacting (presumably fighting) a player OR the chicken IS and the player us US
                        (npc.getInteracting() == null || npc.getInteracting() != null && npc.getInteracting() == myPlayer())
                        //finally if we can reach our little chickeny friend for its death
                        && getMap().canReach(npc);
            }
        });

        //same deailo for our friend the arrow
        GroundItem arrows = getGroundItems().closest(new Filter<GroundItem>() {
            @Override
            public boolean match(GroundItem groundItem) {
                       //generic name check in the filter                                    can we reach this arrow without obstacle?
                return groundItem.getName().equalsIgnoreCase("Bronze arrow") && getMap().canReach(groundItem)
                        //bit of a not needed, but you can only grab arrows of stacks x or more to save time collecting 1 stacks because chickens can run
                        && groundItem.getAmount() >= 1;
            }
        });


        //I assume we're prioritising arrows here so the logic would be as follows
        // I put this if up here because it applies to both cases, don't fucking start with me about nesting ifs
        if(!myPlayer().isUnderAttack() && myPlayer().getInteracting() == null){
            if (arrows != null) {
                //oh there are some arrows that match our filter ! :D
                if (arrows.interact("Take")) {
                    //sleep
                } else {
                    //prolly just moved the camera or walked towards them
                }
            } else if (freeChicken != null) {
                //hmm no arrows but there appears to be a wild chickernnn
                if (freeChicken.interact("Attack")) {
                    //kill da squarker
                } else {
                    //prolly just navigated towards it, don't sleep so the next interact can happen quicker plz
                }
            }
        }
        return random(250, 750);
    }

 

Edited May 21, 2017 by Isolate

[Polymorphism]

Since this is a looping structure, such as Isolate stated -- the code will always run back to back and your issue is the way you've decided to handling looting.

 

Now consider this...

if there are arrows on ground then need to loot

if not in combat then need to combat

That's how your logic is currently processed (not in order)

 

So try something such as

if not in combat AND no arrows on ground then combat

if not in combat AND arrows on ground then loot

or this is what i do in my personal scripts since i usually prioritize loot over combat

if arrows on ground then loot and restart loop

if not in combat then combat

Basically what the above does (the 2nd psuedo-snippet) is anytime there is loot it will loot BEFORE combat and restart the loop which will cause it to check for loot again...repeated until there is no more loot THEN it'll combat

[mrgeorge]

On 5/20/2017 at 9:15 PM, Isolate said:

whole situation is scary. to me

I am sorry but may I ask what is the reason, just starting this as my first script. 

 

16 hours ago, Polymorphism said:

Since this is a looping structure, such as Isolate stated -- the code will always run back to back and your issue is the way you've decided to handling looting.

 

Now consider this...

if there are arrows on ground then need to loot

if not in combat then need to combat

That's how your logic is currently processed (not in order)

 

So try something such as

if not in combat AND no arrows on ground then combat

if not in combat AND arrows on ground then loot

or this is what i do in my personal scripts since i usually prioritize loot over combat

if arrows on ground then loot and restart loop

if not in combat then combat

Basically what the above does (the 2nd psuedo-snippet) is anytime there is loot it will loot BEFORE combat and restart the loop which will cause it to check for loot again...repeated until there is no more loot THEN it'll combat

Thanks Polymorphism, like your name