3

Basicly, I'm making a game which to update the players position, it uses this thread:

@Override
public void run() {
    while(true) {
        System.out.println();
        updatePos(x, y);
    }

}

Which works fine, but if I remove the System.out.println(), it ceases to function. I have no idea why this is, the whole class is as follows:

public class Player extends Block implements KeyListener, Runnable {

int x;
int y;
int speed;

boolean upPressed; 
boolean downPressed;
boolean rightPressed;
boolean leftPressed;

static Sprite sprite = new Sprite("grass.png");

public Player(int x, int y, int speed) {
    super(x, y, sprite);
    this.x = x;
    this.y = y;
    this.speed = speed;
    Thread playerThread = new Thread(this, "Player Thread");
    playerThread.start();
}

@Override
public void keyPressed(KeyEvent e) {
    if (e.getKeyCode() == KeyEvent.VK_UP) {
        y -= speed;
    }

    if (e.getKeyCode() == KeyEvent.VK_DOWN)
        downPressed = true;

    if (e.getKeyCode() == KeyEvent.VK_RIGHT)
        rightPressed = true;

    if (e.getKeyCode() == KeyEvent.VK_LEFT)
        leftPressed = true;

}

@Override
public void keyReleased(KeyEvent e) {
    if (e.getKeyCode() == KeyEvent.VK_UP)
        upPressed = false;

    if (e.getKeyCode() == KeyEvent.VK_DOWN)
        downPressed = false;

    if (e.getKeyCode() == KeyEvent.VK_RIGHT)
        rightPressed = false;

    if (e.getKeyCode() == KeyEvent.VK_LEFT)
        leftPressed = false;

}

@Override
public void keyTyped(KeyEvent e) {

}

@Override
public void run() {
    while(true) {
        System.out.println();
        updatePos(x, y);
    }

}}

Final note: Ingore the boolean upPresses and downPressed, the only thing I'm focusing on is this:

if (e.getKeyCode() == KeyEvent.VK_UP) {
    y -= speed;
}
Derek
  • 882
  • 1
  • 14
  • 36
  • 2
    what exactly isn't working properly? does it run? does it enter updatePos? You can use a breakpoint to find that out... – Eran Zimmerman Gonen Aug 21 '11 at 19:32
  • You accepted an answer that fixed your problem. However your code is not synchronized and it will not work on some other computers, so you should fix the synchronization issues too. Also, it's considered bad practice to start a thread from a constructor. – toto2 Aug 21 '11 at 21:43
  • Possible duplicate of https://stackoverflow.com/questions/25425130/loop-doesnt-see-changed-value-without-a-print-statement – Raedwald Jan 10 '19 at 12:01

5 Answers5

7

A loop like

while(true) {
    updatePos(x, y);
}

would completely hog the CPU. Reason it starts behaving better with a println is probably because you yield a few hundred cycles per iteration for I/O.

I suggest you add a small sleep-method according to the desired frame rate:

while (true) {
    try {
        Thread.sleep(10); // for 100 FPS
    } catch (InterruptedException ignore) {
    }
    updatePos(x, y);
}

or, even better, go with an event driven approach with for instance a java.util.Timer. (That would be more Java idiomatic.)

aioobe
  • 413,195
  • 112
  • 811
  • 826
  • 1
    +1 And it's probably because `System.out.println()` is quite expensive that it inadvertently "fixed" the issue. – Paul Bellora Aug 21 '11 at 19:33
  • 4
    @Kublai: Probably more because `System.out.println` is synchronized. – C. K. Young Aug 21 '11 at 19:36
  • @Chris for the synchronization to be effective between the GUI thread and the running thread, something in the GUI thread would have to sometimes lock on the same lock used by System.out.println (whatever that lock might be). – toto2 Aug 21 '11 at 20:18
  • Even a `sleep(0)` did work for me. Don't know why yet. But without the `sleep(x)`, the instructions AFTER the while loop in the thread would never be called when the thread is meant to stop. With the `sleep(x)` (even with `sleep(0)`) the instructions after the loop are called when I stop the thread. That's interesting. – ElectRocnic Mar 29 '19 at 07:57
  • Oh, there I have found my own answer on my own question from above: https://stackoverflow.com/questions/1600572/are-thread-sleep0-and-thread-yield-statements-equivalent – ElectRocnic Mar 29 '19 at 08:04
1

A Thread.yield() can also do it in some cases. See https://www.javamex.com/tutorials/threads/yield.shtml for example. The author of this link does not recommend to use yield however.

ElectRocnic
  • 1,275
  • 1
  • 14
  • 25
1

Don't know if that would solve it, but it looks like you're giving the JVM a hard time... Try adding a short sleep (which is the correct way to implement a game thread) before each updatePos(x, y) call.

@Override
public void run() {
    while(true) {
    Try
    {
        Thread.sleep(50);
    }
    catch (Exception e){}

    updatePos(x, y);
    }

}}

IncrediApp
  • 10,303
  • 2
  • 33
  • 24
1

You didn't post the code to updatePos. But at a minimum, you need to make that method, as well as keyPressed (and anything else that uses x and/or y) synchronized.

Alternatively you can make both x and y volatile, but then x and y could get updated out of lockstep.

But without any memory synchronisation, changes from one thread are not guaranteed to be observable from another thread.

C. K. Young
  • 219,335
  • 46
  • 382
  • 435
  • @Deza A more detailed explanation if you don't know much about threads and synchronization: `keyPressed` is called from the Swing thread, so the value `y` is changed in that thread (the cache on one of your cpu core has the new `y` value). However, the thread that runs `run` and `updatePos` probably runs on another cpu core where the local cache has no clue that it should update `y` (unless you synchronize your code). – toto2 Aug 21 '11 at 20:09
0

There is yet another appoach. You can use class java.util.Timer that is dedicated for implementation of scheduled tasks. It is very useful if for example you need now 2 threads that periodically perform some kind of different operations. Using Timer you can save your efforts and computer resources using only one thread

AlexR
  • 114,158
  • 16
  • 130
  • 208