While standing here, if both fishing spots are up getClosest() will return this one: 

While standing in the other tile, it will return the other fishing spot, causing the loop to click one spot, then click the other spot over and over. I don't want to use an isAnimating() check because the fishing animation carries on for a few ticks after the fishing spot disappears.

Is there a way to fix this issue? My fishing method: 

NPC currentSpot;
Position currentLocation;

public void fish() {
		NPC fishingSpot = getNpcs().closest(6825);
		if(fishingSpot == null) {
			log("spot == null");
			return;
		}
		if(currentSpot == null) {
			currentSpot = fishingSpot;
			fishingSpot.interact("Bait");
		}
		if(!currentSpot.equals(fishingSpot)) {
			currentSpot = fishingSpot;
			fishingSpot.interact("Bait");
		}
		getMouse().moveOutsideScreen();
	}

 

Nothing wrong with waiting until you're done animating. 

No human instantly clicks after the spot disappears 

2 minutes ago, Juggles said:

Nothing wrong with waiting until you're done animating. 

No human instantly clicks after the spot disappears 

There is a delay in the onLoop(), but I figured that's pretty irrelevant for the core issue, also the RuneLoader client has a beep after you stopped fishing from the spot moving or your inv being full, so I'm trying to replicate the response time from that.

What's really wrong with isAnimating(), or to be exact isMoving()? A good check to do in order not to click around too much,

Would do it more or less like this and not worry about stuff (untested).

Finds closest spot and interacts with it. Would be good to add additional check to be sure that the fishing spot is good, like if it has the required action, is reachable and etc

public void fish() {
		NPC fishingSpot = getNpcs().closest("Fishing spot"); // Or whatever it is called. Don't use ids
		if (fishingSpot != null) {
         	fishingSpot.interact("Bait");
          new ConditionalSleep(5000, 250) {
			@Override
			public boolean condition() throws InterruptedException {
				return !myPlayer().isMoving() && myPlayer().isAnimating();
			}
		};
          getMouse().moveOutsideScreen();
        }
}

 

The fishing spot is closer because it can be 'reached' diagonally vs having to walk over and then reaching it. That's why those results are occurring. (Believe so*)

Just now, dreameo said:

The fishing spot is closer because it can be 'reached' diagonally vs having to walk over and then reaching it. That's why those results are occurring. (Believe so*)

Who knows really? Though if it's just simple fishing, the problem is in implementation

Like nosepicker said, focus on cleaning up the fish method. At the moment you are not making any use of the null check.

I don't really understand what you mean by "other tile" and all that, but usually these issues occur because distance checks are done with ints. So if its at an angle, the distance is 1.41~ which as an int is rounded to 1. So a tile south 1 tile and east 1 tile is the "same" distance as one just south of you, due to the rounding. Then its just a matter of which NPC is first/last in the internal entity list.

Also, you should sleep after interacting until the player is animating/interacting, with a max time (see ConditionalSleep or the other implementations around the forums). Also you can check if the spot exists still using currentSpot.exists(), once the fishing spot is gone exists() will return false.