Looting problem

[liljeep]

Trying to upgrade my fighting script with looting capabilities, but for some reason, adding the loot task has made my bot now freeze where I cannot even move the window or click anything, and eventually crashes the bot. Something is obviously wrong with my code

here is my task list

if(userChoices.equals("Fire giant")) {
            //tasks.add(new Idle(this));
            tasks.add(new Eat(this));
            tasks.add(new FightFiregiant(this));
            tasks.add(new Loot(this));
        }

And here is my loot task code

public class Loot extends Task {

    public int GroundID[] = {
            10006, 303, 954, 11940, 560, 563, 565, 890, 561, 207, 5295, 5300, 5304, 5315, 211, 213, 217, 215, 2485,
            1079, 1113, 1127, 1147, 1163, 1185, 1201, 1213, 1247, 1275, 1289, 1333, 1347, 1359, 1373, 4131, 3122, 1393, 1395, 1397, 1399,
            892, 562, 1147, 985, 987, 2366, 2368, 22347, 4151, 1249, 13265, 1149, 1319,
            4153, 4154, 4101, 445, 2357, 2354, 2359,
            451, 2358, 444, 2356, 2360, 1073
    };

    public Loot(Fighter script) {
        super(script);
    }

    @Override
    public boolean canProcess() throws InterruptedException {
        return  script.getGroundItems().closest(GroundID) != null && script.getGroundItems().closest(GroundID).isOnScreen();
                //script.getGroundItems().closest(GroundID).isVisible() ;
    }

    @Override
    public void process() throws InterruptedException {
        script.currentState = Status.LOOTING;
        GroundItem loot = script.getGroundItems().closest(GroundID);
        //if (GroundID != null && !script.getInventory().isFull())

        if (!script.inventory.isFull()){
            loot.interact("Take");
            sleep(random(999,1777));
        }
    }
}

And help would be appreciated! THANKS!

[Khaleesi]

Ok so for this function:

@Override
    public boolean canProcess() throws InterruptedException {
        return  script.getGroundItems().closest(GroundID) != null && script.getGroundItems().closest(GroundID).isOnScreen();
                //script.getGroundItems().closest(GroundID).isVisible() ;
    }

You are loading a new item 2 times, if you would be walking it could be loading different items or even return Null on the isOnScreen method. + You should check in here if your inventory is full or not.
A better way is to load it once and check on it. is this

@Override
    public boolean canProcess() throws InterruptedException {
		if(script.getInvenotry().isFull){
			return false;
		}

		grountItem item = script.getGroundItems().closest(GroundID);
        return  item != null && item.isOnScreen();
    }

__________________________________________________________________________________________________________________________________________________________________

For the next Part:

@Override
    public void process() throws InterruptedException {
        script.currentState = Status.LOOTING;
        GroundItem loot = script.getGroundItems().closest(GroundID);
        //if (GroundID != null && !script.getInventory().isFull())

        if (!script.inventory.isFull()){
            loot.interact("Take");
            sleep(random(999,1777));
        }
    }

You are not Null checking if there is loot on the ground. (Commented out)
Also move the inventory check to canProcess, else it might get stuck in trying to loot but your inventory is full so it will never loot.

@Override
    public void process() throws InterruptedException {
        script.currentState = Status.LOOTING;
        GroundItem loot = script.getGroundItems().closest(GroundID);

        if (loot != null){
            loot.interact("Take");
            sleep(random(999,1777));
        }
    }

Try to consider using Conditional sleeps instead of a random sleep.
This might click the loot multiple times for no reason.

You could do this instead:

@Override
    public void process() throws InterruptedException {
        script.currentState = Status.LOOTING;
        GroundItem loot = script.getGroundItems().closest(GroundID);

        if (loot != null){
           if(loot.interact("Take")){
                    int prevAmount = script.getInventory().getAmount(loot.getName());
                    new ConditionalSleep(random(5000, 7500)) {
                        @Override
                        public boolean condition() throws InterruptedException {
                            return script.getInventory().getAmount(loot.getName()) > prevAmount;
                        }
                    }.sleep();
                }
          }
    }

I Hope this helped you ^^

Edited January 5, 2021 by Khaleesi

[liljeep]

5 hours ago, Khaleesi said:

Ok so for this function:

@Override
    public boolean canProcess() throws InterruptedException {
        return  script.getGroundItems().closest(GroundID) != null && script.getGroundItems().closest(GroundID).isOnScreen();
                //script.getGroundItems().closest(GroundID).isVisible() ;
    }

You are loading a new item 2 times, if you would be walking it could be loading different items or even return Null on the isOnScreen method. + You should check in here if your inventory is full or not.
A better way is to load it once and check on it. is this

@Override
    public boolean canProcess() throws InterruptedException {
		if(script.getInvenotry().isFull){
			return false;
		}

		grountItem item = script.getGroundItems().closest(GroundID);
        return  item != null && item.isOnScreen();
    }

__________________________________________________________________________________________________________________________________________________________________

For the next Part:

@Override
    public void process() throws InterruptedException {
        script.currentState = Status.LOOTING;
        GroundItem loot = script.getGroundItems().closest(GroundID);
        //if (GroundID != null && !script.getInventory().isFull())

        if (!script.inventory.isFull()){
            loot.interact("Take");
            sleep(random(999,1777));
        }
    }

You are not Null checking if there is loot on the ground. (Commented out)
Also move the inventory check to canProcess, else it might get stuck in trying to loot but your inventory is full so it will never loot.

@Override
    public void process() throws InterruptedException {
        script.currentState = Status.LOOTING;
        GroundItem loot = script.getGroundItems().closest(GroundID);

        if (loot != null){
            loot.interact("Take");
            sleep(random(999,1777));
        }
    }

Try to consider using Conditional sleeps instead of a random sleep.
This might click the loot multiple times for no reason.

You could do this instead:

@Override
    public void process() throws InterruptedException {
        script.currentState = Status.LOOTING;
        GroundItem loot = script.getGroundItems().closest(GroundID);

        if (loot != null){
           if(loot.interact("Take")){
                    int prevAmount = script.getInventory().getAmount(loot.getName());
                    new ConditionalSleep(random(5000, 7500)) {
                        @Override
                        public boolean condition() throws InterruptedException {
                            return script.getInventory().getAmount(loot.getName()) > prevAmount;
                        }
                    }.sleep();
                }
          }
    }

I Hope this helped you ^^

Thanks so much for explaining and going into detail about what I was doing wrong!!! I thought I was checking if item was !null but I guess I was doing grounditem instead and like you said, checking for it twice. And yah, the conditional sleep makes much more sense so the bot can resume once item is in inventory rather than clicking multiple times possibly. Thanks so much! you are a lifesaver :)

[Nbacon]

I just had to solve this problem for myself in my bot. I don't know if it will help but  This will still loot an item  if your is full but you want to pick up a noted item. You can change it to eat food to loot or drop items if your inv is full(second return false is were that logic should go).

 The code I just wrote

Quote

(defn pick-up-item
  [ob]
  (define item-inv-check [id]
          (let [item (. Inventory (getItem (int-array (list id))))]
            (and
              (not (nil? item))
              (or (= (. (. item getDefinition) getNotedId) -1) (. item isNote)))))
  
  (let [emptySlots (. Inventory (getEmptySlots))
        itemOnGround (. GroundItems
                        (closest (if (number? ob)
                                   (int-array (int ob))
                                   (into-array (list ob)))))]
    (if (nil? itemOnGround)
      false
      (if
        (or (not (= emptySlots 0)) (item-inv-check (. itemOnGround getId)))
        (if
          (. itemOnGround (interact (into-array (list "Take"))))
          (sleep #(not (. itemOnGround (exists))) 5000 200)
          false)
        false))))

 

 

 

 

The java varent. 

Quote

    boolean item_inv_check(int id) {
        Item item = getInventory().getItem(id);
        return ((null != item) && (item.getDefinition().getNotedId() == -1 || item.isNote()));
    }

    boolean loot(int... id) {
        int emptySlots = getInventory().getEmptySlots();
        GroundItem ff = getGroundItems().closest(id);
        if (ff == null) {
            return false;
        } else {
            if ((emptySlots != 0 || item_inv_check(ff.getId())) && ff.interact("Take")) {
                Sleep.sleepUntil(() -> !ff.exists(), 5000, 200);
                return !ff.exists();
            } else {
                return false;
            }
        }
    }