Jump to content
  • Advertisement
Sign in to follow this  
EagleGamer

libGDX game code review

This topic is 1056 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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

Edited by EagleGamer

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites

 

		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

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!