Prolax Posted May 15, 2017 Share Posted May 15, 2017 My AutoFighter script is not running smooth. Is there something I can do to improve it? Here is the current script / method: public final class ProlaxAutoFighter extends Script { @Override public final int onLoop() throws InterruptedException { attackNpc(); return random(300,800); } public void attackNpc(){ NPC seagull = getNpcs().closest("Seagull"); if(!seagull.isVisible()) { camera.toEntity(seagull); } if(!myPlayer().isUnderAttack() && seagull.isAttackable() && seagull != null && seagull.exists()){ seagull.interact("Attack"); new ConditionalSleep(5000) { @Override public boolean condition() { return myPlayer().isAnimating() || !seagull.exists(); } }.sleep(); } } } Quote Link to comment Share on other sites More sharing options...
Pseudo Posted May 15, 2017 Share Posted May 15, 2017 "Is there something I can do to improve it?" Yes. A lot. 1 Quote Link to comment Share on other sites More sharing options...
Magarac Posted May 15, 2017 Share Posted May 15, 2017 You should make it State based with two cases at least: a sleep one and a fight one. If myPlayer.isFighting(), then just sleep a small bit and then check again, otherwise if not fighting then initiate a fight. You can also create a filter for the NPC to make it cleaner and make sure to check that npc.getHealth > 0. Also I'm pretty sure .exists() is covered when you call != null. Quote Link to comment Share on other sites More sharing options...
trickked Posted May 15, 2017 Share Posted May 15, 2017 u may wanna be more specific.. like what happens/etc. what is causing it to not run smooth Quote Link to comment Share on other sites More sharing options...
Prolax Posted May 15, 2017 Author Share Posted May 15, 2017 11 minutes ago, Pseudo said: "Is there something I can do to improve it?" Yes. A lot. I was only asking advice on this combat method. I deleted the other methods. 11 minutes ago, Magarac said: You should make it State based with two cases at least: a sleep one and a fight one. If myPlayer.isFighting(), then just sleep a small bit and then check again, otherwise if not fighting then initiate a fight. You can also create a filter for the NPC to make it cleaner and make sure to check that npc.getHealth > 0. Also I'm pretty sure .exists() is covered when you call != null. I think states are not really needed, I'm just calling methods in my onLoop. Quote Link to comment Share on other sites More sharing options...
Prolax Posted May 15, 2017 Author Share Posted May 15, 2017 The actual problem is it sometimes spamclicks the npc 2-3 times. Probably something wrong with my conditional sleep public boolean condition() { return myPlayer().isAnimating() || !seagull.exists(); } Quote Link to comment Share on other sites More sharing options...
Magarac Posted May 15, 2017 Share Posted May 15, 2017 10 minutes ago, Prolax said: The actual problem is it sometimes spamclicks the npc 2-3 times. Probably something wrong with my conditional sleep public boolean condition() { return myPlayer().isAnimating() || !seagull.exists(); } States would only make it better to allow a sleep portion so it isn't always looping to the fight part; but as for your sleep condition there have it return if getCombat().isFighting() instead. Quote Link to comment Share on other sites More sharing options...
Prolax Posted May 15, 2017 Author Share Posted May 15, 2017 (edited) 16 minutes ago, Magarac said: States would only make it better to allow a sleep portion so it isn't always looping to the fight part; but as for your sleep condition there have it return if getCombat().isFighting() instead. Running ok now. Final method: What is a good way to turn camera to npc, when npc is not visible? public void attackNpc(){ NPC seagull = getNpcs().closest("Seagull"); if(!getCombat().isFighting() && !myPlayer().isUnderAttack() && myPlayer().getInteracting() == null && seagull.isAttackable() && seagull.exists() && seagull.isVisible()){ seagull.interact("Attack"); new ConditionalSleep(4000,500) { @Override public boolean condition() throws InterruptedException { return !seagull.exists() || seagull.isUnderAttack(); } }.sleep(); } } Edited May 15, 2017 by Prolax Quote Link to comment Share on other sites More sharing options...
Magarac Posted May 15, 2017 Share Posted May 15, 2017 9 minutes ago, Prolax said: public boolean condition() { return !getCombat().isFighting(); } ^ Like this No. You want to attack the NPC, and then sleep until it is attacked. Then you want to sleep until fighting is done. // if combat is not fighting case FIGHT: NPC seagull = .... if (seagull != null) { seagull.interact("Attack"); conditionalSleep() { private boolean condition() { return getCombat.isFighting() || seagull == null; } } } // if combat is fighting case WAIT: sleep(random(400, 800)); 1 Quote Link to comment Share on other sites More sharing options...
Prolax Posted May 15, 2017 Author Share Posted May 15, 2017 30 minutes ago, Magarac said: No. You want to attack the NPC, and then sleep until it is attacked. Then you want to sleep until fighting is done. // if combat is not fighting case FIGHT: NPC seagull = .... if (seagull != null) { seagull.interact("Attack"); conditionalSleep() { private boolean condition() { return getCombat.isFighting() || seagull == null; } } } // if combat is fighting case WAIT: sleep(random(400, 800)); Thanks for the info. Final method: public void attackNpc(){ NPC seagull = getNpcs().closest("Seagull"); if(!seagull.isVisible()){ camera.toEntity(seagull); } if(!getCombat().isFighting() && !myPlayer().isUnderAttack() && seagull.exists() && !seagull.isUnderAttack() && seagull.getHealthPercent() > 70){ seagull.interact("Attack"); new ConditionalSleep(2000,500) { @Override public boolean condition() throws InterruptedException { return getCombat().isFighting() || seagull == null; } }.sleep(); } } 1 Quote Link to comment Share on other sites More sharing options...
Explv Posted May 15, 2017 Share Posted May 15, 2017 (edited) 1 hour ago, Magarac said: You should make it State based with two cases at least: a sleep one and a fight one. States are unnecessary and messy, cleanest way is: if not attacking seagull: attack seagull else: relax 16 minutes ago, Prolax said: Thanks for the info. Final method: public void attackNpc(){ NPC seagull = getNpcs().closest("Seagull"); if(!seagull.isVisible()){ camera.toEntity(seagull); } if(!getCombat().isFighting() && !myPlayer().isUnderAttack() && seagull.exists() && !seagull.isUnderAttack() && seagull.getHealthPercent() > 70){ seagull.interact("Attack"); new ConditionalSleep(2000,500) { @Override public boolean condition() throws InterruptedException { return getCombat().isFighting() || seagull == null; } }.sleep(); } } That's pretty good, but: You don't need to check if the seagull is visible and call camera.ToEntity You should be checking if your player is already attacking a seagull before calling attackNpc, not in attackNpc You should null check seagull, just in case there aren't any, as your code currently will throw a NullPointerException It would be cleaner to filter a seagull that isn't under attack in the closest method(), rather than after You should check the the "Attack" interaction is successful before sleeping. Your condition in the ConditonalSleep is incorrect. You should check whether your player is interacting with the seagull, or the seagull is not attackable. Take a look at the isAttackable() method for NPCs, it will perform all the checks such as exists, not under attack and has health > 0 for you. NPC seagull = getNpcs().closest(npc -> npc.getName().equals("Seagull") && npc.isAttackable()); if (seagull != null && seagull.interact("Attack")) { new ConditionalSleep(5000) { @Override public boolean condition() { return myPlayer().isInteracting(seagull) || !seagull.isAttackable(); } }.sleep(); } Edited May 15, 2017 by Explv 2 Quote Link to comment Share on other sites More sharing options...
Magarac Posted May 15, 2017 Share Posted May 15, 2017 35 minutes ago, Explv said: States are unnecessary and messy, cleanest way is: if not attacking seagull: attack seagull else: relax To each his own; I prefer how States look and work and they'd accomplish the same task as that and wouldn't be much more complicated for such a simple script tbh. Works either way though Quote Link to comment Share on other sites More sharing options...
Prolax Posted May 15, 2017 Author Share Posted May 15, 2017 41 minutes ago, Explv said: States are unnecessary and messy, cleanest way is: if not attacking seagull: attack seagull else: relax That's pretty good, but: You don't need to check if the seagull is visible and call camera.ToEntity You should be checking if your player is already attacking a seagull before calling attackNpc, not in attackNpc You should null check seagull, just in case there aren't any, as your code currently will throw a NullPointerException It would be cleaner to filter a seagull that isn't under attack in the closest method(), rather than after You should check the the "Attack" interaction is successful before sleeping. Your condition in the ConditonalSleep is incorrect. You should check whether your player is interacting with the seagull, or the seagull is not attackable. Take a look at the isAttackable() method for NPCs, it will perform all the checks such as exists, not under attack and has health > 0 for you. NPC seagull = getNpcs().closest(npc -> npc.getName().equals("Seagull") && npc.isAttackable()); if (seagull != null && seagull.interact("Attack")) { new ConditionalSleep(5000) { @Override public boolean condition() { return myPlayer().isInteracting(seagull) || !seagull.isAttackable(); } }.sleep(); } Thanks a lot, good to learn. Is this if statement in the onLoop enough? The script clicks twice on the npc though, when attacking. Not sure why atm. public final class ProlaxAutoFighter extends Script { @Override public final int onLoop() throws InterruptedException { if(!myPlayer().isUnderAttack() && !getCombat().isFighting()) { attackNpc(); } return random(100,300); } public void attackNpc(){ NPC seagull = getNpcs().closest(npc -> npc.getName().equals("Seagull") && npc.isAttackable()); if (seagull != null && !seagull.isUnderAttack() && seagull.interact("Attack")) { new ConditionalSleep(5000) { @Override public boolean condition() { return myPlayer().isInteracting(seagull) || !seagull.isAttackable(); } }.sleep(); } } } Quote Link to comment Share on other sites More sharing options...
Explv Posted May 15, 2017 Share Posted May 15, 2017 (edited) ---- see below ---- Edited May 15, 2017 by Explv Quote Link to comment Share on other sites More sharing options...
Prolax Posted May 15, 2017 Author Share Posted May 15, 2017 8 minutes ago, Explv said: You could try checking that your player is interacting with a Seagull instead if (!"Seagull".equals(myPlayer().getInteracting())) { attackNpc(); } Hmm, still attacks the npc twice. Anything I can add? public final class ProlaxAutoFighter extends Script { @Override public final int onLoop() throws InterruptedException { if (!"Seagull".equals(myPlayer().getInteracting()) && !getCombat().isFighting()) { attackNpc(); } return random(100,300); } public void attackNpc(){ NPC seagull = getNpcs().closest(npc -> npc.getName().equals("Seagull") && npc.isAttackable()); if (seagull != null && !seagull.isUnderAttack() && !seagull.isAnimating() && seagull.interact("Attack")) { new ConditionalSleep(5000) { @Override public boolean condition() { return myPlayer().isInteracting(seagull) || !seagull.isAttackable(); } }.sleep(); } } } Quote Link to comment Share on other sites More sharing options...