How to give each cloud an individual x position so that I could draw it based on that

Question

So basically I have a scenery project where I created a list of "clouds" based on the constructor in my Cloud class. Each time I created a list of those clouds I am generating a random number for x. When I draw the cloud it has that x value. In the animation I am adding the cloud's x-axis and I want to make it so that each time an individual cloud's x-axis is more than 800 they go to -150. I thought I did it right, but for some reason the clouds are moving really fast :(

import java.awt.Color;
import java.awt.Dimension;
import java.awt.Graphics;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ThreadLocalRandom;
import javax.swing.JButton;
import javax.swing.JPanel;

public class Scenery extends JPanel implements ActionListener {

    private RainSnowDrop[] rain, snow;

    private Cloud[] cloud;

    private double cloudX;

    private Background background;

    private Tree[] tree;

    private Mountain mountain;

    private JButton fallB, winterB, springB, summerB;

    private boolean fall, winter, spring, summer;

    private Color skyColor, grassColor, treeColor, treeStickColor, mountainColor;

    int[] getXs = new int[7];


    public Scenery() {

        setLayout(null);

        fallB = new JButton("Fall");
        fallB.setBounds(50, 475, 80, 40);
        fallB.addActionListener(this);
        add(fallB);

        winterB = new JButton("Winter");
        winterB.setBounds(250, 475, 80, 40);
        winterB.addActionListener(this);
        add(winterB);

        springB = new JButton("Spring");
        springB.setBounds(450, 475, 80, 40);
        springB.addActionListener(this);
        add(springB);

        summerB = new JButton("Summer");
        summerB.setBounds(650, 475, 80, 40);
        summerB.addActionListener(this);
        add(summerB);

        skyColor = (Color.WHITE);
        grassColor = (Color.WHITE);
        treeColor = (Color.WHITE);
        treeStickColor = (Color.WHITE);
        mountainColor = (Color.WHITE);

        snow = new RainSnowDrop[200];
        rain = new RainSnowDrop[200];
        tree = new Tree[5];
        cloud = new Cloud[7];
        background = new Background();
        mountain = new Mountain();

        for (int i = 0; i < rain.length; i++) {
            rain[i] = new RainSnowDrop();
        }
        for (int i = 0; i < snow.length; i++) {
            snow[i] = new RainSnowDrop();
        }
        for (int i = 0; i < tree.length; i++) {
            tree[i] = new Tree();
        }
        for (int i = 0; i < cloud.length; i++) {
            cloud[i] = new Cloud();
            getXs[i] = Cloud.xs.get(i);
        }

        setFocusable(true);
    }

    public Dimension getPreferredSize() {

        return new Dimension(800, 600);

    }

    public void paintComponent(Graphics g) {

        super.paintComponents(g);

        background.drawBackground(g, grassColor, skyColor);

        mountain.drawMountain(g, mountainColor, Color.WHITE, winter);

        for (int i = 0; i < tree.length; i++) {
            tree[i].drawTree(g, treeColor, treeStickColor, winter);
        }

        if (spring) {

            mountainColor = new Color(68, 73, 68);
            treeStickColor = new Color(179, 23, 23);
            treeColor = (Color.GREEN);
            grassColor = new Color(120, 225, 120);
            skyColor = new Color(198, 245, 242);

            for (int i = 0; i < rain.length; i++) {
                rain[i].drawRain(g);
            }
        }

        if (winter) {

            mountainColor = new Color(68, 73, 68);
            treeStickColor = new Color(179, 23, 23);
            treeColor = new Color(210, 210, 210);
            skyColor = (Color.LIGHT_GRAY);
            grassColor = new Color(190, 228, 200);

            for (int i = 0; i < cloud.length; i++) {
                cloud[i].drawCloud(g, getXs[i] + cloudX);
            }

            for (int i = 0; i < snow.length; i++) {
                snow[i].drawSnow(g);
            }
        }

        //Summer

        //Fall
    }

    public void animate() {

        while (true) {

            for (int i = 0; i < cloud.length; i++) {
                System.out.println(getXs[i]);
            }

            try {
                Thread.sleep(5); //in milliseconds
            }
            catch(InterruptedException ex) {
                Thread.currentThread().interrupt();
            }

            cloudX += 0.2;

            for (int i = 0; i < cloud.length; i++) {
                getXs[i] += (int) cloudX;
                if (getXs[i] > 800) getXs[i] = -150;
            }

            if (spring) {
                for (int i = 0; i < rain.length; i++) {
                    rain[i].moveDownRain();
                }
            }
            else if (winter) {
                for (int i = 0; i < snow.length; i++) {
                    snow[i].moveDownSnow();
                }
            }
            repaint();
        }
    }

    public void actionPerformed(ActionEvent e) {

        if (e.getSource() == fallB) {
            fall = true;
            spring = false;
            winter = false;
            summer = false;
        }
        else if (e.getSource() == winterB) {
            winter = true;
            spring = false;
            summer = false;
            fall = false;
        }
        else if (e.getSource() == springB) {
            spring = true;
            winter = false;
            fall = false;
            summer = false;
        }
        else if (e.getSource() == summerB) {
            summer = true;
            spring = false;
            winter = false;
            fall = false;
        }
    }
}

Here is the Cloud class:

import java.awt.Color;
import java.awt.Graphics;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ThreadLocalRandom;

public class Cloud {

    private Color cloudColor;

    public static List<Integer> xs = new ArrayList<Integer>();

    private int x, y;

    private int x1 = 25, x2 = 65, x3 = 106;

    public Cloud () {

        cloudColor = new Color(128, 128, 128);

        x = ThreadLocalRandom.current().nextInt(50, 790);

        y = ThreadLocalRandom.current().nextInt(1, 100);

        xs.add(x);

    }

    public void drawCloud(Graphics g, double x) {

        g.setColor(cloudColor);

        g.fillOval((int) x + x1 + this.x, 25 + y, 70, 58);
        g.fillOval((int) x + x2 + this.x, 15 + y, 70, 58);  
        g.fillOval((int) x + x2 + this.x, 50 + y, 70, 58);
        g.fillOval((int) x + x3 + this.x, 33 + y, 70, 58);

    }
}

What do you expect the speed to be (i.e. how many pixels should they move per second?) — JB Nizet, Dec 01 '19 at 11:10

score 0 · Accepted Answer · answered Dec 01 '19 at 17:12

but for some reason the clouds are moving really fast

Not really sure what the problem is just some general comments about the code:

Thread.sleep(5); //in milliseconds

Sleeping for 5 ms would be a refresh rate of 200 which is way too high. Also, I don't think the Java clock is that precise to allow you to sleep for that small of a time slice. People tend to go for 60 frames which would be about 16 ms.

cloudX += 0.2;

Then you increment the movement by .2, which means the cloud will only move every 5 times through the loop anyway. So why do all the extra processing. Just increment the x value by 1 and change the sleep to 25ms.

public static List<Integer> xs = new ArrayList<Integer>();

You should not be using a static variable. Properties of the cloud should be instance variables.That is the Cloud object should be self contained so it has all the information necessary to paint itself.

So you need a variable like locationX.

for (int i = 0; i < cloud.length; i++) {
    getXs[i] += (int) cloudX;
    if (getXs[i] > 800) getXs[i] = -150;
}

After the above suggestion you get rid of the "getXs" variable. Instead you create a method like move(int pixels) to the Cloud class. The move(...) method simply updates the locationX variable to its new value.

The drawCloud(...) method would then be changed to only need the Graphics object. The painting would be changed to use the locationX variable.

public static List<Integer> xs = new ArrayList<Integer>();

I would guess the problem is the static variable. This means every Cloud object uses the same variable. So every time you increment the value for each Cloud you increase it for all clouds. The "x location" should be in instance variable.

    snow = new RainSnowDrop[200];
    rain = new RainSnowDrop[200];
    tree = new Tree[5];
    cloud = new Cloud[7];

Don't use arrays. Instead use ArrayList. This will allow your code to dynamically add objects without a limitation.

while (true) {

Don't use an infinite loop. Animation is done by using a Swing Timer. Read the section from the Swing tutorial on How to Use Timers for more information.

Check out: get width and height of JPanel outside of the class for an example that implements all the above suggestion. Note how the "Ball" class has all the properties needed to paint the ball. This allows you to customize its behaviour to be a different color and motion.

Thank you very much for giving me the feedback, I realized many things that I've done wrong and could make simpler — David Sinelnikov, Dec 02 '19 at 06:53

How to give each cloud an individual x position so that I could draw it based on that

1 Answers1