Help with enum States

[Criboo]

Hi all

As my first script, I've made a basic combat script which fights a specific monster, eats when health is low, and banks.  It seems to work fine as I have ran it for a few hours without issue!

Everything is in the onLoop at the moment and looks a bit messy, so I'm attempting to tidy up the code.  My first thought was to use the following as a skeleton:-

private enum State {
    FIGHT, GO_TO_BANK, BANKING, RETURN_FROM_BANK
}

public State getState() {
    String food = "Trout";
    if (getBank().isOpen() && !getInventory().contains(food));
        return State.BANKING;
    if (myPlayer().getPosition().equals(BANK) && getInventory().contains(food));
        return State.RETURN_FROM_BANK;
    if (!getInventory().contains(food));
        return State.GO_TO_BANK;
    return State.FIGHT;
}

@Override
public int onLoop() throws InterruptedException {

    switch (getState()) {
        case BANKING:
    }

    switch (getState()) {
        case RETURN_FROM_BANK:
    }

    switch (getState()) {
        case GO_TO_BANK:
    }

    switch (getState()) {
        case FIGHT:
    }

However, the second 'if' statement in the public State getState() { has a red line under it in IntelliJ as it is an 'unreachable statement'.  What does this mean? I'm clearly not understanding how the code flows in some way.

Is there a better way to structure the code?

Should I get rid of the 'RETURN_FROM_BANK' state, or get rid of all three banking states and replace it with one state which goes to the bank, does banking, then returns to my monster?

Thanks in advance for any help!

Chris

Edited April 12, 2018 by Criboo

[Apaec]

There are a couple of syntactic issues with this code:

1. You're putting a semi-colon straight after the if statement. Since if statements without any curly braces {} only encompass the next line, this semi colon is blocking your actual code. See https://stackoverflow.com/questions/15786949/is-there-a-difference-in-removing-the-curly-braces-from-if-statements-in-java
2. Take a look at how switch statements work on google  https://docs.oracle.com/javase/tutorial/java/nutsandbolts/switch.html

With regards to your banking question, splitting it up into states in your getState() method sounds like a good plan.

-Apa

Edit: added some links

Edited April 12, 2018 by Apaec

[liverare]

That's quite possibly the best and most neatest onLoop I've ever seen.

[Chris]

quick fix: dont use enum states

[Criboo]

12 hours ago, Apaec said:

There are a couple of syntactic issues with this code:

1. You're putting a semi-colon straight after the if statement. Since if statements without any curly braces {} only encompass the next line, this semi colon is blocking your actual code. See https://stackoverflow.com/questions/15786949/is-there-a-difference-in-removing-the-curly-braces-from-if-statements-in-java
2. Take a look at how switch statements work on google  https://docs.oracle.com/javase/tutorial/java/nutsandbolts/switch.html

With regards to your banking question, splitting it up into states in your getState() method sounds like a good plan.

-Apa

Edit: added some links

Apa

Thanks for getting back for me and for posting those helpful links.  Not sure I completely follow the second one, but I have my States working, and have revamped (and fleshed out) my script using a lot more curly braces (and far fewer semicolons!).  I'm sure as I attempt to develop more complex scripts I'll figure it out as I tend to learn best through practical exercises!

 

10 hours ago, Chris said:

quick fix: dont use enum states

Chris

Thanks too for getting back, and for your tutorials, without which I'd not have gotten this far!

I had the whole thing in the onLoop before without using states.  It did what I asked of it, but it was messy, and I'm not convinced it followed through every step properly.  I wanted to put it into states to basically learn how to do it, for when I want to make more complicated scripts.

It seems like states are a more intuitive way of parceling up a script into lots of little tasks, but if that's not the case I'm happy to be corrected.

Anyway, thanks again to both of you

Chris

[Apaec]

20 minutes ago, Criboo said:

Apa

Thanks for getting back for me and for posting those helpful links.  Not sure I completely follow the second one, but I have my States working, and have revamped (and fleshed out) my script using a lot more curly braces (and far fewer semicolons!).  I'm sure as I attempt to develop more complex scripts I'll figure it out as I tend to learn best through practical exercises!

No worries - what I meant about switch statements was as follows: While it may work to lay the code out as you have done in your onLoop, it might lead to some unexpected results since it may execute more than one thing per loop of the onLoop. Here's the correct (...or traditional...) way to use a switch statement:

switch (getState()) {
    /* ... */
  case BANKING:
    break;
  case RETURN_FROM_BANK:
    break;
  case GO_TO_BANK:
    break;
    /* ... */
  default:
    break;
}

____________________________

As for using enums in this way to organise your code, I think it's a great way to separate the function of your code (i.e the interaction code, walking, etc) from the logic of your code (the checks, assertions etc). While plain if/else statements in your onLoop works just fine too, it can get quite overwhelming for larger projects.

It is important to note however that the difference between these solutions is purely aesthetic: neither solutions offer any additional functionality. As you progress with scripting and java in general, you will learn about inheritance, encapsulation, and other key object oriented concepts (alongside some common design patterns) which you can apply to your code to make it 'do more for less'.

Best

Apa

 

Edited April 13, 2018 by Apaec

[Criboo]

1 hour ago, Apaec said:

No worries - what I meant about switch statements was as follows: While it may work to lay the code out as you have done in your onLoop, it might lead to some unexpected results since it may execute more than one thing per loop of the onLoop. Here's the correct (...or traditional...) way to use a switch statement:

switch (getState()) {
    /* ... */
  case BANKING:
    break;
  case RETURN_FROM_BANK:
    break;
  case GO_TO_BANK:
    break;
    /* ... */
  default:
    break;
}

____________________________

As for using enums in this way to organise your code, I think it's a great way to separate the function of your code (i.e the interaction code, walking, etc) from the logic of your code (the checks, assertions etc). While plain if/else statements in your onLoop works just fine too, it can get quite overwhelming for larger projects.

It is important to note however that the difference between these solutions is purely aesthetic: neither solutions offer any additional functionality. As you progress with scripting and java in general, you will learn about inheritance, encapsulation, and other key object oriented concepts (alongside some common design patterns) which you can apply to your code to make it 'do more for less'.

Best

Apa

 

Apa

Thanks again for getting back & taking your time to help.

I suspect it was doing things repeatedly when all in the onLoop.  I also am aware that splitting it into States was purely aesthetic, but, as I said, I did it to learn + because my script was messy.  It looks much better now, and I'm finding it easier to debug/add additional functionality and checks now it's split into more manageable chunks. 

Cheers,

Chris

[Apaec]

55 minutes ago, Criboo said:

Apa

Thanks again for getting back & taking your time to help.

I suspect it was doing things repeatedly when all in the onLoop.  I also am aware that splitting it into States was purely aesthetic, but, as I said, I did it to learn + because my script was messy.  It looks much better now, and I'm finding it easier to debug/add additional functionality and checks now it's split into more manageable chunks. 

Cheers,

Chris

Glad it's working well for you. Just dropped the reply to clear things up.

If you have any more questions about anything, don't hesitate to drop me a PM. I'm always happy to help:)

-Apa