Cannot get this to work properly.

This is the code for my first game. So far Its supposed to be a circle that moves to wherever you click the mouse. The code compiles fine but when I run the applet the circle only moves along the y-axis, and not across. Could someone please tell me what’s wrong with it?


import java.applet.*;
import java.awt.*;
import java.awt.event.*;

public class App extends Applet implements MouseListener, Runnable
{
 Image offscreen;
 Graphics buffer;
 int posX, posY, clickX, clickY;
 Thread th;
 
 
 

 public void init()
 {
  offscreen = this.createImage(100,100);
  buffer = offscreen.getGraphics();

  addMouseListener(this);
  
 }


 public void paint(Graphics g)
 {
  buffer.setColor(Color.white);
  buffer.fillRect(0,0,100,100);
  buffer.setColor(Color.red);
  buffer.fillOval(posX - 5,posY - 5,10,10);
  g.drawImage(offscreen,1,1,this);
 }

 public void start()
 {
  if(th == null)
  {
   th = new Thread(this);
  }  
 }

 public void stop()
 { 
  
 }
  

 public void mouseClicked (MouseEvent me) 
 {
  clickX = me.getX();
  clickY = me.getY();
  
  th.start();
 }

 public void run()
 {
  while(isActive())
  {
   try
   {
    //CLICK IS TO THE RIGHT...
    if(clickX > posX)
    {
      //...and lower than ball
      if(clickY > posY)
      {
        posX ++;
        posY ++;
        repaint();
      }
      //...and higher than ball
      if(clickY < posY)
      {
        posX ++;
        posY --;
        repaint();
      }
      //...and equal height to the ball
      if(clickY == posY)
      {
        posX++;
        repaint();
      }
    }
    
    //CLICK IS TO THE LEFT...
    if(clickX > posX)
    {
      //...and lower than ball
      if(clickY > posY)
      {
        posX --;
        posY ++;
        repaint();
      }
      //...and higher than ball
      if(clickY < posY)
      {
        posX --;
        posY --;
        repaint();
      }
      //...and equal height to the ball
      if(clickY == posY)
      {
        posX--;
        repaint();
      }
    }
    
    //CLICK IS SAME X-CO AS BALL...
    if(clickX == posX)
    {
      //...and lower than ball
      if(clickY > posY)
      {
        
        posY ++;
        repaint();
      }
      //...and higher than ball
      if(clickY < posY)
      {
        
        posY --;
        repaint();
      }
      //...and equal height to the ball
      if(clickY == posY)
      {
        repaint();
        th.interrupt();
      }
    }
      
   }
   catch(Exception e)
   {
   }
  }
 }
  public void mouseEntered (MouseEvent me) {} 
  public void mousePressed (MouseEvent me) {} 
  public void mouseReleased (MouseEvent me) {}  
  public void mouseExited (MouseEvent me) {} 



}

I’m not sure exactly what you’re doing wrong, but you’re code is way more complicated than it has to be.

First of all, the Thread th is useless and might even be screwing things up because it gets restarted every time you click the mouse. Just include the code in the run method in the start method instead.

Second, the run method’s code is way too complicated. It should just be:

if(clickX > posX)
   posX ++;
else if(clickX < posX)
   posX--;

//similar code for y - completely seperate from code for x

You don’t need to call repaint because the Applet will get repainted anyways unless you’ve shut off passive rendering (which can be done with “setIgnoreRepaint(true)”, though there’s a little more to it than that). If you did shut off passive rendering, repaint won’t do anything anyways.

Also, the mouseClicked method only gets called when you press and release the mouse button without moving it off the Component. I don’t think that’s an issue here though.

he’s using Applet not JApplet.

//CLICK IS TO THE LEFT...
    if(clickX > posX)
    {
      //...and lower than ball
      if(clickY > posY)
      {
        posX --;
        posY ++;
        repaint();
      }
      //...and higher than ball
      if(clickY < posY)
      {
        posX --;
        posY --;
        repaint();
      }
      //...and equal height to the ball
      if(clickY == posY)
      {
        posX--;
        repaint();
      }
    }

not sure the issue of it not moving on the x-axis, but i do have these observations.

  1. That should be if clickX < posX

  2. use else if statements when you check your y position after already checking if the click is lower than the position. This prevents unnecessary checks from every having to execute.

  3. why execute posX-- or posX++ inside every if statement. Since that is the one constant of the larger if statement, call it outside of your y-value checks, and only execute posY-- or posY++ inside your y-value checks.

  4. there are certainly better ways to do this.

Gotta head to class now, but If no one else has provided a better solution, I’ll reply again later tonight.

Thanks for the replies!

I think this is the reason for not moving along x

the first if clickX > posX moves the ball to the right, and the second if clickX >posX moves it right back!! Silly me! :-[
I changed the code and added in sleep() so the ball moves slowly.

import java.applet.*;
import java.awt.*;
import java.awt.event.*;

public class App extends Applet implements MouseListener, Runnable
{
 Image offscreen;
 Graphics buffer;
 int posX, posY, clickX, clickY;
 Thread th;
 
 
 

 public void init()
 {
  offscreen = this.createImage(100,100);
  buffer = offscreen.getGraphics();

  addMouseListener(this);
  
 }


 public void paint(Graphics g)
 {
  buffer.setColor(Color.white);
  buffer.fillRect(0,0,100,100);
  buffer.setColor(Color.red);
  buffer.fillOval(posX - 5,posY - 5,10,10);
  g.drawImage(offscreen,1,1,this);
 }

 public void start()
 {
  if(th == null)
  {
   th = new Thread(this);
  }  
 }

 public void stop()
 { 
  
 }
  

 public void mouseClicked (MouseEvent me) 
 {
   clickX = me.getX();
   clickY = me.getY();
   
   th.start();
   
 }

 public void run()
 {
  while(isActive())
  {
   try
   {
     if(clickX > posX)
     {
       posX ++;
     }
     else if(clickX < posX)
     {
       posX --;
     }
     
     if(clickY > posY)
     {
       posY ++;
     }
     else if(clickY < posY)
     {
       posY --;
     }
     
     if(posX != clickX || posY !=clickY)
     {
       repaint();
       th.sleep(100);
     }
   }
   catch(Exception e)
   {
   }
  }
 }
  public void mouseEntered (MouseEvent me) {} 
  public void mousePressed (MouseEvent me) {} 
  public void mouseReleased (MouseEvent me) {}  
  public void mouseExited (MouseEvent me) {} 



}

It all works but the second time (and following times) MouseClicked is called I get like 8 error messages in command. I think its because I call start when the thread is already running but I cant see any other way of doing it. Looks like I need some help once again!

Just take the Thread out. You shouldn’t need it at all. You would only need a Thread there if the program were doing alot of computation in mouseClicked, which it isn’t.

Or actually end the Thread when it’s done - all you have to do is take out the while loop. That wouldn’t be perfect though because mouseClicked might get called again before the Thread is done. You would have to create a new Thread each time (or use some kind of ThreadPool). It’s just not worth it here.

Or set a boolean saying rather the Thread is running. This would require some synchronization. I don’t want to spend the time to test it to make sure it works (or post code that doesn’t work here), so you’ll just have to learn about Threads if you want to do it that way.

Frankly, multithreading is complicated. I don’t do it except where necessary. After I move, I plan to buy a book about multithreading and learn more about it. You can’t just toss in Threads at random - you’ll have to learn about multithreading before you can use them effectively.

Or try this;

public void start()
 {
  if(th == null)
  {
   th = new Thread(this);
   th.start();
  }  
 }

 public void mouseClicked (MouseEvent me) 
 {
   clickX = me.getX();
   clickY = me.getY();
 }

My java book shows how to do this but it uses deprecated methods. Fletchergames, when you say take out the thread alltogether, how would you control the speed of the animation? Or use doublebuffering?
When I followed SimonH,s advice the ball wouldn’t move at all. Is there a way of slowing down a loop, timers or something?

Note that using passive rendering the animation is never under your control. Things wil render when AWT/Swing think its appropriate. AWT will consolidate render calls for efficiency so just because you call update 10 times in a second doesn’t mean it will render 10 time in that second, or 10 times at all.

This is one of a number of reasons most serious games don’t use passive rendering and generally avoid Swing altogether.

You might want to try starting with the tutorials at cokeandcode.com

[quote=“106498,post:8,topic:29753”]
Oops! I just compiled and ran it & you’re right! I fixed it though;

 public void run()
 {
  int moveSpeed=4;
  while(true)
  {
   try
   {
     if(clickX > posX)
     {
       posX+=moveSpeed;
     }
     else if(clickX < posX)
     {
       posX-=moveSpeed;
     }
&c...

You can change moveSpeed & the sleep() time to adjust the speed.

Since you’re using mouseClicked, not mousePressed, the circle should only move one pixel (or however far you want it to move) each time you click the mouse. So you should be able to remove the thread. See the following code:

public void mouseClicked (MouseEvent me) 
 {
   clickX = me.getX();
   clickY = me.getY();
   
     if(clickX > posX)
     {
       posX ++;
     }
     else if(clickX < posX)
     {
       posX --;
     }
     
     if(clickY > posY)
     {
       posY ++;
     }
     else if(clickY < posY)
     {
       posY --;
     }
 }

It would be different if you had alot of stuff happening in mouseClicked. If that were the case, the EDT would be busy running that code, and the interface would be unresponsive during that time. That’s when you would need a thread. Or, at least, that’s the way it looks to me.