5
\$\begingroup\$

I'm trying to make a small shooter game, where you can only see a small portion of the map around you. The map will also be much larger then the actual JPanel (The panel is 1000 by 1000, the map currently is 5000 by 5000).

When I added movement, the game seemed to lag slightly, mainly noticeable in the graphics flickering, however the game was still playable

Adding in a few walls drawn on the map significantly dropped performance, however, and now the game is entirely unplayable.

I'm looking for some tips to help optimize my code to lower this lag.

The other major problem I've been having is in regards to how I draw the map. When I move the area of the map I'm able to see down and to the left, the actual image moves up and to the right. This means that the Maps coordinates (and therefore the players coordinates), are negative. This isn't entirely game breaking, but is an annoyance nonetheless.

I've only included classes associated with movement below, but if the JPanel and JFrame classes are necessary, I can add them

Edit Removing the line

image.getGraphics().draw3DRect(10, 10, 10, 10, true);

seemed to speed up the game considerably, to the point of near to no lag However, I'm still almost certain my code is far from efficient - are there any other tweaks I could use to speed it up?

GameManager (Holds the gamethread, and runs the majority of the logic)

package Logic;

import java.util.ArrayList;
import Runner.Panel;
import Entities.*;
import java.awt.Graphics;
import Movement.*;

public class GameManager {
    private final Panel gamepanel;
    private boolean game_is_running = true;
    private final Map map = new Map();
    private Player player = new Player(100, 100);
    private Movement movement = new Movement(0);
    ArrayList<Wall> walls = new ArrayList<Wall>();

    public GameManager(Panel p) {
        gamepanel = p;
        initializeWalls();
        gamethread.start();
    }

    private void initializeWalls() {
        //adds walls on the map image
        walls.add(new Wall(2000, 2000, 100, 50));
        walls.add(new Wall(500,500,1000,500));
        walls.add(new Wall(0,500,100,1000));
        map.addWalls(walls);
    }

    private final Thread gamethread = new Thread() {
        public void run() {
            final byte UPDATES_PER_SECOND = 45;
            final double ups = 1000 / UPDATES_PER_SECOND;
            long iTime = System.currentTimeMillis();
            while (game_is_running) {
                final long ftime = System.currentTimeMillis();
                final long eTime = ftime - iTime;
                if (eTime >= ups) {
                    iTime = System.currentTimeMillis();
                   //updates logic 60 times a second
                    update();
                }
                //repaints the game as fast as it can
                gamepanel.repaint();
            }
        }
    };

    private void update() {
        //Updates the position of the player
        movement.Update(gamepanel, player, map);
    }

    public void Draw(Graphics g) {
        //Calls the drawing of the map and player
        map.Draw(g);
        player.Draw(g, map);
    }
}

Movement (Updates positions of the objects on screen)

package Movement;

import Entities.*;
import Runner.Panel;

public class Movement {
    private float angle = 0;
    private int speed = 20;

    public Movement(float a) {
        angle = a;
    }

    private void addangle(float a) {
        angle += a;
    }

    private void moveForward(Player p) {
        float xpos = (float) (speed * Math.sin(Math.toRadians(angle)));
        float ypos = (float) (speed * Math.cos(Math.toRadians(angle)));
        p.setX(p.getX() - xpos);
        p.setY(p.getY() - ypos);
    }

    public void Update(Panel panel, Player p, Map m) {
        if (panel.isUp()) {
            moveForward(p);
        }
        if (panel.isDown()) {
            moveBack(p);
        }
        if (panel.isLeft()) {
            addangle(5);
        }
        if (panel.isRight()) {
            addangle(-5);
        }
        m.setX(p.getX() - 450);
        m.setY(p.getY() - 450);
    }

    private void moveBack(Player p) {
        float xpos = (float) (speed * Math.sin(Math.toRadians(angle)));
        float ypos = (float) (speed * Math.cos(Math.toRadians(angle)));
        p.setX(p.getX() + xpos);
        p.setY(p.getY() + ypos);
    }

}

Entity (Abstract class for all objects on screen)

package Entities;

import java.awt.image.BufferedImage;

public abstract class Entity {

    float x, y;
    int width, height;
    BufferedImage image;

    public float getX() {
        return x;
    }

    public void setX(float x) {
        this.x = x;
    }

    public float getY() {
        return y;
    }

    public void setY(float y) {
        this.y = y;
    }

    public int getWidth() {
        return width;
    }

    public int getHeight() {
        return height;
    }

    public BufferedImage getImage() {
        return image;
    }

}

Map (Holds the map image)

 package Entities;

 import java.awt.Graphics;
 import java.io.File;
 import javax.imageio.ImageIO;
 import java.util.ArrayList;

 public class Map extends Entity {
     public Map() {
         try {
             image = ImageIO.read(new File("resources/Maps/TestMap.jpg"));
         } catch (Exception ex) {
             System.out.println("WARNING: MAP NOT FOUND" + "\n" + "ERROR CODE 1");
             System.exit(-1);
         }
     }

    public void addWalls(ArrayList<Wall> walls){
        for(Wall w:walls){
            image.getGraphics().drawRect((int)w.getX(),(int)w.getY(),w.getWidth(),w.getHeight());
        }
    }

    public void Draw(Graphics g) {
        image.getGraphics().draw3DRect(10, 10, 10, 10, true);
        g.drawImage(image, (int) x, (int) y, null);
    }
}

Player (Holds position of the player)

package Entities;

import java.awt.Graphics;

public class Player extends Entity {
    public Player(float x, float y) {
        this.x=x;
        this.y=y;
    }

    public void Draw(Graphics g, Map m) {
        g.drawRect((int)(m.getX()*-1-x*-1), (int)(m.getY()*-1-y*-1), 100, 100);
    }
}

Wall (holds position of walls (I've not added collision in yet)

package Entities;
import java.awt.Rectangle;
public class Wall extends Entity{
    public Wall(int x, int y, int width, int height){
        this.x = x;
        this.y = y;
        this.width = width;
        this.height = height;
    }

    public Rectangle getBounds(){
        Rectangle ret = new Rectangle((int)x,(int)y,width,height);
        return ret;
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Hey, just wanted to confirm something. Your code works, right? No crashes or bugs? It's just the performance causing issues? \$\endgroup\$ – Fund Monica's Lawsuit May 21 '16 at 17:58
  • \$\begingroup\$ @QPaysTaxes Yeah it works fine, just laggy is all \$\endgroup\$ – Acuzik May 21 '16 at 19:12
  • \$\begingroup\$ Alright. If it was broken code, it would be off-topic, but that's not the case here. Thanks! \$\endgroup\$ – Fund Monica's Lawsuit May 21 '16 at 19:13
2
\$\begingroup\$

You mentioned that the main goal was performance. I'll summarize some hints, of which some touch the performance aspect, and others may affect the performance, and others are simply intended to improve the code in other ways:


1) You mentioned that removing the line

image.getGraphics().draw3DRect(10, 10, 10, 10, true);

caused a speedup. Although I can't imagine why this should be the case, as this is only done once when the map is painted, one should solve this differently - because it is done once each time that the map is painted. This is simply not necessary. You can add this rectangle to the image once in the constructor, after the image has been loaded. And then, you should dispose the graphics when you no longer need it. The same applies for other cases where you directly operate on image.getGraphics().

IIRC, the dispose call on the actual image implementation returned by ImageIO is a no-op. But this is not guaranteed. Future implementations might require dispose to be called.

Additionally, you should consider setting the color that you want the rectangle to be painted with. Otherwise, it is not clear what color the rectangle will have.

public class Map extends Entity {
    public Map() {
    try {
        image = ImageIO.read(new File("resources/Maps/TestMap.jpg"));
    } catch (Exception ex) {
        System.out.println("WARNING: MAP NOT FOUND" + "\n" + "ERROR CODE 1");
        System.exit(-1);
    }

    Graphics g = image.getGraphics();
    g.setColor(Color.BLACK);
    g.draw3DRect(10, 10, 10, 10, true);
    g.dispose();
}

(Note that the way of loading and the error handling are horrible as well. You should likely not load the image from a fixed path. Instead, you should use getClass().getResourceAsStream() to obtain the image input stream. This will become important when you want to deploy the application as a JAR file. Additionally, System.exit(...) usually is a no-go. Creating a "dummy image" and/or printing an error message should be sufficient here.


2) A wild guess, but this can make a significant (!) difference: You could make sure that the images are in a format that can be painted efficiently. When you load an image with ImageIO, you never know which BufferedImage.TYPE_... the resulting image has. It's often a good idea to convert the image into a known type that can be painted efficiently. For example, you can use a method

private static BufferedImage convertToARGB(BufferedImage image)
{
    BufferedImage newImage = new BufferedImage(
        image.getWidth(), image.getHeight(),
        BufferedImage.TYPE_INT_ARGB);
    Graphics2D g = newImage.createGraphics();
    g.drawImage(image, 0, 0, null);
    g.dispose();
    return newImage;
}    

and then load your image as

image = convertToARGB(ImageIO.read(...));

3) The line

g.drawRect((int)(m.getX()*-1-x*-1), (int)(m.getY()*-1-y*-1), 100, 100);

looks odd. Negating a number by multiplying it with -1 is something that I'd consider as a very bad style. It should likely be

g.drawRect((int)(-m.getX()+x), (int)(-m.getY()+y), 100, 100);

instead.


4) Parts of the "lags" that you are experiencing may come from the game loop. It explicitly says

//repaints the game as fast as it can
gamepanel.repaint();

which should simply not be necessary. Swing is internally coalescing the repaint triggers. This means that when you call repaint() 1000 times in a row in a very short time, it may effectively cause the component to be painted only once. However, this does not come for free. Flooding the paint system with unnecessary repaint() triggers is certainly not beneficial. You should only call repaint() when there really have been changes. Namely, after the call to update(). (In fact, you might even consider a short Thread#sleep in the main loop, but this depends on several other factors).


5) Although it is not used yet: The method

public Rectangle getBounds(){
    Rectangle ret = new Rectangle((int)x,(int)y,width,height);
    return ret;
}

will create a new rectangle instance each time. This may create a lot of garbage, and eventually decrease the performance. But consider that, unfortunately, Rectangle is not immutable, so directly returning a single instance of the rectangle may not be applicable here.

\$\endgroup\$
  • \$\begingroup\$ I used this advice to go and rewrite my code from scratch : its much more efficient, and there is virtually no lag - thank you for the help! \$\endgroup\$ – Acuzik May 24 '16 at 13:00
1
\$\begingroup\$

Perhaps a small change but which could save you some time: as 'speed' of the Movement class is set to 20 and seems to never change. Can't you store an array with the different values comming from th processing of

    float xpos = (float) (speed * Math.sin(Math.toRadians(angle)));
    float ypos = (float) (speed * Math.cos(Math.toRadians(angle)));

that way you woulnd't have to recalculate the valeus used each time you can the methods.

Have a nice day !

\$\endgroup\$
  • \$\begingroup\$ It looks like the angle can change, so I don't think this is a good idea. \$\endgroup\$ – Simon Forsberg May 22 '16 at 18:23
  • \$\begingroup\$ I'm going to try this when I get a chance - Speed is a constant that wont change, so I could just have an array of 360 values - one for each different angle, and then call from the array. \$\endgroup\$ – Acuzik May 23 '16 at 2:25
  • \$\begingroup\$ To optimize that a bit, you could compute the value for the angle and then store it in a Map. That way the first time you compute the value for the angle you store it and retrieve it in futur (key would be the angle and value, the valeu computed) \$\endgroup\$ – damus4 May 23 '16 at 8:26
  • \$\begingroup\$ I heavily doubt that this would bring a noticable speedup. And considering the fact that CPUs are fast and memory is slow, it might even be slower to look up the data in an array than to compute the sin/cos on the fly. (And I'm pretty sure that it would be slower it if was looked up in a Map). \$\endgroup\$ – Marco13 May 23 '16 at 14:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.