This effective use of Hashmap?

I have never actually used Hashmaps, well I have but just normal 2 parameter Hashmaps with a key value and a variable (or object).

Currently I have a level that can consist of any xx amount of Doors, which are essentially Tiles. Therefore upon loading the level I just chuck literally every single Tile into the same array. However this seems to be bad, well not bad as that is required for rendering anyway. What is bad about it is, I need access to very specific tiles in order to do certain things.

An example would be, I have a sliding door. This may be a single door, or double doors together. Every door as an ID which will be used to compare with things such as a key to unlock it.

Now if the doors are together I group them and assign them the same ID, however this has me having to iterate over an array that has 1024 (this is JUST test level) tiles in it, find the doors, get the position of it, check for doors surrounding it, then set there ID’s to the same. This is horrible.

So I am thinking this instead but no idea if this is the way to do it:



/**
	 * All the doors currently loaded into the level, this is used to directly
	 * get a door and it's ID to match a key or way of unlocking it
	 */
	public static HashMap<Integer, HashMap<Vector2, Door>> doors = new HashMap<Integer, HashMap<Vector2, Door>>();


In my case this:



public static HashMap<ID Of Door, HashMap<Door Position, Door>> doors = new HashMap<ID Of Door, HashMap<Door Position, Door>>();


Will this give me any problems? I will have to constantly get the doors in order to control locks. Any other way of doing it? As it stands atm in order to group the doors I have like 5-6 nested loops which check top, bottom, left, right, created hinges, check ID’s…really it is very messy but it works but won’t be easy to manage in the future.

Ideas?


	List<Door> doors = new ArrayList<Door>();

	List<Door>[] doorsInTiles = new List<Door>[level.width * level.height];

	void add(Door door) {
		doors.add(door);

		doorsInTiles[door.xtile + door.ytile * level.width].add(door);
	}

	List<Door> getDoor(int xtile, int ytile) {
		doorsInTiles[xtile + ytile*level.width];
	}

Here is how I handle my entities instead of doors. It is simplified, but the idea is the same.
This isn’t my method, I took it from someone. Not going to tell from who though.

Would you say it is better to just store my doors in another array for doing logic on them rather than a Hashmap?

I read that Hashmaps are basically type safe ways of accessing specific values, that is kind of why I thought “yeah hashmaps!”.

Take a step back. How are you building your data? In your example you need keys to have IDs which match the door that they work on.

As far as I understand you are using a tile map? The easiest and fastest way to locate doors would then be by adding doors to the tiles themselves. Then you could quickly locate any door based on its tile coordinate.

A HashMap might be useful if you would have to look up doors of which you do not know the location (e.g. a “magic key” that would unlock all doors of a certain type). However, if this is a rarely occuring event or if you do not have a huge number of doors in each level it may not be worth the effort and duplication this results in.

As Roquen suggests, it may be a good idea to rethink your data structure if locating doors baed on their location an issue.

Edit: are you using a one-dimensional or two-dimensional array for your tiles?

My data is being read in from an image, I only use that to create the wall/door/floor tiles. All other things (spawners, pickups etc) will be handled by reading in a level config file.

I have 2 keys that I can use, I can either use the location of the door as the key or use the ID of the door itself, upon creation of a door it assigned an integer ID 1-infinite.

I can locate a door given a tile coordinate yes, however I want to be able to access these doors quite often. For instance some doors are open by default (decided by level config), locked, only opened by enemies, lock behind you, can only be open upon killing boss xx etc etc.

Iterating over 1000’s of tiles to find 1 or 2 seems very inefficient, when instead I can isolate the tiles I want direct access to, such as the door.

I am loading the tiles into a 2D integer array, which decides the tile type depending on an enum that compares a binary value of color (white = empty, black = wall, blue = door), it then creates the tile and sets the position of it at x y of array.

Once it has loaded all these tiles in, I have been continuing to use this 2D array to find adjacent tiles and do things, but this only really works inside the LevelLoader class.

Here is some code to give you an idea, this is my Level class. It currently holds the loader, the player etc etc (non level loading stuff is ommited)



public class Level implements Disposable {

	/** Level loader */
	public LevelLoader loader;

	/**
	 * All the tiles currently constructed in the level, used to locate and
	 * place various objects such as doors, spawner, pickups etc
	 */
	public static Array<Tile> tiles = new Array<Tile>();

	/**
	 * All the doors currently loaded into the level, this is used to directly
	 * get a door and it's ID to match a key or way of unlocking it
	 * <p>
	 * The Vector2 is the position of the door and is used as the primary key
	 * for searching and comparing.
	 * <p>
	 * The Integer is the DoorID number
	 * 
	 */
	public static HashMap<Integer, Door> doors = new HashMap<Integer, Door>();

	public Level(String file) throws LevelLoaderException {

		loader = new LevelLoader();
		loader.load();
		loader.generate();
	}


Here is my LevelLoader, it loads a level given an image file and later on a config file to match the image layout.

The init is fairly simple, all it does it takes the color value of the pixel and converts it to binary (which I thought was more readable to myself that some crazy -35346347 integer), then stores these values in a 2D array. I am sure I don’t have to explain how that works…

Like so:



/**
	 * Load an image from file and store it in memory, read the pixel colour
	 * information and store it in the array ready for generation
	 */
	public void load() {

		try {
			levelInfo = new Pixmap(
					Gdx.files.internal("data/levels/DoorSimulation.png"));
			width = levelInfo.getWidth();
			height = levelInfo.getHeight();
			/* Init the array with the width and height of the image file */
			cells = new String[width][height];
			for (int x = 0; x < levelInfo.getWidth(); x++) {
				for (int y = 0; y < levelInfo.getHeight(); y++) {
					String val = Integer.toBinaryString(levelInfo
							.getPixel(x, y));
					cells[x][y] = val;
				}
			}

		} catch (Exception e) {
			e.printStackTrace();
		} finally {
			levelInfo.dispose();
		}

	}


Then this code here, which generates the level given the color values and creates box2d stuff. Walls, Doors etc:



	/**
	 * Generate all the tiles in the level
	 * 
	 * @throws LevelLoaderException
	 */
	public void generate(Level level) throws LevelLoaderException {
		/* Door ID */
		int doorID = 1;
		for (int x = 0; x < width; x++) {
			for (int y = 0; y < height; y++) {
				TileType type = TileType.getType(cells[x][y]);
				float posX = x, posY = y;
				if (type == null)
					throw new LevelLoaderException(
							"Tile must not be null! Colour binary not recognized");
				switch (type) {
				case WALL:
					Level.tiles.add(new Wall(posX, posY));
					break;
				case DOOR:
					Door door = new Door(posX, posY, doorID);
					Level.tiles.add(door);
					Level.tiles.add(new Empty(posX, posY));
					doorID++;
					break;
				case PLAYER_SPAWN:
					Level.player = new Player(posX, posY);
					Level.tiles.add(new Empty(posX, posY));
					break;
				case EMPTY:
					Level.tiles.add(new Empty(posX, posY));
				default:
					break;
				}
			}
		}

		initDoors();

	}


Now all this above works perfectly fine, I am 100% happy with the way it loads in. Although you might be saying “just use Tiled editor”, I wanted to load it in my own way, to learn lol.

The next part is the ugly ass bit, although now that I think about it…it might not be possible to cut any shorter and at the moment it does what it needs to, gets doors, attaches hinges (box2d joints), and groups adjacent doors to have the same ID.

Pastebin

I have changed it a little, before I do anything else I iterate over the tiles array and get all the doors and put them in the hashmap, this just saves the method from iterating over unnecessary tiles. I know it looks terrible, but I don’t plan on refactoring it completely unless it becomes a problem, which at the moment it is because I would like to access doors without searching over tons of tiles pointlessly.

Putting them in the hasmap means if the player has a key with ID 1 and collides with door which has ID one, it is as simple as

doors.get(key.ID).setLocked(false);

At the moment the code is flexible enough for me to load in pretty much any level layout I want but when it comes to writing the level config files, I need a better way of accessing doors. This is probably just the one of many things I will want to access given an ID, no idea what I might need in the future so it would be ideal to have a good method that I can use.

Taking a step back and looking at the schematics to see how your objects/data link and work together isn’t a bad idea. You might want/have to reorganize some code.

Although there’s also value in just making it work. Reorganizing might just tangle it up further.

PS: I really dislike your avatar - it’s pretty gross. I’ve blocked it so I never have to see it again. Thought you should know.

Thanks for the clarification!

Here’s how I would approach this. I’d make the level a 2D array of tiles:

public Tile[][] tiles = new Tile[levelInfo.getWidth()][levelInfo.getHeight()];

Generating this level would be pretty much idential to your current approach. You could possibly leave the empty cells null if they are not rendered (i.e. do not exist).

Now attaching hinges to a door as you do in the pastebin part of the code would become a heck of a lot simpler and faster, for example:

	    // ...
		case DOOR:
            door = (Door) doors;
			if (!door.isHinged) {
				if (Level.tiles[door.x][door.y - 1].getType = TileType.WALL) {
					door.connectedHinges(tile.getBody(), new Vector2(-1, 0)).setHinged(true);
				}
              // else check other adjancent tiles...
			}
			break;
		// ...

Of course this would lead to border cases that you would need to handle (maybe just keep the tiles around the edge row empty).

Yeah I think I will write my level config files and see exactly what needs to be changed, at the moment there is pretty much no reason to change it but when I start to introduce level configs, going to have to.

Yeah, will probably change it

Omg why the hell am I not doing that basic if statement…seriously I need to be shot, I think I got so hung up with loops I forget to even try something so simple lol.