I am working on a project that has a GUI and tanks that move. While tanks move fine, I am not able to figure out how to move/rotate them individually. I also need to clean my code as I feel I have a lot of extra stuff going on.

Here is some code, I can post more if required, but here's what I tried.

I have 4 classes. Missiles, Tanks, and Board. I am calling keylisteners in Tank class, should I do that in the doDrawing method? doDrawing method is in Board class.

private void doDrawing(Graphics g)
{
    final double rads = Math.toRadians(120);
    final double sin = Math.abs(Math.sin(rads));
    final double cos = Math.abs(Math.cos(rads));
    final int w = (int) Math.floor(tank1.getX() * cos   tank1.getX() * sin);
    final int h = (int) Math.floor(tank1.getY() * cos   tank1.getY() * sin);

    Graphics2D g2d = (Graphics2D) g;
    g2d.translate(w, h);
    g2d.rotate(rot, tank1.getX(), tank1.getY());

    AffineTransform backup = g2d.getTransform();
    AffineTransform trans = new AffineTransform();
    
    g2d.setTransform(backup);
    //g2d.drawImage(tank1.getImage(), tank1.getX(), tank1.getY(), this);
    
    trans.setToIdentity();
    trans.rotate(rot, h, w); 
    trans.translate(h,w);
    trans.setTransform(backup);
                
    g2d.drawImage(tank1.getImage(), tank1.getX(), tank1.getY(), this);

    //g2d.drawImage(tank1.getImage(), tank1.getX(), tank1.getY(), this);
    g2d.drawImage(tank2.getImage(), tank2.getX(), tank2.getY(), this);
    
    List<Missile> missiles = tank1.getMissiles();

    for (Missile missile : missiles)
    {    
        //trans.rotate(Math.toRadians(rads), w/2, h/2); 
        g2d.drawImage(missile.getImage(), missile.getX(), missile.getY() - 7, this);
        //g2d.rotate(rot, missile.getX(), missile.getY() - 7);
    }
}

CodePudding user response：

Recommendations:

Keep logic separate from display using a model-view code structure
In keeping with that, create a Tank logical class, one that knows its own position and its own orientation (or angle), and save your java.util.List<Tank> in the program's model.
Change the Tank position and orientation within a listener.
Use the View classes (the GUI classes) only to a) draw the state of the class's model and b) get input from the user.
Side recommendation: If this is Swing, avoid KeyListeners and instead use Key Bindings
You can use separate affine transforms for each tank

CodePudding user response：

If your tanks move/rotate synchronously I guess the reason is that they all receive the same key events and react the same way.

Put the keylistener (or even better KeyBindings as suggested in another answer) into your Board class. The listener will check which key is pressed/released and give the correct tank the command to move or turn or whatever. This way you can make one tank react to WASD, while the other reacts on up/down/left/right.

The methods in the tank class however do not react to key events but could be named like forward, backward, turnleft, turnright.

The doDrawing method should do exactly as it's name suggests: just perform the drawing. This can happen any amount of times to get good screen refresh rates or redraw the board after another window no longer hides your game.

CodePudding user response：

So, just looking at...

private void doDrawing(Graphics g)
{
    //...
    Graphics2D g2d = (Graphics2D) g;
    g2d.translate(w, h);
    g2d.rotate(rot, tank1.getX(), tank1.getY());

    AffineTransform backup = g2d.getTransform();
    AffineTransform trans = new AffineTransform();
    
    g2d.setTransform(backup);
    //g2d.drawImage(tank1.getImage(), tank1.getX(), tank1.getY(), this);
    
    trans.setToIdentity();
    trans.rotate(rot, h, w); 
    trans.translate(h,w);
    trans.setTransform(backup);
                
    g2d.drawImage(tank1.getImage(), tank1.getX(), tank1.getY(), this);
    g2d.drawImage(tank2.getImage(), tank2.getX(), tank2.getY(), this);
    //...
}

It's immediately obvious that you're applying a transformation to the Graphics context which is never undone, so everything paint will have the same transformation applied to it.

The first thing you need to do is break the paint logic down so that it can paint a single entity.

The second thing you need to do is create a copy of the Graphics context before you make any modifications to it, this will prevent the parent copy from carrying those modifications forward which will then be compounded with future modifications.

Maybe something like...

private void doDrawing(Graphics g) {
    // Because I don't trust anybody
    Graphics2D g2d = (Graphics2D) g.create();
    drawTank(tank1, g2d);
    g2d.dispose();
    g2d = (Graphics2D) g.create();
    drawTank(tank2, g2d);
    g2d.dispose();
}

private void drawTank(Tank tank, Graphics2D g) {
    final double rads = Math.toRadians(120);
    final double sin = Math.abs(Math.sin(rads));
    final double cos = Math.abs(Math.cos(rads));
    final int w = (int) Math.floor(tank.getX() * cos   tank.getX() * sin);
    final int h = (int) Math.floor(tank.getY() * cos   tank.getY() * sin);

    Graphics2D g2d = (Graphics2D) g.create();
    g2d.translate(w, h);
    // `rot` should be derived from the tank ... no idea where this is
    g2d.translate(tank.getX(), tank.getY());
    // Origin has now been translated to the tanks position
    g2d.rotate(rot, 0, 0);
    g2d.drawImage(tank.getImage(), 0, 0, this);
    
    // We no longer to need to keep these resources
    g2d.dispose();
    // Create a new "clean" copy
    g2d = (Graphics2D) g.create();
    // I don't know if it's needed, but I would, personally, calculate
    // any required rotation again, but that's me
    List<Missile> missiles = tank1.getMissiles();
    for (Missile missile : missiles) {
        g2d.drawImage(missile.getImage(), missile.getX(), missile.getY() - 7, this);
    }
}

For example, see: