Why is this unreachable code?

[liverare]

IF some condition THEN RETURN something
ELSE IF some condition THEN RETURN something
ELSE RETURN something

RETURN something

Look at this pseudo code.

RETURN statements are the exit points to your code.

That last ELSE statement is a catch-all for when the aforementioned conditions weren't met.

Your program can't get to that very lats RETURN statement, because there's a RETURN in your last ELSE statement.

 

:edit:

Programming using states can be really useful if you know what you're doing. A few tips are:

• If you're going to have one routine (aka. "functions") that finds the state and another that acts upon the state, please for the love of god let those functions share what they already know! If both of those routine are having to find your fire, then you're doing it wrong. This is where field variables come in handy. Look it up.
• If you've got less than 5 states in total, this might not be the best approach; it's an unnecessary pile of extra lines of code for no actual beneficial outcome.
• Even though you're technically programming in Java, the end product is a script. So unless you're doing a complex project, just write a script like you would a script.

 

Here's a quick ad hoc way of how I'd go about writing a script like the one you're doing:

	
	// ...
	
	RS2Object fire;
	
	@Override
	public int onLoop() {
	
		if ( isLightingAFire() ) {
			
			waitForFireToBeLit();
			
		} else if ( isCooking() ) {
			
			sleepUntilWereOutOfFood();
			
		} else if ( shouldBank() ) {
			
			goBank();
			
		} else if ( isFireAvailable() ) {
		
			cookOnFire();
		}
		
		return 100;
	}
	
	/*
	 * Other methods
	 */
	
	private boolean isLightingAFire() {
		return myPlayer().getAnimation() == 0; // TODO - change to fire animation
	}
	
	private void waitForFireToBeLit() {
		// TODO
	}
	
	
	
	private boolean isCooking() {
		return myPlayer().getAnimation() == 0; // TODO - change to cooking animation
	}
	
	private void sleepUntilWereOutOfFood() {
		// TODO
		// use conditional sleep
	}
	
	
	
	private boolean shouldBank() {
		return !inventory.contains("Raw lobsters")
			|| !inventory.contains("Logs")
			|| !inventory.contains("Tinderbox");
	}
	
	private void goBank() {
		// TODO
	}
	
	
	
	private void cookOnFire() {
		// TODO
		// Note: you can reference 'fire' as it's a field variable (aka. accessible to this function)
		//       also we can assert that fire exists, becuase this function doesn't run unless it does
	}
	
	private boolean isFireAvailable() {
		
		if (fire == null || !fire.exists()) {
		
			fire = objects.closest("Fire");
			
		}
		
		return fire != null && fire.exists();
	}
	
	// ...

I've padded it out with line-breaks to try make it visually clearer. But pay close attention to the onLoop function; that's effectively our header block for the script, and you can sit on the other side of the room and still be able to read exactly what the bot's doing. The onLoop function is so clearly laid out that nobody, no matter how new they are to scripting, should have an issue trying to understand that the bot's trying to do. It's direct, readable, and can be easily maintained.

Edited August 31, 2017 by liverare

[dreameo]

1 hour ago, kingbutton said:

Um, of course I read the api. 

ofcourse you read the api but cant read about control flow -.-

[Sysm]

i lol'd ty

[roguehippo]

3 hours ago, FrostBug said:

Stop being silly; all of you.

You should under no circumstance use exists() in this scenario.. It's an expensive method, and the fire will always exist if it isn't null since he just retrieved the instance.

i am aware he shouldnt use exists, but if someone really wanted to, they would have to check if its != null . it is pretty silly though

[liverare]

Before we circlejerk ourselves into never using exist, here's why you need it: things disappear. A tree becomes a stump when cut down; a guard becomes a pile of bones when slayed; a fire burns itself to ashes.

 

I don't think there's ever a case for calling getClosest more than once for the same entity. You should be caching the entities you've found, because they're known. Then you should validate that entity before scrapping it to find it again. If the entity's null, or the entity no longer exists, or the entity no longer tests true against the initial filters, then you should re-find that entity.