hi guys,

 

I made a chicken killer to try and familiarise myself with the API and learn how to do stuff and I think its just about done. It just ran for 2 hours no problem so im wanting to move on and make something else now. before I do I was wondering if anyone could have a really quick look at the code to see if theres anything I should do differently next time

import org.osbot.rs07.api.filter.Filter;
import org.osbot.rs07.api.map.Area;
import org.osbot.rs07.api.model.NPC;
import org.osbot.rs07.api.ui.Skill;
import org.osbot.rs07.script.Script;
import org.osbot.rs07.script.ScriptManifest;
import java.awt.*;
import java.util.concurrent.TimeUnit;

@ScriptManifest(name = "EmpiresChickenKiller", author = "Empires", version = 1.0, info = "Basic Chicken Killer for Falador Farm", logo = "") 
public class EmpiresChickenKiller extends Script {
	
	private long timeBegan;
	private long timeRan;
	private int beginningXp;
	private int currentXp;
	private int xpGained;
	private int currentLevel;
	private int beginningLevel;

	final Area CHICKEN_COOP = new Area(3014,3298,3020,3282);
	
    @Override
    public void onStart() {
        log("Welcome to EmpiresChickenKiller.");
        log("Lets get started...");
        
        timeBegan = System.currentTimeMillis();
        beginningXp = skills.getExperience(Skill.STRENGTH);
        beginningLevel = skills.getStatic(Skill.STRENGTH);
    }
    
    private enum State {
    	ATTACK, WAIT
    };
    
    private State getState(){
    	if(myPlayer().isMoving() || combat.isFighting() || myPlayer().isUnderAttack())
    		return State.WAIT;	 	
    	else
    		return State.ATTACK;
    }
    
    @SuppressWarnings("unchecked")
	private NPC getTarget(){
		NPC targetChicken = npcs.closest(new Filter<NPC>(){
			public boolean match(NPC theChicken){
				return theChicken != null && theChicken.getName().equals("Chicken") && theChicken.isAttackable() && (theChicken.getHealth() != 0) && CHICKEN_COOP.contains(theChicken);
			}
		});
    	return targetChicken;
    }

    @Override
    public int onLoop() throws InterruptedException {

    	switch(getState()) {
    	case ATTACK:	
    		NPC chickenToKill = getTarget();
    		
    		if(!chickenToKill.isOnScreen())
    			camera.toEntity(chickenToKill);
    		
    		chickenToKill.interact("Attack");
    		sleep(random(600,1000));
    		
    		if(chickenToKill.isInteracting(myPlayer()))
    				sleep(random(1200,2100));
    			else
    				break;
    			
    	case WAIT:
    		sleep(random(400,600));
    		break;		
    	}
        return 100; 
    }
    
    @Override
    public void onExit() {
        log("Thanks for using my script!");
    }

    @Override
    public void onPaint(Graphics2D g) {
    	timeRan = System.currentTimeMillis() - this.timeBegan;
    	g.drawString("Runtime: " + ft(timeRan), 561,221);
    	
    	currentXp = skills.getExperience(Skill.STRENGTH); 
    	xpGained = currentXp - beginningXp; 
    	g.drawString("XP Gained: " + xpGained, 561,246);
    	
    	currentLevel = skills.getStatic(Skill.STRENGTH);
    	g.drawString("Lvl Started: " + beginningLevel, 561,271);
    	g.drawString("Current Lvl: " + currentLevel, 561,296);
    	g.drawString("Lvls Gained: " + (beginningLevel - currentLevel), 561,321);
    }
    
    private String ft(long duration) 
	{
		String res = "";
		long days = TimeUnit.MILLISECONDS.toDays(duration);
		long hours = TimeUnit.MILLISECONDS.toHours(duration)
		- TimeUnit.DAYS.toHours(TimeUnit.MILLISECONDS.toDays(duration));
		long minutes = TimeUnit.MILLISECONDS.toMinutes(duration)
		- TimeUnit.HOURS.toMinutes(TimeUnit.MILLISECONDS
		.toHours(duration));
		long seconds = TimeUnit.MILLISECONDS.toSeconds(duration)
		- TimeUnit.MINUTES.toSeconds(TimeUnit.MILLISECONDS
		.toMinutes(duration));
		if (days == 0) {
		res = (hours + ":" + minutes + ":" + seconds);
		} else {
		res = (days + ":" + hours + ":" + minutes + ":" + seconds);
		}
		return res;
	} 

}

I basically copy/pasted the paint stuff from a different tutorial btw, but I understand it

i didnt look at all of it but for your paint/xp/levels, take a look at the ExperienceTracker class in the API, it has the methods for what youre trying to do

 

http://osbot.org/api/org/osbot/rs07/api/util/ExperienceTracker.html

gonna run for a time and see

 

timebegan couldve been final, and coop should be private, but those are minor.

all in all, looks really good... +1 like. i remember seeing your post about the gui earlier as well. nice work

i didnt look at all of it but for your paint/xp/levels, take a look at the ExperienceTracker class in the API, it has the methods for what youre trying to do

 

http://osbot.org/api/org/osbot/rs07/api/util/ExperienceTracker.html

 

thanks for that. ill be sure to use it next time

gonna run for a time and see

 

good! do let me know how it gets on. I ran it for about 90 mins earlier with no problems so lemme know if something does come up. thanks man 

timebegan couldve been final, and coop should be private, but those are minor.

all in all, looks really good... +1 like. i remember seeing your post about the gui earlier as well. nice work

 

that does make sense.  ill make sure I get the types right next time thanks a lot man! 

Looks dank m8

hi guys,

 

I made a chicken killer to try and familiarise myself with the API and learn how to do stuff and I think its just about done. It just ran for 2 hours no problem so im wanting to move on and make something else now. before I do I was wondering if anyone could have a really quick look at the code to see if theres anything I should do differently next time

 

I basically copy/pasted the paint stuff from a different tutorial btw, but I understand it

 

1) You should have used ExperienceTracker for tracking xp, levels in skills etc.

2) The format time method is ugly

 

For the above two points, Consider reading my Paint tutorial http://osbot.org/forum/topic/87697-explvs-dank-paint-tutorial/

 

3) You are using fixed sleeps every where, you should use ConditionalSleep instead where appropriate, so that you can sleep until a condition is true, or for a timeout. 

 

4) There is no point using "States" here, a single if condition would have done the trick, and generally states don't really make anything more readable.

 

5) Although you have mostly done it, I would recommend splitting your code up into more methods. A method should only be doing one thing. For example, your attacking code should be in a separate method.

 

6) Adding log statements in onStart() and onExit() like "Welcome to blah". Obviously, this is not an issue, but I don't understand why people do it. When you start a script the logger will already print "Script Started blah", and when you stop it "Script stopped blah". So overriding the onExit() method just to add that seems pointless. 

Edited August 4, 20169 yr by Explv

Good going for a first script got the basics down there.

 

There isn't a whole lot to critique, but for me there are two main points you should bare in mind:

1) ConditionalSleep. Make a habit of using this, it prevents many bugs and makes the script appear far more responsive.

2) Null checks. For the most part, you have done it. However, what happens if there are no valid chickens nearby? findNpc() will return null and the script will break when you try and interact with it.

 

Using states for this short script is more hassle than it's worth. As Explv stated, it doesn't add anything. If you were to add to this with more possible states, then an argument can be made for them.

 

As I said at the beginning though, good shit