Advertisement

libGDX game code review

Started by August 26, 2015 10:07 PM
1 comment, last by EagleGamer 9 years, 4 months ago

Hello there!

I finished my game a while ago, I edited a lot of it and tried to make it as neat as possible but I know that its just messy code. biggrin.png

The concept behind the was just like "Egg catcher", where you'd have a bag and try to collect as much eggs as possible.

When I first finished it and tested it both on PC and Android I usually had an array "indexOutOfBounds" exception.

It turned out it occured when there was a lot of collision detection happening between the "bag" and the "eggs".

Eventually I reorganized the code and the exception that made the game crash semi-disappeared as now it only happens when you

shift the bag left and right very vast. If you have any tip that could fix that or make me a more organized programmer I would really appreciate it.


import java.util.Iterator;

import com.badlogic.gdx.Gdx;
import com.badlogic.gdx.Input.Keys;
import com.badlogic.gdx.Screen;
import com.badlogic.gdx.audio.Music;
import com.badlogic.gdx.audio.Sound;
import com.badlogic.gdx.graphics.GL20;
import com.badlogic.gdx.graphics.OrthographicCamera;
import com.badlogic.gdx.graphics.Texture;
import com.badlogic.gdx.graphics.g2d.Sprite;
import com.badlogic.gdx.graphics.g2d.SpriteBatch;
import com.badlogic.gdx.math.MathUtils;
import com.badlogic.gdx.math.Rectangle;
import com.badlogic.gdx.math.Vector3;
import com.badlogic.gdx.utils.Array;
import com.badlogic.gdx.utils.TimeUtils;

public class GameScreen implements Screen {
	final Eidya game;

	public static Texture backgroundTexture;
	public static Sprite backgroundSprite;
	SpriteBatch spriteBatch;

	Texture fils, note, wallet, bug, bug2, lolipop, sixabal;
	Sound bling, eat;
	Music song;
	OrthographicCamera camera;
	Rectangle walletImage;
	Array<Rectangle> coins, money, bugger, bugger2, loli, six;
	float playertime = 60.0f;
	long lastDropTime;
	static int dropsGathered;

	public GameScreen(final Eidya gam) {
		game = gam;

		setDrops();
		backgroundTexture = new Texture("backgroundy.jpg");
		backgroundSprite = new Sprite(backgroundTexture);

		bug = new Texture(Gdx.files.internal("Bug.png"));
		bug2 = new Texture(Gdx.files.internal("bugger.png"));
		lolipop = new Texture(Gdx.files.internal("Candy.png"));
		sixabal = new Texture(Gdx.files.internal("Nut.png"));

		fils = new Texture(Gdx.files.internal("fils.png"));
		note = new Texture(Gdx.files.internal("dinar.png"));
		wallet = new Texture(Gdx.files.internal("Wallet.png"));

		eat = Gdx.audio.newSound(Gdx.files.internal("Eat.mp3"));
		bling = Gdx.audio.newSound(Gdx.files.internal("coin.wav"));
		song = Gdx.audio.newMusic(Gdx.files.internal("EidSong.mp3"));
		song.setLooping(true);

		camera = new OrthographicCamera();
		camera.setToOrtho(false, 1280, 720);
	 
		

		walletImage = new Rectangle();
		walletImage.x = 1280 / 2 - 64 / 2;
		walletImage.y = 20;
		walletImage.width = 150;
		walletImage.height = 100;

		money = new Array<Rectangle>();
		coins = new Array<Rectangle>();
		bugger = new Array<Rectangle>();
		bugger2 = new Array<Rectangle>();
		loli = new Array<Rectangle>();
		six = new Array<Rectangle>();
		spawn();

	}

	public static int getdrops() {
		return dropsGathered;
	}

	public void setDrops() {
		dropsGathered = 0;
	}

	private void spawn() {
		Rectangle coin = new Rectangle();
		coin.x = MathUtils.random(0, 1280 - 64);
		coin.y = 720;
		coin.width = 64;
		coin.height = 64;
		coins.add(coin);
		lastDropTime = TimeUtils.nanoTime();

		Rectangle bugs = new Rectangle();
		bugs.x = MathUtils.random(0, 1280 - 64);
		bugs.y = 720;
		bugs.width = 64;
		bugs.height = 64;
		bugger.add(bugs);

		Rectangle dinar = new Rectangle();
		dinar.x = MathUtils.random(0, 1280 - 64);
		dinar.y = 720;
		dinar.width = 64;
		dinar.height = 64;
		money.add(dinar);

		Rectangle buggy = new Rectangle();
		buggy.x = MathUtils.random(0, 1280 - 64);
		buggy.y = 720;
		buggy.width = 64;
		buggy.height = 64;
		bugger2.add(buggy);

		Rectangle sixo = new Rectangle();
		sixo.x = MathUtils.random(0, 1280 - 64);
		sixo.y = 720;
		sixo.width = 64;
		sixo.height = 100;
		six.add(sixo);

		Rectangle lolo = new Rectangle();
		lolo.x = MathUtils.random(0, 1280 - 64);
		lolo.y = 720;
		lolo.width = 64;
		lolo.height = 100;
		loli.add(lolo);

	}

	@Override
	public void render(float delta) {
		Gdx.gl.glClearColor(0, 0, 0.2f, 1);
		Gdx.gl.glClear(GL20.GL_COLOR_BUFFER_BIT);
		camera.update();

		game.batch.setProjectionMatrix(camera.combined);
		game.batch.begin();

		game.batch.draw(backgroundTexture, 1280, 720);
		game.batch.draw(backgroundSprite, 0, 0);

		playertime -= delta;

		game.font.setScale(3);
		game.font.draw(game.batch, "" + playertime, 915, 698);

		game.font.draw(game.batch, "" + dropsGathered, 25, 700);
		game.batch.draw(wallet, walletImage.x, walletImage.y);
		for (Rectangle coin : coins) {
			game.batch.draw(fils, coin.x, coin.y);
		}
		for (Rectangle lolo : loli) {
			game.batch.draw(lolipop, lolo.x, lolo.y);
		}
		for (Rectangle sixo : six) {
			game.batch.draw(sixabal, sixo.x, sixo.y);
		}
		for (Rectangle buggy : bugger2) {
			game.batch.draw(bug2, buggy.x, buggy.y);
		}

		for (Rectangle bugs : bugger) {
			game.batch.draw(bug, bugs.x, bugs.y);
		}

		for (Rectangle dinar : money) {
			game.batch.draw(note, dinar.x, dinar.y);
		}
		game.batch.end();

		if (playertime < 0) {
			game.setScreen(new WinState(game));
			dispose();
		}

		if (Gdx.input.isTouched()) {
			Vector3 touchPos = new Vector3();
			touchPos.set(Gdx.input.getX(), Gdx.input.getY(), 0);
			camera.unproject(touchPos);
			walletImage.x = touchPos.x - 64 / 2;
		}
		if (Gdx.input.isKeyPressed(Keys.LEFT))
			walletImage.x -= 200 * Gdx.graphics.getDeltaTime();
		if (Gdx.input.isKeyPressed(Keys.RIGHT))
			walletImage.x += 200 * Gdx.graphics.getDeltaTime();

		if (walletImage.x < 0)
			walletImage.x = 0;
		if (walletImage.x > 1280 - 64)
			walletImage.x = 1280 - 64;

		if (TimeUtils.nanoTime() - lastDropTime > 1000000000)
			spawn();

		Iterator<Rectangle> iter = coins.iterator();
		while (iter.hasNext()) {
			Rectangle coin = iter.next();
			coin.y -= 345 * Gdx.graphics.getDeltaTime();
			if (coin.y + 60 < 0)
				iter.remove();

			if (coin.overlaps(walletImage)) {
				iter.remove();

				if (MainMenuScreen.start == false)
					bling.stop();
				else
					bling.play();
				dropsGathered++;
			}
		}
		Iterator<Rectangle> itor = bugger.iterator();
		while (itor.hasNext()) {
			Rectangle bugs = itor.next();
			bugs.y -= 555 * Gdx.graphics.getDeltaTime();
			if (bugs.y + 60 < 0)
				itor.remove();

			if (bugs.overlaps(walletImage)) {
				itor.remove();

				if (MainMenuScreen.start == false)
					eat.stop();
				else
					eat.play();

				dropsGathered -= 3;

				if (dropsGathered < 4) {
					game.setScreen(new Lost(game));
					dispose();

				}
			}
		}

		Iterator<Rectangle> iterr = money.iterator();
		while (iterr.hasNext()) {
			Rectangle dinar = iterr.next();
			dinar.y -= 444 * Gdx.graphics.getDeltaTime();
			if (dinar.y + 60 < 0)
				iterr.remove();

			if (dinar.overlaps(walletImage)) {
				iterr.remove();

				if (MainMenuScreen.start == false)
					bling.stop();
				else
					bling.play();
				dropsGathered++;
			}

		}

		Iterator<Rectangle> itora = loli.iterator();
		while (itora.hasNext()) {
			Rectangle lolo = itora.next();
			lolo.y -= 400 * Gdx.graphics.getDeltaTime();
			if (lolo.y + 60 < 0)
				itora.remove();

			if (lolo.overlaps(walletImage)) {
				itora.remove();

				if (MainMenuScreen.start == false)
					bling.stop();
				else
					bling.play();
				dropsGathered++;
			}
		}

		Iterator<Rectangle> itorer = six.iterator();
		while (itorer.hasNext()) {
			Rectangle sixo = itorer.next();
			sixo.y -= 300 * Gdx.graphics.getDeltaTime();
			if (sixo.y + 60 < 0)
				itorer.remove();

			if (sixo.overlaps(walletImage)) {
				itorer.remove();

				if (MainMenuScreen.start == false)
					bling.stop();
				else
					bling.play();

				dropsGathered++;
			}
		}

		Iterator<Rectangle> itori = bugger2.iterator();
		while (itori.hasNext()) {
			Rectangle buggy = itori.next();
			buggy.y -= 432 * Gdx.graphics.getDeltaTime();
			if (buggy.y + 60 < 0)
				itori.remove();

			if (buggy.overlaps(walletImage)) {
				itori.remove();

				if (MainMenuScreen.start == false)
					eat.stop();
				else
					eat.play();

				dropsGathered -= 3;

				if (dropsGathered < 4) {
					game.setScreen(new Lost(game));
					dispose();

				}
			}
		}

	}

	@Override
	public void resize(int width, int height) {
		// TODO Auto-generated method stub

	}

	@Override
	public void show() {

		if (MainMenuScreen.start == false) {
			song.stop();
		} else
			song.play();
	}

	@Override
	public void hide() {
		// TODO Auto-generated method stub

	}

	@Override
	public void pause() {
		// TODO Auto-generated method stub

	}

	@Override
	public void resume() {
		// TODO Auto-generated method stub

	}

	@Override
	public void dispose() {

		song.dispose();
		bling.dispose();
		wallet.dispose();
		fils.dispose();
		note.dispose();
		eat.dispose();
		bug.dispose();
		bug2.dispose();
		lolipop.dispose();

	}

}

If you still don't know how the game should function here's the link for it:

https://play.google.com/store/apps/details?id=com.goldeneagle.eidya.android&hl=en

Sorry for the long read.Thank you for reading and have a nice day. smile.png

		Iterator<Rectangle> itor = bugger.iterator();
		while (itor.hasNext()) {
			Rectangle bugs = itor.next();
			bugs.y -= 555 * Gdx.graphics.getDeltaTime();
			if (bugs.y + 60 < 0)
				itor.remove();

			if (bugs.overlaps(walletImage)) {
				itor.remove();

				if (MainMenuScreen.start == false)
					eat.stop();
				else
					eat.play();

				dropsGathered -= 3;

				if (dropsGathered < 4) {
					game.setScreen(new Lost(game));
					dispose();

				}
			}
		}


This code exhibits a few code smells really.

First of all, there are a few blocks of code like this that all look roughly the same - there is clearly a common pattern of code being executed multiple times with only slight variations - some refactoring to common these out to a single method would be appropriate (with the variations passed in as parameter values).

Next of all I can see that this code is calling dispose(). In general you should aim to architect your code so that objects don't have to self-destruct because self-destructing objects are one of those classic sources of bugs. Ideally some higher-level object should be managing the lifetime of this object and calling the dispose() method when its lifetime should end - rather like how you are calling dispose() on the Music object (the Music object does not arbitrarily dispose of itself).

Finally, you IndexOutOfBoundsException is probably due to the fact that this code may invoke itor.remove() twice during just one iteration of the loop - which would end up attempting to remove 2 elements from the Array and will therefore fail with an IndexOutOfBoundsException if the Array contained fewer than 2 elements.
Notice that if both bugs.y + 60 < 0 and bugs.overlaps(walletImage) were true then you invoke itor,remove() once for each (twice in total). That's almost certainly a bug. Chances are you want to use e.g. an "else if" for the overlaps test.
Advertisement


		Iterator<Rectangle> itor = bugger.iterator();
		while (itor.hasNext()) {
			Rectangle bugs = itor.next();
			bugs.y -= 555 * Gdx.graphics.getDeltaTime();
			if (bugs.y + 60 < 0)
				itor.remove();

			if (bugs.overlaps(walletImage)) {
				itor.remove();

				if (MainMenuScreen.start == false)
					eat.stop();
				else
					eat.play();

				dropsGathered -= 3;

				if (dropsGathered < 4) {
					game.setScreen(new Lost(game));
					dispose();

				}
			}
		}

This code exhibits a few code smells really.

First of all, there are a few blocks of code like this that all look roughly the same - there is clearly a common pattern of code being executed multiple times with only slight variations - some refactoring to common these out to a single method would be appropriate (with the variations passed in as parameter values).

Next of all I can see that this code is calling dispose(). In general you should aim to architect your code so that objects don't have to self-destruct because self-destructing objects are one of those classic sources of bugs. Ideally some higher-level object should be managing the lifetime of this object and calling the dispose() method when its lifetime should end - rather like how you are calling dispose() on the Music object (the Music object does not arbitrarily dispose of itself).

Finally, you IndexOutOfBoundsException is probably due to the fact that this code may invoke itor.remove() twice during just one iteration of the loop - which would end up attempting to remove 2 elements from the Array and will therefore fail with an IndexOutOfBoundsException if the Array contained fewer than 2 elements.
Notice that if both bugs.y + 60 < 0 and bugs.overlaps(walletImage) were true then you invoke itor,remove() once for each (twice in total). That's almost certainly a bug. Chances are you want to use e.g. an "else if" for the overlaps test.

That was fascinating! Thanks for pointing those out. I actually thought disposing of objects would make the memory "lighter" so I never saw it from that point before. I'll try editting that if statement and reorganize the code soon.

Thanks again. happy.png

This topic is closed to new replies.

Advertisement