Jump to content

AutoFighter script not running smooth


Prolax

Recommended Posts

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

 

Link to comment
Share on other sites

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.

 

 

Link to comment
Share on other sites

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.

Link to comment
Share on other sites

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. 

Link to comment
Share on other sites

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 by Prolax
Link to comment
Share on other sites

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

 

  • Like 1
Link to comment
Share on other sites

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

 

  • Like 1
Link to comment
Share on other sites

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 by Explv
  • Like 2
Link to comment
Share on other sites

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

Link to comment
Share on other sites

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

 

Link to comment
Share on other sites

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

 

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...