5

I'm making pong in java

If the ball goes out of bounds, pause is set to true:

    if (ball.getX() <= 0) {
        score2++;   
        pause = true;       
    }
    if (ball.getX() >= this.getWidth()-ballWidth) {
        score1++;
        pause = true;
    }

which should sleep the timer... after the thread has slept for 1000ms, pause will be set to false and the ball should continue moving (ball.autoMove()):

public void timer() {
    int initialDelay = 1000;

    timer.scheduleAtFixedRate(new TimerTask() {
        public void run() {
            if (pause) {
                try {   
                    ball.reset(width/2, height/2);
                    Thread.sleep(1000);     
                    pause = false;                      
                } 
                catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }
            ball.autoMove(velocityX, velocityY);    
            collisionCheck();           
        }
    }, initialDelay, 100);  
}

Ball Class AutoMove() function:

public void autoMove(double velX, double velY) {
    xLoc = this.getX()+velX;    
    yLoc = this.getY()+velY;    
}

It does all of that... it sleeps, pause is set to false, etc... but when the ball is reset (reset in the middle of the screen...) after it pauses for 1 second, it jumps to the far side of the game panel which tells me that while the thread was "sleeping", ball.autoMove(velocityX, velocityY); was still updating the coordinates.

Why is this happening? Shouldn't that not be run?

Thanks!

user3871
  • 12,432
  • 33
  • 128
  • 268

3 Answers3

6

The instruction

timer.scheduleAtFixedRate(new TimerTask() {...}, initial, 100 );

add 10 occurrences of autoMove to the timer queue during the sleep of 1 second.

The scheduling at fixed rate is right but you have to do nothing in place of sleeping.

Set a counter to 10 and decrement it each time the method is called and when it reach zero, keep going on the normal way (in opposition to the "pause" way).

timer.scheduleAtFixedRate(new TimerTask() {
   public void run() {
      if( pauseCount == 10 ) {
         ball.reset(width/2, height/2);
      }
      if( pauseCount > 0 ) {
         --pauseCount;
      }
      else {
         ball.autoMove( velocityX, velocityY );
         collisionCheck();
      }
   }
}, initialDelay, 100 );

Follow the execution log with a sleep.

First the program :

import java.util.Timer;
import java.util.TimerTask;
import java.util.concurrent.TimeUnit;

public class SO {

   static boolean isTrue = true;

   public static void main( String[] args ) {
      new Timer().scheduleAtFixedRate( new TimerTask() {
         @Override public void run() {
            System.out.println(
               System.currentTimeMillis() + ": " + Thread.currentThread());
            try{
               if( isTrue ) {
                  TimeUnit.SECONDS.sleep( 1 );
                  isTrue = false;
               }
            }
            catch( InterruptedException e ){
               e.printStackTrace();
            }
         }
      }, 0, 100 );
   }
}

And the log, observe the time:

1369769673294: Thread[Timer-0,5,main]
1369769674308: Thread[Timer-0,5,main] <-- #1 at 308
1369769674308: Thread[Timer-0,5,main] <-- #2 at 308
1369769674308: Thread[Timer-0,5,main]
1369769674308: Thread[Timer-0,5,main]
1369769674308: Thread[Timer-0,5,main]
1369769674308: Thread[Timer-0,5,main]
1369769674308: Thread[Timer-0,5,main]
1369769674308: Thread[Timer-0,5,main]
1369769674308: Thread[Timer-0,5,main]
1369769674308: Thread[Timer-0,5,main] <-- #10 at 308, queued during the sleep
1369769674402: Thread[Timer-0,5,main]
1369769674496: Thread[Timer-0,5,main]
1369769674605: Thread[Timer-0,5,main]
1369769674699: Thread[Timer-0,5,main]
1369769674808: Thread[Timer-0,5,main]
1369769674901: Thread[Timer-0,5,main]
1369769674995: Thread[Timer-0,5,main]

We observe the activation are like queued but in fact, the class Timer has only one thread. It's its logic which simulate the queue of events, the documentation of java.util.Timer is clear:

In fixed-rate execution, each execution is scheduled relative to the scheduled execution time of the initial execution. If an execution is delayed for any reason (such as garbage collection or other background activity), two or more executions will occur in rapid succession to "catch up." In the long run, the frequency of execution will be exactly the reciprocal of the specified period (assuming the system clock underlying Object.wait(long) is accurate). As a consequence of the above, if the scheduled first time is in the past, then any "missed" executions will be scheduled for immediate "catch up" execution.

Aubin
  • 14,617
  • 9
  • 61
  • 84
  • Really? Wow... How do I fix that? – user3871 May 28 '13 at 18:43
  • 2
    @Growler I think you should be fine just using `scheduleWithFixedDelay` instead. – Mattias Buelens May 28 '13 at 18:44
  • when is timer() called? if is called repeatedly then you can use what was suggested by @Aubin and make the variable volatle so same value is read in all threads. you have not shown in which thread pause is made true ( i guess the UI thread?) – tgkprog May 28 '13 at 18:51
  • @Aubin I don't understand why I can't use `Thread.sleep()` then... when is it okay to use that? – user3871 May 28 '13 at 18:54
  • 1
    Don't mix Sleep and Schedule, the first pause a thread and the second guarantee that a method will be called exactly every xx delay. It's like running into a plane, sit down and relax, the plane fly even you do nothing... – Aubin May 28 '13 at 18:58
  • @Marko Topolnik: Yes, it's true and the log is the proof – Aubin May 28 '13 at 19:46
2

While this thread is sleeping, it is guaranteed to not be calling autoMove. But, without looking at the code, I'd bet that autoMove is moving the ball based on the change in real time. That will make the ball appear to jump once the pause is through and you call autoMove again.

You will need to change autoMove to be based on relative time, or you will need to change the time variable it uses and subtract out your paused time.

greedybuddha
  • 7,488
  • 3
  • 36
  • 50
  • It's more than a guess. We know for a fact it's not calling it while sleeping. So with that being guaranteed, this is the only explanation – greedybuddha May 28 '13 at 18:42
0

Will be good to see the full code of where ever pause is being changed.

How do you know it happened while it was sleeping? can you add loggers with time stamp to be sure? But I will take a stab.

If the pause is being set in another thread then you need to synchronize on it and be a great idea if you declare it volatile. synchronized in this method and where ever else pause is being set.

private volatile Boolean pause = false;
//...
//make a setter and getter for pause
synchronized ( pause    ){
    return  pause ;    
}

synchronized ( pause    ){
    return  pause = newPauseValue;    
}
tgkprog
  • 4,493
  • 4
  • 41
  • 70