I'm writing a goblin killer after my chicken killer worked out and I'm having a weird problem.

Sometimes it loots coins and buries bones as I expect it to, but most of the time it skips that step and just attacks another goblin. I can't really tell what's going on and I've made a bunch of minor adjustments to the code trying to figure it out. Can anyone give me a hint?

 

I tried moving the calls to loot() and buryBones() to within the attackGoblin() void, as well as some random shit like adding that whole "currentGoblin" thing so it didn't pick a new goblin before the bones/coins actually spawn, but I'm at a loss.

 

Code:

Spoiler

	public int onLoop() throws InterruptedException {

		//skillHandler(currentSkill);

		if (dialogues.isPendingContinuation()) {
			dialogues.clickContinue();
		}

		goblin = npcs.closest("goblin");
		
		if (goblin != null && goblin.exists() && goblin.getHealthPercent() > 0 && !myPlayer().isInteracting(goblin)
				&& !myPlayer().isUnderAttack() && !goblin.isUnderAttack() && map.canReach(goblin) && goblinArea.contains(goblin))
		{
			attackGoblin(goblin);
		}
		
		if (lootOption) {
			loot();
		}

		if (buryBonesOption) {
				buryBones();
		}

		return random(200, 300);
	}

	private void attackGoblin(NPC currentGoblin) throws InterruptedException {
		action = "Attacking";
		currentGoblin.interact("Attack");
		antiBan();
		new ConditionalSleep(25000, 500) {
			@Override
			public boolean condition() throws InterruptedException {
				goblinPosition = currentGoblin.getPosition();
				return !currentGoblin.exists();
			}
		}.sleep();
		antibanAction = "None";
		killCount += 1;
		
	}

	private void loot() throws InterruptedException {
		GroundItem item = getGroundItems().closest(lootItemName);
		if (item != null && item.getPosition().equals(goblinPosition) && !inventory.isFull()) {
			action = "Looting";
			item.interact("Take");
			new ConditionalSleep(2500, 500) {
				@Override
				public boolean condition() throws InterruptedException {
					return !item.exists();
				}
			}.sleep();
		}
	}

	private void buryBones() throws InterruptedException {
		GroundItem bones = getGroundItems().closest("Bones");
		if (bones != null && bones.getPosition().equals(goblinPosition) && !inventory.isFull()) {
			action = "Burying";
			bones.interact("Take");
			new ConditionalSleep(3500, 250) {
				@Override
				public boolean condition() throws InterruptedException {
					return !bones.exists();
				}
			}.sleep();
			inventory.getItem("Bones").interact("Bury");
			sleep(random(200, 800));
		}
	}

 

 

Edited November 5, 20187 yr by Dot

I think what might happen is that it attacks a target. When your target is doing a dying animation, it looks for the next target to attack and attacks the next target when such a target is found. The previous target will drop loot after your next target is already chosen, and thus the goblinPosition variable is already overwritten by the new target's position, therefore the if-statement in your loot() method will not be satisfied. The first thing and (to me) most obvious thing to recommend is that you wait until your target has dropped the loot, but there are also other ways to make it work.

tldr: you have to write your code to loot and to bury when you have met those conditions ELSE you fight the goblins

Any suggestions on how to wait until it's dropped loot? I didn't run into this problem with my chicken killer and I'm not seeing any difference in the code that should cause this.

Just now, dreameo said:

tldr: you have to write your code to loot and to bury when you have met those conditions ELSE you fight the goblins

That would work. Is that the standard practice?

What you might do is, and also what @dreameo is suggesting:

if (!myPlayer.isUnderAttack() {
	if (lootOption) {
		loot();
	} else if (buryBonesOption) {
		buryBones();
	} else if (goblin != null && goblin.exists() && goblin.getHealthPercent() > 0 && !myPlayer().isInteracting(goblin)
			&& !goblin.isUnderAttack() && map.canReach(goblin) && goblinArea.contains(goblin))
	{
		attackGoblin(goblin);
	}
}

This will solve your problem. This might, however, introduce a new problem. You'll have to test it.

Thanks a lot guys. I can definitely work that into my script.

8 minutes ago, Dot said:

Thanks a lot guys. I can definitely work that into my script.

Yeah lol was in game and had to rush but what @feggit posted is about right.

Thanks dudes.

My lootOption and buryBonesOption are actually just booleans pulled from the GUI to decide whether the user wants the script to bury bones, but I added a "flag" for those functions and got it working.

 

		if (!myPlayer().isUnderAttack()) {
			
			if (lootOption && !looted) {
				loot();
				looted = true;
			}
	
			if (buryBonesOption && !buried) {
				buryBones();
				buried = true;
			}
			
			if (goblin != null && goblin.exists() && goblin.getHealthPercent() > 0 && !myPlayer().isInteracting(goblin)
					&& !goblin.isUnderAttack() && map.canReach(goblin) && goblinArea.contains(goblin)
					&& looted && buried)
			{
				attackGoblin(goblin);
				looted = false;
				buried = false;
			}
		}

 

Just in case anyone sees this in the future, I was still having the same problem even after doing this, so I added a short sleep() to that start of my loot() and buryBones() functions. Just as @feggit suggested, my script was picking a new target before the items actually appeared on screen. I don't know if there's a better API function for this, but the sleep works fine.

 

Thanks again guys.

I think it would help if you made it so all your loop method does is run a bunch of checks and makes a bunch of calls to other functions. Don't handle anything else. That way, you'll know precisely what your script is doing:

@Override
public int onLoop() throws InterruptedException {

	if (dialogues.isPendingContinuation()) {
		dialogues.clickContinue();
		
	} else if (isFighting()) {
		if (isLowOnHealth()) {
			if (canHeal()) {
				heal();
			} else {
				runAway();
			}
		}
		
	
	} else if (areBonesPresentInInventory()) {
		if (!isIdle()) {
			buryNextBone();
		}
		
	} else if (lootingEnabled && isLootPresent()) {
		if (isInventoryFull()) {
			if (isJunkPresent()) {
				dropJunk();
			} else {
				goBankStuff();
			}
		} else {
			loot();
		}
	
	} else {
		fight();
	}

}

 

Thanks for the suggestion. I actually did put my changes to the booleans within the functions themselves but moved them out just to show what I did.

 

I will make cleaning my code up with better functions a priority in my next script.

Somehow still having this problem on my third script. The only thing that makes it work is to have an insanely high ConditionalSleep, which I don't think is proper.

	@Override
	public int onLoop() throws InterruptedException {

		skillHandler();
		
		if (!banking) {
			if (dialogues.isPendingContinuation())
				dialogues.clickContinue();
			else if (!combat.isFighting()) {
				if (lootEnabled && !looted)
					loot();
				else if (bonesEnabled && !buried)
					buryBones();
				else if (looted && buried)
					attack();
			} else if (foodEnabled)
				foodHandler();
		} else if (!inventory.contains(foodName) && !combat.isFighting())
			bankHandler();

		return random(200, 300);
	}
	
	@SuppressWarnings("unchecked")
	private void attack() throws InterruptedException {
		antibanAction = "None";
		target = npcs.closest(n -> n.getName().equalsIgnoreCase(targetName) && n.exists()
								&& n.getHealthPercent() > 0 && !myPlayer().isInteracting(n)
								&& !n.isUnderAttack() && n != null);
		if (!map.canReach(target)) doorHandler.getNextObstacle(target);
		else {
			action = "Attacking";
			target.interact("Attack");
			skipKill = false;
			if (lootEnabled) looted = false;
			if (bonesEnabled) buried = false;
			antiBan();
			new ConditionalSleep(() -> targetPosition = target.getPosition(), 5000, 100) {
				@Override
				public boolean condition() throws InterruptedException {
					if (!combat.isFighting() && target.isUnderAttack()) {
						log("Someone else is fighting that. Looping.");
						skipKill = true;
					}
					return combat.isFighting() || skipKill;
				}
			}.sleep();
			if (!skipKill) killCount += 1;
		}
	}

 

The looted & buried vars are only set within the loot() and bury() functions, but somehow they reset to true even with a 2-3 second sleep time at the start of those functions.

onLoop() is setup in a way that attack() can't be called until loot() and bury() complete. I have tried adding ConditionalSleeps to those functions to wait until the item exists but that seems to have no effect (without a huge timeout).

I'm getting really frustrated. The fuck am I doing wrong?

Edit: I seem to be getting a massive chain of NPE's when my script doesn't loot/bury. No info other than "[ERROR][Bot #1][11/12 08:51:38 PM]: Error in script executor! java.lang.NullPointerException"

 

Edit 2: Huge oversight on my part by having the targetPosition only get updated during the ConditionalSleep and not at any point after attack() has returned. Fairly sure that's where my problem lies.

I think I can solve this by adding a hasDied() function that checks the NPC the player is interacting with for a death animation and stores the targetPosition when it happens, and call bury() and loot() when it returns true.

Edited November 14, 20187 yr by Dot