0

I'm trying to move a rectangle after setting the scene, however it doesnt seem to work.

I start with a scene to load a file that is used to create the next scene on button click.

@Override
public void start(Stage primaryStage) {
    Scene scene = CreateStartScene();
    primaryStage.setScene(scene);

    // Load Click Event
    loadButton.setOnAction(new EventHandler<ActionEvent>() {

        @Override
        public void handle(ActionEvent e) {
            FileChooser fileChooser = new FileChooser();
            fileChooser.setTitle("Select File");
            fileChooser.getExtensionFilters().addAll(new ExtensionFilter("Text Files", "*.txt"));

            File file = fileChooser.showOpenDialog(null);

            if (file != null) {
                fileTextField.setText(file.getAbsolutePath());
                startButton.setVisible(true);
            }
        }

    });

    // Start Click Event
    startButton.setOnAction(new EventHandler<ActionEvent>() {

        @Override
        public void handle(ActionEvent arg0) {
            if (!fileTextField.getText().isEmpty()) {
                // Read in maze file
                File file = new File(fileTextField.getText());
                ReadFile(file);

                Scene scene = CreateMainScene();
                primaryStage.setScene(scene);

                Solve();
            } else {
                errorLabel.setText("Please select a file");
            }
        }

    });

    primaryStage.show();
}

In the CreateMainScene method, shapes are added to the scene, one of which is this rectangle that is an instance variable

private Rectangle current;

which is added like this:

current = new Rectangle();
current.setFill(Color.CORNFLOWERBLUE);

current.setWidth(10);
current.setHeight(10);

current.setX(j * 10);
current.setY(i * 10);
pane.getChildren().add(current);

that all works fine, however in the solve method (which is recursive) I'm trying to change the position of that rectangle, and the pause for half a second.

    current.setX(x * 10);
    current.setY(y * 10);

    try {
        TimeUnit.MILLISECONDS.sleep(500);
    } catch (InterruptedException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }

This does not work. The window freezes for a few seconds, before finally updating. I want to show the rectangle moving but I cant figure out how to do it.

Thomas Colbert
  • 152
  • 1
  • 15
  • If you are going to be moving GUI components, try using one of `Java's` libraries that are designed to handle the situation in `JavaFX`. `Java's` [`AnimationTimer`](https://docs.oracle.com/javase/8/javafx/api/javafx/animation/AnimationTimer.html) is probably the best bet. – SedJ601 Apr 12 '18 at 18:48
  • 1
    See https://stackoverflow.com/questions/9966136/javafx-periodic-background-task – James_D Apr 12 '18 at 19:36

1 Answers1

2

You are executing everything on the UI thread. While waiting on the UI thread you block it from doing anything else. There are cleaner solutions, but this is meant to demonstrate the underlying problem:

// create and start new Thread, so we don't block the UI
new Thread(() -> {
    try {
        // wait some time on the new Thread
        TimeUnit.MILLISECONDS.sleep(500);
    } catch (InterruptedException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }
    // give this to the UI system to be executed on the UI
    // thread as soon as there are resources 
    Platform.runLater(() -> {
        // do UI stuff
    });
}).start();
Neuron
  • 5,141
  • 5
  • 38
  • 59
  • Extending `Thread` is generally discouraged, it would be better to do something like `new Thread(() => { ... })` or `new Thread(new Runnable() { ... })` instead. – apetranzilla Apr 12 '18 at 17:31
  • @apemanzilla Really? This is new to me. Could you point me to some resource? In that case I would update my answer – Neuron Apr 12 '18 at 17:33
  • [Here's a good answer specific to Thread](https://stackoverflow.com/a/541506/4072509) - generally, you should avoid extending something when you can instead use existing behavior (passing a `Runnable` to the constructor). – apetranzilla Apr 12 '18 at 17:35
  • @apemanzilla well, that question is not about inline threading. The argument is, that by extending `Runnable` you can also extend other interfaces, which we certainly don't want to do here. Also, the [oracle website itself extends `Thread`s](https://docs.oracle.com/javase/7/docs/api/java/lang/Thread.html) – Neuron Apr 12 '18 at 17:40
  • @apemanzilla I switched to implementing `Runnable`, but mainly due to the line of code saved :) – Neuron Apr 12 '18 at 17:44
  • It really comes down to [composition vs inheritance](https://en.wikipedia.org/wiki/Composition_over_inheritance). Unless you need to add extra behavior (overriden methods, new fields, etc) it's usually preferred to use existing structures than extending them to create your own. (This also has the benefit of being shorter with the use of lambdas) – apetranzilla Apr 12 '18 at 17:46
  • @apemanzilla yes, I am aware of that idea. But if you create the class inline, all arguments for composition vanish (all but the lambda one) – Neuron Apr 12 '18 at 17:48