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();
        }
    }
}

 

"Is there something I can do to improve it?"

 

Yes. A lot.

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.

 

 

u may wanna be more specific.. like what happens/etc. what is causing it to not run smooth

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.

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();
}

 

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. 

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, 20178 yr by Prolax

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));

 

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 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:

1. You don't need to check if the seagull is visible and call camera.ToEntity
2. You should be checking if your player is already attacking a seagull before calling attackNpc, not in attackNpc
3. You should null check seagull, just in case there aren't any, as your code currently will throw a NullPointerException
4. It would be cleaner to filter a seagull that isn't under attack in the closest method(), rather than after
5. You should check the the "Attack" interaction is successful before sleeping.
6. Your condition in the ConditonalSleep is incorrect. You should check whether your player is interacting with the seagull, or the seagull is not attackable.
7. 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, 20178 yr by Explv

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 

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:

1. You don't need to check if the seagull is visible and call camera.ToEntity
2. You should be checking if your player is already attacking a seagull before calling attackNpc, not in attackNpc
3. You should null check seagull, just in case there aren't any, as your code currently will throw a NullPointerException
4. It would be cleaner to filter a seagull that isn't under attack in the closest method(), rather than after
5. You should check the the "Attack" interaction is successful before sleeping.
6. Your condition in the ConditonalSleep is incorrect. You should check whether your player is interacting with the seagull, or the seagull is not attackable.
7. 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();
        }
    }
}

 

---- see below ----

 

Edited May 15, 20178 yr by Explv

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();
        }
    }
}