Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

TimerTask not work as expected

18 views
Skip to first unread message

SamuelXiao

unread,
Jan 12, 2011, 1:10:27 AM1/12/11
to
Hi all, I am writing a simple monopoly board game, there're only 2
tokens, one is controlled by human, another by PC. I am trying to
make it turn based and move around the map step by step. Then when I
use TimerTask to trigger the step-forward movement, it's ok for the
token controlled by human, but for the PC one. It doesn't move as
expected. Below is part of the codes for Dice Roll button & Done
button.

// btnRoll() function
public void btnRoll() {
Timer timer = new Timer();
final int index = turn - 1;
boolean snakeEyes = false;
Dice1 = (int)(Math.random() * 6 + 1);
Dice2 = (int)(Math.random() * 6 + 1);

if(Dice1 == Dice2) {
snakeEyes = true;
rolled = false;
}
else
rolled = true;

if(snakeEyes == true){
tempFlagPlayer = true;
}else{
tempFlagPlayer = false;
}
timer.schedule(new TimerTask(){
private int temp = Dice1 + Dice2;
public void run(){
if (temp > 0){
SystemLogHelper.debug("btnRoll()'s players.get(index): " + index);
movePlayer(players.get(index), tempFlagPlayer); // move player one
space each time
temp --;
}else{
checkPlayerMovedStatus(players,tempFlagPlayer);
cancel();
}
repaint();
}
}, 100L,100L);
repaint();
}

// PC turn
public void AIturn(int tempNumOfPlayers){
btnRoll();
SystemLogHelper.info("players.get(turn-1): " +
players.get(turn-1).getName());
SystemLogHelper.info("players.get(turn-1).getPosition(): " +
players.get(turn-1).getPosition());
if(propertymanager.Properties[players.get(turn-1).getPosition()]
[0] == 0){
SystemLogHelper.info("enter btnBuy()");
btnBuy();
}
if(rolled) btnDone(tempNumOfPlayers);
}

// done button, next player's turn
public void btnDone(int tempNumOfPlayers){
rolled = false;
if(this.getTurn() == tempNumOfPlayers){
this.setTurn(1);
}
else{
turn += 1;
// SystemLogHelper.info("AI's this.getTurn(): " + turn);
if((AutoPlayer)players.get(turn - 1) instanceof AutoPlayer){
AIturn(tempNumOfPlayers);
}
}
repaint();
}


The main problem is, I found that
SystemLogHelper.info("players.get(turn-1): " +
players.get(turn-1).getName());
and SystemLogHelper.info("players.get(turn-1).getPosition(): " +
players.get(turn-1).getPosition());
will run before the btnRoll() function.

how could I make sure that btnRoll() is done then go to the next
code?
Any help would be appreciated.

Travers Naran

unread,
Jan 12, 2011, 1:45:41 AM1/12/11
to
On 11/01/2011 10:10 PM, SamuelXiao wrote:
> Hi all, I am writing a simple monopoly board game, there're only 2
> tokens, one is controlled by human, another by PC. I am trying to
> make it turn based and move around the map step by step. Then when I
> use TimerTask to trigger the step-forward movement, it's ok for the
> token controlled by human, but for the PC one. It doesn't move as
> expected. Below is part of the codes for Dice Roll button& Done
> button.
>
> timer.schedule(new TimerTask(){
> private int temp = Dice1 + Dice2;
> public void run(){
> if (temp> 0){
> SystemLogHelper.debug("btnRoll()'s players.get(index): " + index);
> movePlayer(players.get(index), tempFlagPlayer); // move player one
> space each time
> temp --;
> }else{
> checkPlayerMovedStatus(players,tempFlagPlayer);
> cancel();
> }
> repaint();
> }
> }, 100L,100L);
> how could I make sure that btnRoll() is done then go to the next
> code?
> Any help would be appreciated.

Timer runs TimerTask in a _separate_ thread. There are a few ways you
could synchronize this, but I'd recommend looking at wait()/notify().
Try to remember that you are waiting for your TimerTask to be called
Dice1+Dice2 times before you leave.

SamuelXiao

unread,
Jan 12, 2011, 3:25:42 AM1/12/11
to
> Dice1+Dice2 times before you leave.- Hide quoted text -
>
> - Show quoted text -

Hi Travers,

Thanks for youjr suggestion, I use wait() now, but it comes to another
problem. I added wait() in the AITurn() method..Then now an exception
was caught..

public void AIturn(int tempNumOfPlayers){
long temp = (long) (Dice1 + Dice2) * 100;


btnRoll();
SystemLogHelper.info("players.get(turn-1): " +
players.get(turn-1).getName());
SystemLogHelper.info("players.get(turn-1).getPosition(): " +
players.get(turn-1).getPosition());

try {
this.wait(temp);
// notify();
// Thread.sleep(temp);
}
catch(InterruptedException e){}

if(propertymanager.Properties[players.get(turn-1).getPosition()]
[0] == 0){
SystemLogHelper.info("enter btnBuy()");
btnBuy();
}

btnDone(tempNumOfPlayers);

// }
// if(rolled) btnDone(tempNumOfPlayers);
}

// IllegalMonitorStateException
java.lang.IllegalMonitorStateException
at java.lang.Object.wait(Native Method)
at com.xxx.applet.MonopolyBoard.AIturn(MonopolyBoard.java:822)
at com.xxx.applet.MonopolyBoard.btnDone(MonopolyBoard.java:485)
at com.xxx.applet.MonopolyEntry.actionPerformed(MonopolyEntry.java:
140)
at javax.swing.AbstractButton.fireActionPerformed(Unknown Source)
at javax.swing.AbstractButton$Handler.actionPerformed(Unknown Source)
at javax.swing.DefaultButtonModel.fireActionPerformed(Unknown Source)
at javax.swing.DefaultButtonModel.setPressed(Unknown Source)

seems that if I use wait(), these btnDone()/btnRoll() will be lost its
eventlistener? Is my understanding correct? I tried notify()/sleep()
and so on...but they doesn't work. wait() method can provide the token-
movement as I wanted but it is able to trigger btnDone() method

Lew

unread,
Jan 12, 2011, 7:47:57 AM1/12/11
to
SamuelXiao wrote:
>>> Hi all, I am writing a simple monopoly board game, there're only 2
>>> tokens, one is controlled by human, another by PC. I am trying to
>>> make it turn based and move around the map step by step. Then when I
>>> use TimerTask to trigger the step-forward movement, it's ok for the
>>> token controlled by human, but for the PC one. It doesn't move as
>>> expected. Below is part of the codes for Dice Roll button& Done
>>> button.
>>
>>> timer.schedule(new TimerTask(){

Don't use TAB characters to indent Usenet posts; use spaces (up to four per
level).

>>> private int temp = Dice1 + Dice2;

You have not synchronized access do 'Dice1' (variable names should start with
a lower-case letter) or 'Dice2'.

...


>>> how could I make sure that btnRoll() is done then go to the next
>>> code?

Read up on 'volatile', 'synchronized' and other concurrent-programming
constructs in Java. Buy, read and study /Java Concurrency in Practice/ by
Brian Goetz, et al.

>>> Any help would be appreciated.

Travers Naran wrote:
>> Timer runs TimerTask in a _separate_ thread. There are a few ways you
>> could synchronize this, but I'd recommend looking at wait()/notify().
>> Try to remember that you are waiting for your TimerTask to be called
>> Dice1+Dice2 times before you leave.

SamuelXiao wrote:
> Thanks for youjr suggestion, I use wait() now, but it comes to another
> problem. I added wait() in the AITurn() method..Then now an exception
> was caught..
>
> public void AIturn(int tempNumOfPlayers){
> long temp = (long) (Dice1 + Dice2) * 100;
> btnRoll();
> SystemLogHelper.info("players.get(turn-1): " +
> players.get(turn-1).getName());
> SystemLogHelper.info("players.get(turn-1).getPosition(): " +
> players.get(turn-1).getPosition());
> try {
> this.wait(temp);

Putting 'this.' in front of method calls is useless and misleading.

> // notify();
> // Thread.sleep(temp);
> }
> catch(InterruptedException e){}
>
> if(propertymanager.Properties[players.get(turn-1).getPosition()]
> [0] == 0){
> SystemLogHelper.info("enter btnBuy()");
> btnBuy();
> }
> btnDone(tempNumOfPlayers);
>
> // }
> // if(rolled) btnDone(tempNumOfPlayers);
> }
> // IllegalMonitorStateException
> java.lang.IllegalMonitorStateException
> at java.lang.Object.wait(Native Method)
> at com.xxx.applet.MonopolyBoard.AIturn(MonopolyBoard.java:822)
> at com.xxx.applet.MonopolyBoard.btnDone(MonopolyBoard.java:485)

That exception is thrown when you call 'wait()' without having a "monitor"
(lock) held on the 'this' object. This is clearly stated in the Javadocs for
the method. Did you read them?

http://download.oracle.com/javase/6/docs/api/java/lang/Object.html#wait(long)
"The current thread must own this object's monitor."

You have to synchronize access to data shared between threads. This is a
rather large topic.

Read up on concurrent programming. Measure twice, cut once.

--
Lew
Ceci n'est pas une pipe.

SamuelXiao

unread,
Jan 12, 2011, 10:17:18 AM1/12/11
to
> http://download.oracle.com/javase/6/docs/api/java/lang/Object.html#wa...)

> "The current thread must own this object's monitor."
>
> You have to synchronize access to data shared between threads.  This is a
> rather large topic.
>
> Read up on concurrent programming.  Measure twice, cut once.
>
> --
> Lew
> Ceci n'est pas une pipe.

for dice1 & dice2 both are primitive type. And only AITurn() method
need to wait. May I ask how can I wait until another thread finish?
From my search, it just tells 1 thread call another thread to wait
instead of one thread wait for another thread to finish...

Ken Wesson

unread,
Jan 12, 2011, 10:23:21 AM1/12/11
to
On Wed, 12 Jan 2011 07:17:18 -0800, SamuelXiao wrote:
> May I ask how can I wait until another thread finish?

theThreadYouWantFinished.join();

SamuelXiao

unread,
Jan 12, 2011, 11:08:05 AM1/12/11
to

SOrry, I think my explanation is not clear. Because I use TimerTask
instead of thread, and for btnRoll()/btnDone() methods, these are in
fact the actionPerformed methods attached to those Roll/Done buttons,
I just wanna within the AITurn(), the btnRoll() will be finished
first(a timerTask is within it).

Travers Naran

unread,
Jan 12, 2011, 11:14:32 AM1/12/11
to
On 12/01/2011 12:25 AM, SamuelXiao wrote:
> long temp = (long) (Dice1 + Dice2) * 100;
> btnRoll();
> SystemLogHelper.info("players.get(turn-1): " +
> players.get(turn-1).getName());
> SystemLogHelper.info("players.get(turn-1).getPosition(): " +
> players.get(turn-1).getPosition());
> try {
> this.wait(temp);
> // notify();
> // Thread.sleep(temp);
> }
> catch(InterruptedException e){}
>
> // IllegalMonitorStateException
> java.lang.IllegalMonitorStateException
> at java.lang.Object.wait(Native Method)
> at com.xxx.applet.MonopolyBoard.AIturn(MonopolyBoard.java:822)
> at com.xxx.applet.MonopolyBoard.btnDone(MonopolyBoard.java:485)
> at com.xxx.applet.MonopolyEntry.actionPerformed(MonopolyEntry.java:
> 140)
> at javax.swing.AbstractButton.fireActionPerformed(Unknown Source)
> at javax.swing.AbstractButton$Handler.actionPerformed(Unknown Source)
> at javax.swing.DefaultButtonModel.fireActionPerformed(Unknown Source)
> at javax.swing.DefaultButtonModel.setPressed(Unknown Source)
>
> seems that if I use wait(), these btnDone()/btnRoll() will be lost its
> eventlistener? Is my understanding correct? I tried notify()/sleep()
> and so on...but they doesn't work. wait() method can provide the token-
> movement as I wanted but it is able to trigger btnDone() method

That's not how you use wait/notify. You've got it completely wrong,
Sam. Wait() is not some sort of sleep() command. It is part of a
synchronization system.

Please read:
http://download.oracle.com/javase/tutorial/essential/concurrency/index.html

Again, in your btnRoll() method, TimerTask is running in a SEPARATE
THREAD. I imagined you would figure out to do the synchronization in
btnRoll().

Your TimerTask needs to let your main thread know when it is done. So
think about two processes running beside each other with one process
waiting for the first process to finish.

Lew

unread,
Jan 12, 2011, 12:14:45 PM1/12/11
to

There is no successful "just wanna" in concurrent programming. It is
not cookie-cutter programming. Again, take the time to study the
matter before you jump in. You haven't even read the Javadocs for the
methods you're using, much less the Java tutorials on concurrent
programming, much less any serious study. The answers that you have
already received give you study points. Go thou and study them!

You are not going to get a successful solution with a brief Usenet
answer. Answers here are signposts to further research. Don't ask us
to write your program for you. Do the work. Study the materials.
There is no shortcut.

None.

--
Lew

Travers

unread,
Jan 12, 2011, 3:06:28 PM1/12/11
to
I don't think you understand the Java API you are using. You will not get the results you want until you understand what you are playing with.

Timer() spawns a thread and returns _instantly_ thus btnRoll() ends before the TimerTask is even executed. Timer is not "synchronous"--it does not wait for the tasks to finish.

But I also think you have structured your program poorly. For event-driven GUI programs, especially games, you have to give up "Do A then do B" structure.

But if you want to make sure btnRoll()'s Timer task thread finishes first, you should add code in btnRoll to _wait_ for the Timer task to finish.

Roedy Green

unread,
Jan 12, 2011, 3:26:15 PM1/12/11
to
On Tue, 11 Jan 2011 22:10:27 -0800 (PST), SamuelXiao
<foolsm...@gmail.com> wrote, quoted or indirectly quoted someone
who said :

>Dice1 = (int)(Math.random() * 6 + 1);

see http://mindprod.com/jgloss/pseudorandom.html
for better code to do that.
--
Roedy Green Canadian Mind Products
http://mindprod.com
To err is human, but to really foul things up requires a computer.
~ Farmer's Almanac
It is breathtaking how a misplaced comma in a computer program can
shred megabytes of data in seconds.

Roedy Green

unread,
Jan 12, 2011, 3:28:10 PM1/12/11
to
On Tue, 11 Jan 2011 22:10:27 -0800 (PST), SamuelXiao
<foolsm...@gmail.com> wrote, quoted or indirectly quoted someone
who said :

>Hi all, I am writing a simple monopoly board game, there're only 2


>tokens, one is controlled by human, another by PC

The usual problem is the background task is not allowed to touch the
GUI. Only the Swing thread is. When the background thread wants to
modify the GUI, it must use invokeAndWait or invokeLater.

See http://mindprod.com/jgloss/thread.html and chase links.

markspace

unread,
Jan 12, 2011, 5:27:53 PM1/12/11
to
On 1/12/2011 12:28 PM, Roedy Green wrote:
> On Tue, 11 Jan 2011 22:10:27 -0800 (PST), SamuelXiao
> <foolsm...@gmail.com> wrote, quoted or indirectly quoted someone
> who said :
>
>> Hi all, I am writing a simple monopoly board game, there're only 2
>> tokens, one is controlled by human, another by PC
>
> The usual problem is the background task is not allowed to touch the
> GUI. Only the Swing thread is. When the background thread wants to
> modify the GUI, it must use invokeAndWait or invokeLater.
>
> See http://mindprod.com/jgloss/thread.html and chase links.


Roedy has a point. javax.swing.Timer executes its tasks on the EDT, but
java.util.Timer does not. There's an unfortunate conicidence of class
names there; I think perhaps they should have named the former
javax.swing.SwingTimer or something.

Originally I thought that the OP was using javax.swing.Timer, but I no
longer think that is the case. Something else for him to fix up.

Mike Schilling

unread,
Jan 14, 2011, 12:17:54 AM1/14/11
to

"Travers" <tna...@gmail.com> wrote in message
news:be002bab-fb58-493f...@glegroupsg2000goo.googlegroups.com...


> I don't think you understand the Java API you are using. You will not get
> the results you want until you understand what you are playing with.
>
> Timer() spawns a thread and returns _instantly_

Or might not return for some time, depending on the whims of the thread
scheduler. The point is that you can't be sure either way without careful
use of synchronization.

Roedy Green

unread,
Jan 14, 2011, 7:32:06 PM1/14/11
to
See http://mindprod.com/jgloss/timer.html
for tips on making various flavours of timer work.

Arne Vajhøj

unread,
Jan 17, 2011, 7:56:37 PM1/17/11
to

wait()/notify() is not exactly the easiest way to synchronize
in Java.

It is more like the most difficult.

synchronized keyword or or some of the stuff in java.util.concurrent
would be a lot easier to get right.

Arne

markspace

unread,
Jan 17, 2011, 11:17:08 PM1/17/11
to
On 1/17/2011 4:56 PM, Arne Vajhøj wrote:
>
> wait()/notify() is not exactly the easiest way to synchronize
> in Java.
...

> some of the stuff in java.util.concurrent
> would be a lot easier to get right.


Specifically a CountDownLatch would be the way to go I think.

SamuelXiao

unread,
Jan 17, 2011, 11:54:09 PM1/17/11
to

Thanks for your help, I think I have solved the synchronous problem. I
have added an Object lock to do the syn.

public void btnRoll(){
final Timer timer = new Timer();


final int index = turn - 1;
boolean snakeEyes = false;

dice1 = (int)(Math.random() * 6 + 1);
dice2 = (int)(Math.random() * 6 + 1);

if(dice1 == dice2) {


snakeEyes = true;
rolled = false;
}

if(snakeEyes == true){


tempFlagPlayer = true;
}else{
tempFlagPlayer = false;
}

timer.schedule(new TimerTask(){
private int diceSum = dice1 + dice2;

public void run(){
synchronized(lock) {
if (diceSum > 0){


movePlayer(players.get(index), tempFlagPlayer); // move player
one space each time

diceSum --;
repaint();
}else{
checkPlayerMovedStatus(players,tempFlagPlayer);
propertymanager.CheckProperty(turn,
players.get(index).getPosition());
lock.notify();
rolled = true;
timer.cancel();
}
repaint();
}
}
}, 100L,100L);
repaint();
}

and in AIturn(int tempNumOfPlayers){

public void AIturn(int tempNumOfPlayers){
btnRoll();

synchronized(lock) {
if(!rolled){
try{
lock.wait();


}
catch(InterruptedException e){}
}
if(propertymanager.Properties[players.get(turn-1).getPosition()]
[0] == 0){
SystemLogHelper.info("enter btnBuy()");
btnBuy();
}

if(rolled) {
btnDone(tempNumOfPlayers);
}
}
}

but I found there is another problem for AIturn(int tempNumOfPlayers),
in the btnRoll() in AIturn(),

if (diceSum > 0){


movePlayer(players.get(index), tempFlagPlayer); // move player one
space each time

diceSum --;
repaint();
}

I found the repaint() doesn't work when it comes to AIturn() call, is
it repaint must be triggered by ActionListener component? If there any
way to force repaint()? Thanks.

John B. Matthews

unread,
Jan 18, 2011, 7:13:34 AM1/18/11
to
In article <ih348a$s0f$1...@news.eternal-september.org>,
markspace <nos...@nowhere.com> wrote:

OP: There's a nice example that uses CountDownLatch here:

<http://groups.google.com/group/comp.lang.java.programmer/msg/1d9c821dcda040f6>

--
John B. Matthews
trashgod at gmail dot com
<http://sites.google.com/site/drjohnbmatthews>

Lew

unread,
Jan 18, 2011, 8:49:14 AM1/18/11
to

Dude, that is just horrid code.

The idea with critical sections (parts that share data between threads) is to
keep them short and not do much inside them.

What data are you protecting with the lock?

I don't see any shared data that is completely guarded. That's fatal right there.

At least 'repaint()' is one of those methods that you can safely call outside
the EDT. Did you know that?

Read http://java.sun.com/products/jfc/tsc/articles/painting/index.html

You might not need to call 'repaint()' explicitly. I'd be surprised if you do.

Don't use 'wait()/notify()' at all.

You truly need to study concurrent Java programming. "Throw enough shit at
the wall and some of it will stick" is not good programming.

0 new messages