My worst code design decision

First of all, calling blocks “Tiles” in the codebase is horribly confusing. Also, TileEntities being named similar to Entities doesn’t help at all.

But the worst piece of design is older than that, dating all the way back to the cave game test build. It’s a walking null pointer exception waiting to happen.

I keep a static Tile[256] list that maps a byte value to a Tile instance, which is used to do all the block logic. For example, to find out what material a block is (say, id 40) I’d go Tile.tiles[40].getMaterial(). Works fine.
But for some reason, I decided to make air have a null block, and be identified instead by its id, 0. This means that everywhere I want to do something with a Tile instance, I have to do something like “int id = level.getTileAt(x, y, z); if (id==0) return false; else return Tile.tiles[id].getMaterial()==Material.rock;” when checking for rock instead of just a simple “return level.getTileAt(x, y, z).getMaterial()==Material.rock;”

A couple of months ago, I made the effort to try to fix this, mainly by removing the ability to even ASK for a raw block type byte from the level, but it was really surprising how many places relied on those bytes. The renderer and level generators, primarely, act on the raw bytes a lot for speed. And additionally, there are a lot of places that rely on a Tile.tiles[getTileAt(x, y, z)]==null to check for air spaces.

The problem is just big and scary enough for my laziness to prevent me from fixing it, and it’s not dangerous enough to force me to fix it (either the game works, or it doesn’t. It can’t corrupt saves or anything like that).

And the Seecret Friday Update is either coming within a few hours, or some time after many more, due to me attending a birthday party! (Happy birthday, if you’re reading this. :D)

posted 13 years ago