Not sure why my script isn't running

[kingbutton]

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

@ScriptManifest(author = "Kingbutton", info = "Farms Anchovies", logo = "", name = "Crustaceans", version = 0)
public class Main extends Script {

	Player player = myPlayer();

	Area draynorBank = new Area(3092, 3240, 3097, 3246);
	Area draynorFish = new Area(3087, 3227, 3089, 3233);

	final String fnet = "Small fishing net";
	final String anch = "Raw anchovies";
	final String shrimp = "Raw shrimps";
	final NPC spot = npcs.closest("Fishing spot");
	final NPC banker = npcs.closest("Banker");

	public void onStart() {

	}

	public void onExit() {

	}

	@Override
	public int onLoop() throws InterruptedException {

		if (getFull()) {
			goBank();
		} else {
			goFish();
		}

		return 50;
	}

	public boolean getFull() {
		return inventory.onlyContains(fnet, anch);
	}

	public void goBank() {
		if (!draynorBank.contains(player)) {
			getWalking().walk(draynorBank);
		}
	}

	public void goFish() {
		if (!draynorFish.contains(player)) {
			getWalking().walk(draynorFish);
		} else if (spot != null && spot.isVisible() && !player.isAnimating() && !player.isMoving()
				&& spot.interact("Fish")) {
			new ConditionalSleep(5000) {

				@Override
				public boolean condition() throws InterruptedException {

					return myPlayer().isAnimating();
				}
			}.sleep();

		}

	}

}

Trying to practice using methods and using less if statements and more && statements.

Middle of the Code i'm trying to test to see if it'll do anything, see if I'm doing this stuff right.

It's not running, and since I'm new to methods, I'm not sure how to even see what's wrong.

[slazter]

This, already answered, and the reason why aswell  
with other words, 

	final NPC spot = npcs.closest("Fishing spot");
	final NPC banker = npcs.closest("Banker");

can't be placed like that.

Edited August 2, 2017 by slazter

[HeyImJamie]

Is there a reason everyone keeps placing their Strings as a global variable?  

[Apaec]

10 minutes ago, HeyImJamie said:

Is there a reason everyone keeps placing their Strings as a global variable?  

What's the problem with this? 

[dreameo]

how he used string is good, if you put it in a method, you would be creating a string object everytime. Note, anything final should be left in uppercase (final string NAME = "")

 

[Team Cape]

40 minutes ago, kingbutton said:

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

@ScriptManifest(author = "Kingbutton", info = "Farms Anchovies", logo = "", name = "Crustaceans", version = 0)
public class Main extends Script {

	Player player = myPlayer();

	Area draynorBank = new Area(3092, 3240, 3097, 3246);
	Area draynorFish = new Area(3087, 3227, 3089, 3233);

	final String fnet = "Small fishing net";
	final String anch = "Raw anchovies";
	final String shrimp = "Raw shrimps";
	final NPC spot = npcs.closest("Fishing spot");
	final NPC banker = npcs.closest("Banker");

	public void onStart() {

	}

	public void onExit() {

	}

	@Override
	public int onLoop() throws InterruptedException {

		if (getFull()) {
			goBank();
		} else {
			goFish();
		}

		return 50;
	}

	public boolean getFull() {
		return inventory.onlyContains(fnet, anch);
	}

	public void goBank() {
		if (!draynorBank.contains(player)) {
			getWalking().walk(draynorBank);
		}
	}

	public void goFish() {
		if (!draynorFish.contains(player)) {
			getWalking().walk(draynorFish);
		} else if (spot != null && spot.isVisible() && !player.isAnimating() && !player.isMoving()
				&& spot.interact("Fish")) {
			new ConditionalSleep(5000) {

				@Override
				public boolean condition() throws InterruptedException {

					return myPlayer().isAnimating();
				}
			}.sleep();

		}

	}

}

Trying to practice using methods and using less if statements and more && statements.

Middle of the Code i'm trying to test to see if it'll do anything, see if I'm doing this stuff right.

It's not running, and since I'm new to methods, I'm not sure how to even see what's wrong.

no calls to methods inherited from Script are allowed before onLoop().

This means you can't call myPlayer() or npcs.closest()

[Final]

26 minutes ago, HeyImJamie said:

Is there a reason everyone keeps placing their Strings as a global variable?  

Programming 101 for newbies. Thought you'd be well versed ;)

[kingbutton]

46 minutes ago, Imateamcape said:

no calls to methods inherited from Script are allowed before onLoop().

This means you can't call myPlayer() or npcs.closest()

Where am I supposed to declare my things then? Like EVERYTHING.

Strings,Areas, the player and such. am I supposed to do it in the loop or something?

[dreameo]

5 minutes ago, kingbutton said:

Where am I supposed to declare my things then? Like EVERYTHING.

Strings,Areas, the player and such. am I supposed to do it in the loop or something?

 

Somewhat, you can create your own methods and then call them in the onLoop. When interacting with NPCs or Entitys, you need the references to those objects updated (i.e within the loop). Constants can be global as you have them (Strings, areas), the rest you should do within methods ;)

[Team Cape]

39 minutes ago, kingbutton said:

Where am I supposed to declare my things then? Like EVERYTHING.

Strings,Areas, the player and such. am I supposed to do it in the loop or something?

Firstly, you're not going to be using only 1 fishing spot, so there's no reason to only call it once. Secondly, if there's a variable that you need Script to obtain, define it at the top, and assign it in onStart()

Edited August 2, 2017 by Imateamcape

[kingbutton]

3 hours ago, Imateamcape said:

Firstly, you're not going to be using only 1 fishing spot, so there's no reason to only call it once. Secondly, if there's a variable that you need Script to obtain, define it at the top, and assign it in onStart()

So have fishing spot in the onLoop(), and have everything else in onStart()?

[Apaec]

14 hours ago, dreameo said:

how he used string is good, if you put it in a method, you would be creating a string object everytime. Note, anything final should be left in uppercase (final string NAME = "")

 

I wouldn't say everything final has to be left in upper case, that's preference really. Perhaps for user-defined constants which modify how your code runs directly and will only ever be the value you provide, but for final attributes initialised in a classes constructor, while the values may be constant locally, globally across other instances this is not guaranteed!

[Apaec]

13 hours ago, kingbutton said:

Where am I supposed to declare my things then? Like EVERYTHING.

Strings,Areas, the player and such. am I supposed to do it in the loop or something?

You don't have to declare stuff before using it, you can declare stuff as you use it. I'd suggest reading up a bit about java, but the idea is:

String tree = "Oak tree";
RS2Object tree = getObjects().closest(tree);
//... is functionally equivalent to
RS2Object tree = getObjects().closest("Oak tree");

//... likewise

Area area = new Area(0,0,0,0);
getWalking().walk(area);
//... is functionally equivalent to
getWalking().walk(new Area(0,0,0,0));

 

[dreameo]

4 hours ago, Apaec said:

I wouldn't say everything final has to be left in upper case, that's preference really. Perhaps for user-defined constants which modify how your code runs directly and will only ever be the value you provide, but for final attributes initialised in a classes constructor, while the values may be constant locally, globally across other instances this is not guaranteed!

 

This comes from Oracles code convention.

Quote

The names of variables declared class constants and of ANSI constants should be all uppercase with words separated by underscores ("_"). (ANSI constants should be avoided, for ease of debugging.)

 

Not sure what you're saying at the end lol.

[Alek]

There are compiler optimizations where I'm sure strings aren't created every time. Likewise, it's Java and nobody here cares about performance - otherwise people would stop using paints and task/node frameworks. 

To answer your question:

final NPC spot = npcs.closest("Fishing spot");

Fishing spots change. Once this fishing spot is gone, "spot" will not exist anymore. Search for the fishing spot NPC every time and don't use final.