Andrew Brook
Andrew Brook

Reputation: 29

How Can I Fix This Infinite While Loop?

This is sort of a continuation from a previous question I asked, but I need help with part of this small adventure game that I have been making as a final project for my AP Java Class.
In my adventure, I have a function that allows the player to battle various monsters if they are in a given location. My problem is that once I get to this location in my program, the program stops responding.
After doing some debug, I figured out that it was a while loop I had that does everything in the doBattle() method while player.isAlive and enemy.isAlive. The only problem is, once I remove the while loop, the program just jumps right to after either the monster is dead or the player is dead, whereas it's supposed to give you the choice on whether to attack the monster or not repeatedly until either entities are dead.

Here is the code of the doBattle() method:

public void doBattle(Monster enemy)
    {
        //boolean fled = false;
        //Greets player into battle by telling what monster they are fighting
        text.appendText("\n" + "A wild " + enemy.getName() + " has appeared!" + "\n" );
        mobImagePane.setImage(enemy.getImage());

        //System.out.print("THIS RAN"); //debug


        while ( p.getHealth() > 0 && enemy.getHealth() > 0 ) //while enemy and player are alive
        {
            //Prompts user to attack or run
            text.appendText("Attack " + enemy.getName() + "? (Y/N) " + "\n");
            inputText.setOnAction(event ->
            {
                String fightChoice = inputText.getText();
                fightChoice = fightChoice.toUpperCase();
                //String fightChoice = this.choice;


                if (fightChoice.equals("Y"))//if they want to fight
                {
                    //Player turn
                    enemy.setHealth(enemy.getHealth() - p.getDamage()); //Sets the monsters health as their current health minus the players damage
                    text.appendText("You attack " + enemy.getName() + " for " + p.getDamage() + " damage!" + "\n" + "Enemy health is " + enemy.getHealth() + "\n");

                    //Monster turn
                    p.setHealth(p.getHealth() - enemy.getDamage()); //Sets the players health as their current health minus the monsters damage
                    text.appendText("The " + enemy.getName() + " hit you for " + enemy.getDamage() + " damage!" + "\n" + "Your health is " + p.getHealth() + "\n"); //prints how much damage the monster does to the player
                    if (p.health < 20.0) {
                        text.appendText("Your health is low, you should return home and restore health!" + "\n");
                    }

                    //checks if the player or monster is dead
                    this.checkLife();
                    enemy.checkLife();

                } else {
                    if (fightChoice.equals("N")) // if they don't want to fight
                    {
                        mobImagePane.setImage(null);
                        text.appendText("You fled from the fight!" + "\n");
                        this.setNewLoc("TOWN"); // brings you back to town
                        fled = true;
                        //JOptionPane.showMessageDialog(null, "You are now in " + this.currentLoc.getName() + "\n" + this.currentLoc.getDescription());
                        //String move2 = JOptionPane.showInputDialog(null,"Which way do you want to go? (N, E, S, W)");
                        //makeMove(move2);
                        //break;
                    } else // they don't make any sense
                    {
                        text.appendText("Unrecognized command" + "\n");
                    }
                }

                //}

                //when someone dies
                if (!fled) {
                    if (p.alive) // if you are still standing
                    {
                        //print results (money earned, health remaining)
                        mobImagePane.setImage(null);
                        p.wallet += enemy.getLoot();
                        playerInfo.setText(p.getPlayerName() + "\n" + "Health: " + p.getHealth() + "\n" + "Wallet: " + p.getWallet() + "\n");
                        text.setText("You shrekt the " + enemy.getName() + "\n" + "You got $" + enemy.getLoot() + " for winning!" + "\n" + "You now have $" + p.wallet + "\nYour health is " + p.getHealth() + "\n");
                    } else //if you died
                    {
                        mobImagePane.setImage(null);
                        text.setText("You have been shrekt by the " + enemy.getName() + "\n" + "GAME OVER" + "\n");
                        text.appendText("\nPlay again? (Y/N)" + "\n");
                        inputText.setOnAction(event2 -> {
                            String answer = inputText.getText();
                            answer.toUpperCase();
                            //String answer = this.choice;

                            if (answer.equals("Y")) //if they want to play again
                            {
                                text.appendText("Alright! Let's go!" + "\n");
                                this.reset();
                            } else //if they want to quit
                            {
                                text.appendText("Wow. What a skrub, okay bye." + "\n");
                                System.out.close();
                            }
                        });

                    }
                }
            });
        }
    } //end doBattle

Keep in mind I am new to this site, and somewhat new to java, so please let me know if you need more information or anything else to help me get better suggestions, all help is appreciated.
Also, please do not downvote without at least telling me why, I want to get better at this sort of thing.
I should also mention that I am using JavaFX for this, and text is a TextArea and inputText is a TextField

Upvotes: 1

Views: 197

Answers (1)

Joeri Hendrickx
Joeri Hendrickx

Reputation: 17445

You seem to be misunderstanding how your code is executed. You put your input handling code (inputText.setOnAction) inside your loop, and you seem to think that means it will be executed at that time. But that is not correct; the handler is asynchronous and will be executed when an event occurs. What actually happens in your loop is that the handler is set, and set, and set, and set, and set, until eternity. (not sure about thread-safety of Swing, but I would guess the handler is not even ever set because you don't give the event-thread the chance to synchronize)

What you really want is for your code to actually continue when an input event occurs. That means that all of the continuation logic should be in the handler, and not around it.

So I see three ways for you to continue:

  • move to a simpeler way of working and use console instead (fewest code changes). This would work because console reads are blocking (you wouldn't use an event handler but a simple read instead)
  • move this code into a separate thread, block the current thread (wait) after the handler is set, and notify it when an event occurs
  • completely split the logic up so that any continuation is driven by events. That would mean moving more into a state machine way of working.

The downside of the second approach is that it involves threads and synchronization. It's a pain to get right. But your game logic will still 'read' like a whole. If you nicely encapsulate the thread handler into a UserInput class you can probably make it manageable (but I don't think you're at that level yet)

The downside of the third approach is that your logic ends up being spread all over the place.

Taking your beginner level into approach, the most pragamtic advise I could give you is to take approach #1, and go console-based instead.

For approach #2

Some details on what the second approach would look like -- mind that this is pseudo-code. I'm also not claiming that this would be an ideal design nor codebase. But it's a quickfix for the code that is passable.

// this should not run on the event handling thread of javafx

Holder<String> inputReceived = new Holder<String>(); //Holder is an often used hack to set values from a closure.  It's not builtin, but you'll find hundreds of examples.

inputText.setHandler(e->{
   inputReceived.set(inputText.getText());
});

while (player.isAlive() && monster.isAlive()){
   // wait for input
   while (!isValidInput(inputReceived.get())){
      Thread.sleep(200);
   }
   ... do your code based on the value in inputReceived
}

Upvotes: 2

Related Questions