Why Minecraft is so badly written!

This is just a topic, to show you all how badly Minecraft was written, paying NO attention to memory usage whatsoever.

This is just some code snippets:

Why add a bottleneck for no reason?

public void onUpdate()
{
		this.onEntityUpdate();
}

Why so many bad and random method names??

public int getBlockTextureFromSideAndMetadata(int par1, int par2)

Noo! What is he doing?

public static final Block[] blocksList = new Block[4096];

WHAT THE FLYING FUCK???

/** How much light is subtracted for going through this block */
public static final int[] lightOpacity = new int[4096];


/** Array of booleans that tells if a block can grass */
public static final boolean[] canBlockGrass = new boolean[4096];

/** Amount of light emitted */
public static final int[] lightValue = new int[4096];

public static final boolean[] requiresSelfNotify = new boolean[4096];

And that is only a LITTLE bit of his coding failures…

you realized the variable and method names have been named by others (MCP) as all of these are obfuscated in the minecraft jar, he may be using entirely different thing.

wouldnt explain the bottleneck though

Or why he's defining arrays for single objects…

although I agree that minecraft is badly written it is mainly in other ways;

the bottleneck may be so that other classes are able to override/extend upon this method.

also it is considered good practice to hide variables in java, instead using get/set methods in order to get access to these variables, this method (although seemingly pointless at first) may be used to (as I said) more easily modify in order to do things "onUpdate" whilst still keeping onEntityUpdate() the same

onEntityUpdate is also most likely inherited which, once again, helps suggests that this method was created to be overridden.



also what is wrong with "blocksList" and "lightOpacity"
the arrays each tell the game a different thing, instead of keeping everything stored in "Block", presumable so that light updates etc. can be applied more easily, the arrays are being defined per-chunk for every block in each chunk.

mad, on 07 January 2013 - 07:51 AM said:

Why add a bottleneck for no reason?

public void onUpdate()
{
		this.onEntityUpdate();
}

Flexibility? onUpdate is what gets called from all entities, if updates it in all necessary aspects. This is what gets overridden because it's general. Then if all that the base class 'Entity' needs to do is updating the entity, it just calls onEntityUpdate(). It's not a big deal, it preserves modularity and, keep in mind, such things all get optimized out by modern compilers/VM's. It will most likely be reduced to only one jump instruction at a native level

mad, on 07 January 2013 - 07:51 AM said:

Why so many bad and random method names??

public int getBlockTextureFromSideAndMetadata(int par1, int par2)

What billysback said, I guess the MCP devs want to avoid confusion. :P/>

mad, on 07 January 2013 - 07:51 AM said:

Noo! What is he doing?

public static final Block[] blocksList = new Block[4096];

It's static final, it will probably get optimized at runtime. Also, 4kb of ram usage is practically nothing. It beats the complexity of lists, best fit arrays or other data structures.

mad, on 07 January 2013 - 07:51 AM said:

WHAT THE FLYING FUCK???

/** How much light is subtracted for going through this block */
public static final int[] lightOpacity = new int[4096];


/** Array of booleans that tells if a block can grass */
public static final boolean[] canBlockGrass = new boolean[4096];

/** Amount of light emitted */
public static final int[] lightValue = new int[4096];

public static final boolean[] requiresSelfNotify = new boolean[4096];

Same as above.

It's not like I don't agree with the fact that Minecraft isn't efficient. You're just picking the totally wrong examples. These all have minor to none effect at runtime.

There's no words to describe how many stupid statements are in this thread.

Cloudy, on 07 January 2013 - 01:54 PM said:

There's no words to describe how many stupid statements are in this thread.

<_</> Do us a favor and point them out. :P/> I feel dumb already.

Maybe when I get on my computer. You're 100% correct in your statements though.

mad, on 07 January 2013 - 07:51 AM said:

Noo! What is he doing?

public static final Block[] blocksList = new Block[4096];

He's… Declaring an array?

mad, on 07 January 2013 - 07:51 AM said:

This is just a topic, to show you all how badly Minecraft was written, paying NO attention to memory usage whatsoever.

This is just some code snippets:

Why add a bottleneck for no reason?

public void onUpdate()
{
		this.onEntityUpdate();
}

I too, can't see a reason for this - however just because you can't see one, doesn't mean there is none. Could be a throwback from old code, for example.

mad, on 07 January 2013 - 07:51 AM said:

Why so many bad and random method names??

public int getBlockTextureFromSideAndMetadata(int par1, int par2)

Epic fail much? Look what MCP actually does, and then go sulk in a corner.

mad, on 07 January 2013 - 07:51 AM said:

Noo! What is he doing?

public static final Block[] blocksList = new Block[4096];

He's declaring an array to hold all the blocks in the game. Why is that a bad thing? Sure, there's other methods of holding items - but sometimes you don't need anything more complex than an array. And by "he" I assume you mean Mojang - Notch hasn't been a coder on Minecraft for a long time.

mad, on 07 January 2013 - 07:51 AM said:

WHAT THE FLYING FUCK???

/** How much light is subtracted for going through this block */
public static final int[] lightOpacity = new int[4096];


/** Array of booleans that tells if a block can grass */
public static final boolean[] canBlockGrass = new boolean[4096];

/** Amount of light emitted */
public static final int[] lightValue = new int[4096];

public static final boolean[] requiresSelfNotify = new boolean[4096];

Again, I don't see the problem there. Care to show why you think it is a bad thing?

billysback, on 07 January 2013 - 10:44 AM said:

although I agree that minecraft is badly written it is mainly in other ways;
his organization is appalling (no packets)

You do realise the obfuscation strips out packages (which I assume is what you meant) right?

billysback, on 07 January 2013 - 10:44 AM said:

the way in which the world generates is very inefficient (he could have tried and made it generate the world in one loop of the blocks)

I'm not sure what you mean by this?

billysback, on 07 January 2013 - 10:44 AM said:

he could have taken further consideration in to the possibility of multi-player support before he got too far in to the project

True - but great steps have now been made to make it truly a multiplayer game - even singleplayer is multiplayer!

Cloudy, on 09 January 2013 - 05:59 AM said:

You do realise the obfuscation strips out packages (which I assume is what you meant) right?

I didn't realize* that actually :P/>
oh well, I do now.

Cloudy, on 09 January 2013 - 05:59 AM said:

I'm not sure what you mean by this?

which part, his generation code is kind of slow and one way he could have optimized it is by making it generate in one loop of the blocks, instead of looping multiple times adding different things in each loop. Unless that doesn't happen anymore, I think the generation code has been improved a few times.

Cloudy, on 09 January 2013 - 05:59 AM said:

mad, on 07 January 2013 - 07:51 AM said:

Noo! What is he doing?

public static final Block[] blocksList = new Block[4096];

He's declaring an array to hold all the blocks in the game. Why is that a bad thing? Sure, there's other methods of holding items - but sometimes you don't need anything more complex than an array. And by "he" I assume you mean Mojang - Notch hasn't been a coder on Minecraft for a long time.

mad, on 07 January 2013 - 07:51 AM said:

WHAT THE FLYING FUCK???

/** How much light is subtracted for going through this block */
public static final int[] lightOpacity = new int[4096];


/** Array of booleans that tells if a block can grass */
public static final boolean[] canBlockGrass = new boolean[4096];

/** Amount of light emitted */
public static final int[] lightValue = new int[4096];

public static final boolean[] requiresSelfNotify = new boolean[4096];

Again, I don't see the problem there. Care to show why you think it is a bad thing?

Block[4096];

There it is: "4096". Why allocate so much memory, if there are 300 blocks in the game, or something like that?
And he is also declaring arrays for single objects, like

int lightValue = new int[4096];

mad, on 09 January 2013 - 07:12 AM said:

There it is: "4096". Why allocate so much memory, if there are 300 blocks in the game, or something like that?
And he is also declaring arrays for single objects, like

int lightValue = new int[4096];

if 'he' didn't do 4096 and only did 300 we wouldn't have mods.
the light values are STATIC meaning its not a single object. its the class. that array is for the class not the object.

billysback, on 09 January 2013 - 06:42 AM said:

Cloudy, on 09 January 2013 - 05:59 AM said:

I'm not sure what you mean by this?

which part, his generation code is kind of slow and one way he could have optimized it is by making it generate in one loop of the blocks, instead of looping multiple times adding different things in each loop. Unless that doesn't happen anymore, I think the generation code has been improved a few times.

Do you even know about cache misses and locality of reference? Running the same compact block of code across a large amount of data is much more efficient than running a complex block of code across a small amount of data (e.g. chunk). And also, modular code is much easier to maintain than a gigantic loop trying to do everything at once.

mad, on 09 January 2013 - 07:12 AM said:

Block[4096];

There it is: "4096". Why allocate so much memory, if there are 300 blocks in the game, or something like that?
And he is also declaring arrays for single objects, like

int lightValue = new int[4096];

As I stated before, it gets optimized out. It's declared as static, so it's easy for the JVM to limit memory allocation to the required minimum.

Orwell, on 09 January 2013 - 07:44 AM said:

billysback, on 09 January 2013 - 06:42 AM said:

Cloudy, on 09 January 2013 - 05:59 AM said:

I'm not sure what you mean by this?

which part, his generation code is kind of slow and one way he could have optimized it is by making it generate in one loop of the blocks, instead of looping multiple times adding different things in each loop. Unless that doesn't happen anymore, I think the generation code has been improved a few times.

Do you even know about cache misses and locality of reference? Running the same compact block of code across a large amount of data is much more efficient than running a complex block of code across a small amount of data (e.g. chunk). And also, modular code is much easier to maintain than a gigantic loop trying to do everything at once.

I have not idea what the hell "cache missed and locality of reference" is, people seem to forget that I (trust me, I hate using my age as an "excuse") am most likely much younger than them; explain?
either way, surly deciding what you want to do with each block then looping to generate these decisions is more efficient?

billysback, on 09 January 2013 - 07:50 AM said:

Orwell, on 09 January 2013 - 07:44 AM said:

* snip *

I have not idea what the hell "cache missed and locality of reference" is, people seem to forget that I (trust me, I hate using my age as an "excuse") am most likely much younger than them; explain?
either way, surly deciding what you want to do with each block then looping to generate these decisions is more efficient?

Well, no, it wouldn't be more efficient because you'd need much more complex computations. Overriding a block 4 times is still more efficient when you use solid small computations in a loop to generate this block.

I'll try to explain it as simple as possible. In order to do calculations on data, a processor needs to load the data from RAM into its registers first. But loading data from RAM is very slow compared to the processor. In order to avoid this, there's a cache in between the CPU and the RAM. This cache is much smaller, much more expensive and has a faster connection to the CPU which it doesn't need to share with other IO devices. So the cache is much more faster than RAM. When the CPU needs data at a certain address, it will ask the cache. If the cache doesn't has this data, this is called a cache miss, it will load it from RAM into the cache, this takes longer. Data that's used more often will stay in the cache for a longer period. Now, locality of reference means that some data will be accessed more often than others; for example in a loop. Local variables in a small loop block will be accessed a lot more frequently than others, thus it will stay in the cache for most of the time. That way, these regions in memory will be accessed a lot faster than other data.

By having complex loops, you'd push the data out of the cache all the time because you need new data for other calculations in the same loop. So you'd get a lot of cache misses and accessing data would be slower. Having small solid loops will keep most of the needed data in the cache and speed up calculations significantly.

mad, on 07 January 2013 - 07:51 AM said:

public static final boolean[] canBlockGrass = new boolean[4096];

Secret leftover code from Cave Game, when lighted blocks became grass?

Simple explanation: when you're copying out lines, "I will not throw baseballs at the alligators", you could go line by line, across the page and then down when you've finished a row, or you could go word by word, down the page and then across when you've finished a column.
If you go line-by-line, you have to remember the entire line every time. If you go word-by-word, you only have to remember the current word you're doing.
Same with world generation in Minecraft.
Less to remember = faster = better

(Well, it doesn't work quite like that, but it makes sense, so…)

Dlcruz129, on 09 January 2013 - 05:09 PM said:

mad, on 07 January 2013 - 07:51 AM said:

public static final boolean[] canBlockGrass = new boolean[4096];

Secret leftover code from Cave Game, when lighted blocks became grass?

Or… For Dirt, which can become grass?

Nah from memory its another piece of code that does that.

D3matt, on 09 January 2013 - 09:24 PM said:

Dlcruz129, on 09 January 2013 - 05:09 PM said:

mad, on 07 January 2013 - 07:51 AM said:

public static final boolean[] canBlockGrass = new boolean[4096];

Secret leftover code from Cave Game, when lighted blocks became grass?

Or… For Dirt, which can become grass?

Oh derp. That would make sense. :P/>