First script feedback

[GearsBy]

Sorry if this is the wrong place but I'm looking for feedback on my first script. Other than general feedback I also have a couple of questions, what would be the best way to look for just my loot when fighting in a busy area, and are there any ways to make the script to be more human, for example how can i force a miss click/would this be worth including or would every so often attacking a chicken which is already under attack be worth while, is there anything I should be doing while fighting the chicken?

 

import org.osbot.rs07.api.model.NPC;
import org.osbot.rs07.script.Script;
import org.osbot.rs07.script.ScriptManifest;
import org.osbot.rs07.utility.ConditionalSleep;
import org.osbot.rs07.api.filter.Filter;
import org.osbot.rs07.api.map.Area;

import java.awt.*;



@ScriptManifest(author = "GearsBy", name = "Simple Chicken Slayer", info = "Just an empty script :(", version = 0.1,
        logo = "")





public final class ChickenSlayer extends Script  {

    @Override
    public final void onStart() {
    }

    @Override
    public final int onLoop() throws InterruptedException {

        if (!inChickenPen()){
            walkToChickenPen();
        } else if (getCombat().isFighting()){
            log("Fighting Chicken");
        } else {
            fightChicken();
            new ConditionalSleep(5000, 500) {
                @Override
                public boolean condition() throws InterruptedException {
                    log("Waiting to see if we fight a chicken");
                    return (getCombat().isFighting() || !myPlayer().isMoving()) ;
                }
            }.sleep();

        }

    return random(1000,3000);
    }


    @Override
    public void onPaint(final Graphics2D g) {
   }


    @Override
    public final void onExit() {
        log("This will be printed to the logger when the script exits");
    }

    private boolean inChickenPen(){
        Area chickenPen = new Area(3225, 3301, 3235, 3288);
        if (!chickenPen.contains(myPlayer())){
            log("Player not in area");
            return false;
        } else {
            return true;
        }

    }

    private void walkToChickenPen(){
        Area chickenPen = new Area(3225, 3301, 3235, 3288);
        log("Walking back to pen");
        getWalking().webWalk(chickenPen); //Web as gate may be closed
        log("Back at pen");
    }

    private boolean fightChicken(){
        NPC chicken = npcs.closest(new Filter<NPC>() {
            @Override
            public boolean match (NPC npc) {
                return npc.exists() && npc.getName().equals("Chicken") && npc.isAttackable() && npc.getHealthPercent() >0;
            }
        });
        if (chicken == null){
            log("Found no chickens :(");
            return false;
        } else {
            if(chicken.isOnScreen()) {
                chicken.interact("Attack");
                log("Fight that chicken");
                return true;
            } else {
                //how?
                return false;
            }
        }
    }

}

Lastly any other feature or things that I could add to start challenging myself?

 

Thank you for your time!

[Eagle Scripts]

@Override
    public final void onStart() {
    }

No need to include this if you're simply overriding a method with an empty body. You can remove the entire onStart .
 

^^ Same applies for onPaint.

private void walkToChickenPen(){
        Area chickenPen = new Area(3225, 3301, 3235, 3288);
        log("Walking back to pen");
        getWalking().webWalk(chickenPen); //Web as gate may be closed
        log("Back at pen");
    }

you can move the chickenPen up one level:

private static final Area CHICKEN_PEN = new Area(3225, 3301, 3235, 3288);

 

fightChicken();
            new ConditionalSleep(5000, 500) {
                @Override
                public boolean condition() throws InterruptedException {
                    log("Waiting to see if we fight a chicken");
                    return (getCombat().isFighting() || !myPlayer().isMoving()) ;
                }
            }.sleep();

Since you're already making a separate method for attacking the chicken; why not sleep inside that method as well?

Remove the sleep here.

private boolean fightChicken(){
        NPC chicken = npcs.closest(new Filter<NPC>() {
            @Override
            public boolean match (NPC npc) {
                return npc.exists() && npc.getName().equals("Chicken") && npc.isAttackable() && npc.getHealthPercent() >0;
            }
        });
        if (chicken == null){
            log("Found no chickens :(");
            return false;
        } else {
            if(chicken.isOnScreen()) {
                chicken.interact("Attack");
                log("Fight that chicken");
                return true;
            } else {
                //how?
                return false;
            }
        }
    }

The above method can be refactored to:

private void fightChicken(){
  final NPC chicken = getNpcs().closest(npc -> npc.exists() && npc.getName().equals("Chicken") && npc.isAttackable() && npc.getHealthPercent() > 0);

  if (chicken != null) {
      if (chicken.isOnScreen()) {
          log("Fight that chicken");

          if (chicken.interact("Attack")) {
              new ConditionalSleep(5000, 500) {
                @Override
                public boolean condition() throws InterruptedException {
                    log("Waiting to see if we fight a chicken");
                    return (getCombat().isFighting() || !myPlayer().isMoving()) ;
                }
              }.sleep();
          } 
      }
  } else {
    log("Found no chickens :(");
  }
}

Also; notice how I changed the method's type from boolean to void? There's no point in making it a boolean method if you're not doing anything with its return value .
 

Edited December 8, 2018 by Eagle Scripts

[GearsBy]

Thank you very much