OSBOTTESTER101 Posted June 20, 2020 Share Posted June 20, 2020 (edited) public class Attack { private static Script sI; NPC npc = sI.getNpcs().closest("Rat"); public void getAllNpcs(Script sI, String npcsName){ if(!sI.myPlayer().isUnderAttack()){ if(npc.exists()){ sI.npcs.closest(npcsName).interact("Attack"); }else{ sI.log("Cant find the current NPCS " + npcsName); } } } } Above is my code and this is only the " ATTACK class, that creates a getAllNpcs which when is suppose to attack the given String npcsName now before I had a collection of Npcs that is why you see the method ask for a given String basically it was like this private static List<NPC> npc = sI.npcs.getAll(); rather then the method you use above anyways am I doing something wrong ? because when I click play and try to use the bot script it does nothing and after I do I can't even click on the osrs game anymore it is basically like if there were a JFrame over my game not letting me click anything on that screen after I try to run the unsucessful script Anyways I was thinking that maybe I should use a constructor to pass the script instance? Such as public class Attack { private Script sI; public Attack(Script sI){ this.sI = sI; } //private static List<NPC> npc = sI.npcs.getAll(); NPC npc = sI.getNpcs().closest("Rat"); public void getAllNpcs(Script sI, String npcsName){ if(!sI.myPlayer().isUnderAttack()){ if(npc.exists()){ sI.npcs.closest(npcsName).interact("Attack"); }else{ sI.log("Cant find the current NPCS " + npcsName); } } } } and below I will give you my main method public int onLoop () throws InterruptedException{ switch (getState()){ case EAT: print(getState()); Eat.eatFood(); break; case ATTACK: print(getState()); attack.getAllNpcs(this, "Rat"); break; case BANK: print(getState()); bank.bankMode(this, Banks.VARROCK_WEST); break; } return random(3000); } private States getState(){ if(myPlayer().getHealthPercent()<20){ return States.EAT; }else if(!myPlayer().isUnderAttack()){ return States.ATTACK; }else return States.BANK; // do not mind the return States.BANK as I have not finished the code but all I was trying to do is make the player attack is all and my player is not under attack so it should return States.attack Oh yes by the way I have never created States or Classes that have separate task, this is my first time and that is why maybe it is not working for me but if the code looks fine to everyone than maybe I am not setting the build path correctly and I am using IntelliJ Edited June 20, 2020 by OSBOTTESTER101 Quote Link to comment Share on other sites More sharing options...
Token Posted June 20, 2020 Share Posted June 20, 2020 Don't call API methods before the onStart() executes, so nothing that assigns the result to class fields or in constructors/initializer blocks Quote Link to comment Share on other sites More sharing options...
OSBOTTESTER101 Posted August 9, 2020 Author Share Posted August 9, 2020 On 6/20/2020 at 1:25 PM, Token said: Don't call API methods before the onStart() executes, so nothing that assigns the result to class fields or in constructors/initializer blocks What do you mean exactly because I did not call anything on the onStart method I just used onLoop method Quote Link to comment Share on other sites More sharing options...
Camaro Posted August 9, 2020 Share Posted August 9, 2020 You're generating a null pointer when you run the script. NPC npc = sI.getNpcs().closest("Rat"); This will initialize before the constructor code runs, so the script object never actually gets set. This will cause your script to crash. if(npc.exists()){ sI.npcs.closest(npcsName).interact("Attack"); There are also multiple issues in these two lines. Since the 'npc' variable is only set at the beginning of the script, it never gets updated. exists() will always return false once that npc despawns. You are also not null checking anything. npcs.closest(npcsName) will return null when no npcs are found, which will crash your script. Since this is such a small amount of code, you don't really need to create a separate class for it yet. Start with just calling things within onloop until you understand how a script works more. case ATTACK: if (!myPlayer().isUnderAttack()) { NPC rat = getNpcs().closest("Rat"); if (rat != null && rat.interack("Attack")) { Sleep.sleepUntil(() -> myPlayer().isUnderAttack(), 5000); } } break; This should be enough for your attack state. Notice how I use a conditional sleep after attacking the rat. If you don't, then the bot will spam-click the rat until you are actually under attack. This is an extremely necessary thing to learn for writing efficient scripts. Quote Link to comment Share on other sites More sharing options...