First autofighter. Need criticism

[urmom]

Hello, it's my first ever script. Wrote it in one week. I want to get some criticism, just to improve and see my mistakes. Really appreciate it  want to include picking up items by price later

Spoiler

 

package mypackage;

import org.osbot.rs07.api.map.Position;
import org.osbot.rs07.api.model.GroundItem;
import org.osbot.rs07.api.model.Item;
import org.osbot.rs07.api.model.NPC;
import org.osbot.rs07.api.ui.Skill;
import org.osbot.rs07.api.ui.Tab;
import org.osbot.rs07.script.Script;
import org.osbot.rs07.script.ScriptManifest;
import org.osbot.rs07.utility.ConditionalSleep;

import java.awt.*;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

@ScriptManifest(name = "AIOFighter", author = "urmom", version = 1, info = "ITEM PCKER", logo = "")

public class MainClass extends Script {

	private int deadCount = 0;
	private Skill attmethod = Skill.ATTACK;
	private Font font = new Font("Sans-Serif", Font.BOLD, 10);
	private String npcName = "Goblin", action = "Setup";
	private boolean isLooting, isBury;
	private int percent;
	Position npcPosition;

	@Override
	public void onPaint(Graphics2D g) {
		g.setFont(font);
		g.setColor(Color.WHITE);
		g.drawString("Enemies killed: " + deadCount, 10, 250);
		g.drawString("XpLeft to next lvl: " + skills.experienceToLevel(attmethod), 10, 265);
		g.drawString("Exp gained: " + experienceTracker.getGainedXP(attmethod), 10, 280);
		g.drawString("Levels gained: " + experienceTracker.getGainedLevels(attmethod), 10, 295);
		g.drawString(action, 10, 310);
		g.drawRect(mouse.getPosition().x - 3, mouse.getPosition().y - 3, 6, 6);
		g.drawRect(0, 230, 150, 100);
	}

	@Override
	public void onStart() throws InterruptedException {
		log("onstart");
		GUI gui = new GUI(this);
		gui.setVisible(true);
		while (gui.isVisible()) {
			sleep(4000);
		}
		npcName = gui.getName();
		isLooting = gui.isLooting();
		percent = gui.getPercent();
		isBury = gui.isBury();
		if (npcName == null) {
			stop(false);
		}
		gui.dispose();
		experienceTracker.start(getAttMethod());
	}

	@Override
	public int onLoop() throws InterruptedException {
		action = "Status: Attacking npc";
		NPC npcToAttack = getNpcs().closest(npc -> npc.getName().contains(npcName) && !npc.isUnderAttack()
				&& npc.getHealthPercent() != 0 && map.canReach(npc));
		if (!myPlayer().isUnderAttack() && npcToAttack != null) {
			npcToAttack.interact("Attack");
			new ConditionalSleep(20000, 500) {
				@Override
				public boolean condition() throws InterruptedException {
					return npcToAttack.getHealthPercent() == 0;
				}
			}.sleep();
			if (isLooting) {
				npcPosition = npcToAttack.getPosition();
				new ConditionalSleep(2000) {
					@Override
					public boolean condition() throws InterruptedException {
						return !npcToAttack.exists();
					}
				}.sleep();
			}
			deadCount += 1;
		} else {
			new ConditionalSleep(20000, 1000) {
				@Override
				public boolean condition() throws InterruptedException {
					return !myPlayer().isUnderAttack();
				}
			}.sleep();
		}

		if (isLooting) {
			action = "Status: Looting";
			GroundItem item = getGroundItems()
					.closest(_item -> _item.getName().equals("Iron arrow") && _item.getPosition().equals(npcPosition));
			if (item != null) {
				item.interact("take");
				new ConditionalSleep(5000, 500) {
					@Override
					public boolean condition() throws InterruptedException {
						return !item.exists();
					}
				}.sleep();
			}
		}
		if (isBury) {
			action = "Status: Burying";
			GroundItem item = getGroundItems().closest(a -> a.getName().equals("Bones") && map.canReach(a));
			if (item != null && !inventory.isFull()) {
				item.interact("take");
				new ConditionalSleep(5000, 500) {
					@Override
					public boolean condition() throws InterruptedException {
						return !item.exists();
					}
				}.sleep();
				inventory.getItem("Bones").interact("Bury");
			}
		}
		setRunning();
		eat(percent);
		antiban();

		return 700;
	}

	private void eat(int percent) {
		if (myPlayer().getHealthPercent() < percent) {
			percent = random(60, 90);
			action = "Status: Eating";
			if (tabs.getOpen() != Tab.INVENTORY) {
				tabs.open(Tab.INVENTORY);
			}
			while (myPlayer().getHealthPercent() < percent) {
				List<Item> foodItem = inventory.filter((item -> item.hasAction("Eat")));
				if (!foodItem.isEmpty()) {
					foodItem.get(random(foodItem.size())).interact("Eat");
					new ConditionalSleep(2000, 1000) {
						@Override
						public boolean condition() throws InterruptedException {
							// TODO Auto-generated method stub
							return !myPlayer().isAnimating();
						}
					};
				} else {
					getLogoutTab().logOut();
					stop();
				}

				foodItem = null;
			}
		}
	}

	private void antiban() throws InterruptedException {
		int rnd = random(100);
		if (rnd == 5) {
			action = "Status: antiban hovering training skill";
			hoverskill();
		}
		if (rnd == 38) {
			action = "Status: antiban break";
			getMouse().moveOutsideScreen();
			sleep(random(50000, 90000));
		}
		if (rnd == 98) {
			action = "Status: antiban hovering skill";
			getWidgets().get(320, 9).hover();
		}
	}

	private void hoverskill() throws InterruptedException {
		if (tabs.getOpen() != Tab.SKILLS) {
			tabs.open(Tab.SKILLS);
		}
		switch (attmethod) {
		case ATTACK:
			getWidgets().get(320, 1).hover();
			break;

		case STRENGTH:
			getWidgets().get(320, 2).hover();
			break;

		case DEFENCE:
			getWidgets().get(320, 3).hover();
			break;
		}
		sleep(random(4000, 7000));
	}

	private Skill getAttMethod() {
		switch (getConfigs().get(43)) {
		case 0:
			attmethod = Skill.ATTACK;
			break;
		case 1:
			attmethod = Skill.STRENGTH;
			break;
		case 3:
			attmethod = Skill.DEFENCE;
			break;
		}
		return attmethod;
	}

	private void setRunning() {
		if (!settings.isRunning()) {
			if (settings.getRunEnergy() > random(20, 60)) {
				settings.setRunning(true);
			}
		}
	}

}

 

[urmom]

bump ;L

[Tommm39]

Looks pretty good to me, I would consider using the Viking's Timing class in the place of conditional sleep to make your code a little cleaner

Also maybe look to refactor your code so there's less in your onLoop method, for example my auto fighters looks more like 

public int onLoop() {

if (myPlayer().getHealthPercentage() < 40 ) {

eat();}

else {

attack();}

}

[urmom]

HEy! Thanks  Already did it, because the code was quite messy haha. It now has grand exchange support  I am thinking about safespots with cannon.Just don't know how to track cannon balls.

[geoffrey456]

Can u add a jar will try it out