2

I'm trying to create a simple game but I'm already failing with the Background. My code works only for a couple of seconds before I'm getting this error:

Exception in thread "JavaFX Application Thread" java.lang.NullPointerException 
    at javafx.scene.Parent.updateCachedBounds(Parent.java:1592) 
    at javafx.scene.Parent.recomputeBounds(Parent.java:1535) 
    at javafx.scene.Parent.impl_computeGeomBounds(Parent.java:1388) 
    at javafx.scene.layout.Region.impl_computeGeomBounds(Region.java:3078)
    at javafx.scene.Node.updateGeomBounds(Node.java:3579) 
    at javafx.scene.Node.getGeomBounds(Node.java:3532) 
    at javafx.scene.Node.getLocalBounds(Node.java:3480) 
    at javafx.scene.Node.updateTxBounds(Node.java:3643) 
    at javafx.scene.Node.getTransformedBounds(Node.java:3426) 
    at javafx.scene.Node.updateBounds(Node.java:559) 
    at javafx.scene.Parent.updateBounds(Parent.java:1719) 
    at javafx.scene.Parent.updateBounds(Parent.java:1717) 
    at javafx.scene.Scene$ScenePulseListener.pulse(Scene.java:2404) 
    at com.sun.javafx.tk.Toolkit.lambda$runPulse$29(Toolkit.java:398) 
    at java.security.AccessController.doPrivileged(Native Method) 
    at com.sun.javafx.tk.Toolkit.runPulse(Toolkit.java:397) 
    at com.sun.javafx.tk.Toolkit.firePulse(Toolkit.java:424) 
    at com.sun.javafx.tk.quantum.QuantumToolkit.pulse(QuantumToolkit.java:510)
    at com.sun.javafx.tk.quantum.QuantumToolkit.pulse(QuantumToolkit.java:490)
    at com.sun.javafx.tk.quantum.QuantumToolkit.lambda$runToolkit$403(QuantumToolkit.java:319)
    at com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)
    at com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method) 
    at com.sun.glass.ui.gtk.GtkApplication.lambda$null$48(GtkApplication.java:139)
    at java.lang.Thread.run(Thread.java:748)

I tried finding it out on my own, but I have no idea.

Here's my code:

public class RocketMain extends Application{

    int width = 640;
    int height = 1000;
    Pane root;
    Stars stars;

    public static void main(String[] args){
        Application.launch();
    }

    @Override
    public void start(Stage primaryStage) throws Exception {
        root = new Pane();
        Scene sc = new Scene(root, width, height);

        primaryStage.setScene(sc);
        primaryStage.setResizable(false);
        primaryStage.centerOnScreen();

        stars = new Stars(width, height);

        new Thread(() -> stars.createStars()).start();
        new Thread(() -> update()).start();

        root.getChildren().add(stars);

        primaryStage.show();
        primaryStage.setOnCloseRequest(e -> {
            Platform.exit();
            System.exit(0);
        });
    }

    public void update(){
        while(true){
            Platform.runLater(() -> {
                stars.getChildren().removeAll(stars.getChildren());
                stars.getChildren().addAll(stars.stars);

                root.getChildren().removeAll(root.getChildren());
                root.getChildren().add(stars);
            });

            try {
                Thread.sleep(100);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
}

The Stars Class which keeps spawning new sprites and moves/deletes them when offscreen:

public class Stars extends Pane {

    public ArrayList<Rectangle> stars;
    ArrayList<Rectangle> rem;
    double width, height;

    public Stars(int width, int height){
        stars = new ArrayList<>();
        rem = new ArrayList<>();

        this.width = width;
        this.height = height;
    }

    public void createStars(){
        int slowing = 3;

        while(true){

            if (Math.random() < (1.0/slowing)){
                int size = (int) Math.floor(Math.random()*8);
                Rectangle temp = new Rectangle(Math.random() * width, -10, size, size);
                temp.setFill(Color.BLACK);
                this.stars.add(temp);
                Platform.runLater(() -> {
                    this.getChildren().remove(temp);
                    this.getChildren().add(temp);
                });
            }

            for (Rectangle r : stars) {
                r.setY(r.getY() + r.getHeight() / slowing);
                if (r.getY() > height / 2) {
                    rem.add(r);
                }
            }

            stars.removeAll(rem);
            rem.clear();

            try {
                Thread.sleep(20);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }    
    }    
}

tia I hope this was understandable

c0der
  • 18,467
  • 6
  • 33
  • 65
  • 5
    I haven't analyzed your code in depth, but you appear to have threading issues. Make sure you are properly synchronizing access to references shared between threads and that you only update the UI on the FX thread. – Slaw Jan 18 '20 at 00:59

1 Answers1

3

You seem to have moved almost all code that updates from background threads nodes that are part of a scene into Platform.runLater calls. The stacktrace indicates this is the case though.

You've missed one place where updates happen though:

r.setY(r.getY() + r.getHeight() / slowing);

in the enhanced for loop inside createStars.

However there's another issue that could cause similar issues, but the stacktrace indicates that in this run this was not the case:

You update the lists from the background thread, but read them from the javafx application thead. There is no synchronisation done for this though which could result in one of the threads accessing a list that is in an inconsistent state.

However your logic is more complicated that it needs to be. The code you run in every loop iteration doesn't take long to execute, if we ignore Thread.sleep. JavaFX provides you with a class that allows you to schedule regularly running logic on the JavaFX application thread which I recommend using (Timeline).

public class Stars extends Pane {

    double width, height;
    private final Timeline timeline;

    public void startAnimation() {
        timeline.play();
    }

    public Stars(int width, int height){
        timeline = new Timeline(new KeyFrame(Duration.millis(20), new EventHandler<ActionEvent>() {

            private int iteration = 0;
            private final Random random = new Random();
            private final List<Rectangle> stars = new LinkedList<>();
            private final Set<Rectangle> toRemove = new HashSet<>();

            @Override
            public void handle(ActionEvent event) {
                // iteration is 0 on every 5th iteration to simulate both background threads at once
                iteration = (iteration + 1) % 5;

                final int slowing = 3;

                if (random.nextInt(slowing) == 0) { // also true in 1 of 3 cases
                    int size = random.nextInt(8);
                    Rectangle temp = new Rectangle(random.nextDouble() * Stars.this.width, -10, size, size);
                    temp.setFill(Color.BLACK);

                    this.stars.add(temp);
                    getChildren().add(temp);
                }

                Iterator<Rectangle> iter = stars.iterator();
                while (iter.hasNext()) {
                    Rectangle r = iter.next();
                    if (r.getY() > Stars.this.height / 2) {
                        // move rect list to set
                        toRemove.add(r);
                        iter.remove();
                    }
                }

                if (iteration == 0) {
                    // logic from background thread with lower frequency
                    getChildren().removeAll(toRemove);
                    toRemove.clear();
                }
            }
        }));
        timeline.setCycleCount(Animation.INDEFINITE);
        this.width = width;
        this.height = height;
    }

}

This way all you need to do is call startAnimation, no threads needed, no issues with concurrency.

Further notes:

list.removeAll(list);

should be changed to

list.clear();

Since you follow this up by

list.addAll(list2);

you can also replace both lines with

list.setAll(list2);

(ObservableList provides this functionality)

fabian
  • 80,457
  • 12
  • 86
  • 114
  • 1
    I fully agree with fabian although I would use an AnimationTimer for this task. The principle idea remains the same though. – mipa Jan 18 '20 at 09:05
  • Wow! Thanks for this detailed answer. Even though I made many things with Java now, there’s always a better way to do things that I don’t know of i will try this –  Jan 18 '20 at 09:28
  • One more thing: when I’m adding more stuff, like meteors as obstacles, should I go for using the same logic? Another animation with that timeline? Let’s say I have meteors and the player/rocket. When I have 3 of these timelines, can I check collision between them? –  Jan 18 '20 at 10:27
  • hey! thanks for introducing me to timelines, ive been using them a lot since then, keep up what ur doing over here! –  Apr 06 '20 at 20:43