Attacking code that works separate but not together?

[tammyt 0]

I am very new to scripting as many who post on here seem to be. So do bear with me.

I have 2 methods that attack an NPC the first one that attacks a new closest npc in an area:

private NPC getNextTarget() {
        return getNpcs().closest(n -> n != null && attackArea.contains(n) && n.getName().contains("Moss giant") && !n.isInCombat());
    }

and then if I need to resume attacking an NPC I was previously in combat with:

private NPC getCurrentTarget() {
        return getNpcs().closest(c -> c != null && attackArea.contains(c) && c.isInteracting(getLocalPlayer()) && c.getName().contains("Moss giant"));
    }

when I test both of these parts of code separately they both work as intended. but as soon as I put them both together with an if statement it doesn't. the if itself seems to work but the else doesn't. Many variations where one part will work and the other wont. But both work just fine separately

case ATTACK:
            log("Not in combat");
            if (getCurrentTarget().interact("Attack")) {
                log("Resume attacking Moss Giant");
            } else {
                log("Start attacking Moss Giant");
                getNextTarget().interact("Attack");
            }
            break;

case ATTACK:
            log("Not in combat");
            if (getCurrentTarget().exists()) {
                log("Resume attacking Moss Giant");
                getCurrentTarget().interact("Attack");
            } else {
                log("Start attacking Moss Giant");
                getNextTarget().interact("Attack");
            }
            break;

both work to resume attacking the current target but will not attack a new target if a current target doesn't exist. If I comment out the current target part, the script will find a new target with no issue either. I've also tried a single if with a break; in it and then the getNextTarget() outside of the if statement but nothing seems to work at all. i am so confused. If i comment out the if statement altogether:

case ATTACK:
                log("Not in combat");
                log("Resume attacking Moss Giant");
                getCurrentTarget().interact("Attack");
                log("Start attacking Moss Giant");
                getNextTarget().interact("Attack");
            break;

it logs as only: "Not in combat" and "Resume attacking Moss Giant" and never goes any further, so it must end at that stage. But even if I create a separate case with "if (getCurrentTarget().exists() && !getLocalPlayer().isInCombat())" to return to the resume attack case and "if (!getLocalPlayer().isInCombat())" to return to attack it still breaks. so I have no idea at this stage where to go next.

Sorry for the long post but as you can imagine I've been trying to fix this myself for a while now. Any help is much appreciated.

[tammyt 0]

I never thought to give information or look for errors being logged. "Exception has occurred while running! Please report error to developer if problem persists:java.lang.NullPointerExceptionat Main.onLoop(Main.java:82)at org.dreambot.api.script.AbstractScript.java:264)at java.lang.Thread.run(Thread.java:748)"

line 82 is the end of my if statement, line 264 and 748 are part of the includes I assume as my script doesn't go that far.

[tammyt 0]

After a good night sleep and another attempt I realised I was having a type mismatch in my if statement. Trying to use the NPC as a boolean. If anybody is curious here is the code I fixed it with if anybody is curious. Also if there is a better way of doing this I can be told I'm stupid but here it is:

private NPC getCurrentTarget() {
	if (getNpcs().closest(c -> attackArea.contains(c) && c.isInteracting(getLocalPlayer()) && c.getName().contains("Moss giant")) != null) {
		return getNpcs().closest(c -> attackArea.contains(c) && c.isInteracting(getLocalPlayer()) && c.getName().contains("Moss giant"));
	} else {
		return null;
	}
}

if (getCurrentTarget() != null) {
	log(" Resume attacking Moss Giant");
	getCurrentTarget().interact("Attack");
} else {
	log("Start attacking Moss Giant");
	getNextTarget().interact("Attack");
}

 

[Harold 11]

just a little bit of code feedback:

note that you might as well rewrite your first method as

private NPC getCurrentTarget() {
	return getNpcs().closest(c -> attackArea.contains(c) && c.isInteracting(getLocalPlayer()) && c.getName().contains("Moss giant"));
}

the second part is fine, although you could do with just a single call to getCurrentTarget:

NPC currentTarget = getCurrentTarget();
if (currentTarget != null) {
	log("Resume attacking Moss Giant");
	currentTarget.interact("Attack");
} else {
	log("Start attacking Moss Giant");
	getNextTarget().interact("Attack");
}

 

[tammyt 0]

I see I call the method twice there rather than storing that into a variable. That would make a lot more sense and I assume the reason to do that is to be less cpu intensive?. You have helped make clear why the original code didn't work in the first place too. I was excluding a null result and then asking if the result was null in my if statement.  So when I made it manually return null with the else it worked but would have just worked and returned null if I removed "c != null"  in the first place. I feel a bit silly because it seem so obvious now.

Thank you so much for the help and feedback.