Voxel-based chunk manager

I'm creating a voxel-based game and have been working on it for quite a while.

I've come to a point where I have to load and unload chunks on the fly. I've created a chunk manager class and it's working, but it's quite slow and I would like to improve its speed.

Header file:

#pragma once
#include 
#include 
#include 
#include 
#include "Chunk.h"
#include "Renderer.h"
#include "Noise.h"

#define WORLD_HEIGHT 4
#define SEED 5000
#define WATER_LEVEL 128
#define MAX_HEIGHT 256
#define BLOCK_BIOME_WEIGHT 1 //less is havier
#define BIOME_MIN 1.4
#define BIOME_MAX 0.7

using namespace std;

class Chunk;
class ChunkManager
{
public:
    ChunkManager(void);

   Chunk* getChunkAt(int x, int y, int z);
   void render(Renderer& renderer);
   void initialize(Camera& camera);
   void rebuild(double delta, Renderer& renderer);
   void update(double delta, Camera& camera, vector& frustumPlanes);

   float getWorldHeight(int x, int y);//TODO: Create new world class and add world related methods there

public:
    bool centerLoaded;

private:
    int renderDistance;
    int oldChunkX, oldChunkY, oldChunkZ;
    float oldCamYaw, oldCamPitch;

    unordered_map loadedChunks;
    vector rebuildChunks;
    vector removableChunks;
    vector renderChunks;
    vector visibleChunks;

    double timer, timer2, removeTimer, tickTimer;

    vector noises;

private:
    void addChunks(int x, int z);
    void cullAABB(vector& frustumPlanes);
    void loadChunks(vector& tempChunks, int& chunkX, int& chunkZ);
};

Class:

```
#include "ChunkManager.h"

ChunkManager::ChunkManager() {
renderDistance = 8;//[5, 6, 7, 8, 12, 13, 14, 15, 20, 22]
centerLoaded = false;
timer = 0;
timer2 = 0;
tickTimer = 0;
removeTimer = 0;
srand(SEED);

noises.push_back(new Noise(0, 0, 0, rand()));
noises.push_back(new Noise(1, 0, 0, rand()));
noises.push_back(new Noise(2, 0, 0, rand()));
}

float ChunkManager::getWorldHeight(int x, int z){
return (noises[0]->getNoise(x

Header file:

-
It's usually preferred to have your own headers before library headers. This can help avoid possible dependencies and also keep the headers more organized.

-
Do not use using namespace std in a header file. It can be okay to have it in an implementation file due to the locality (.cpp files do not get imported), but having it in a header file will force users to have the std namespace exposed, which can possibly break their code. Just leave it out and use std:: where appropriate.

-
Macro constants (or macros in general) are not very common in C++, and are primarily discouraged as there are better ways of doing things. Specifically, the const keyword should be used to make something constant and prevent any further changes to it.

const int WORLD_HEIGHT = 4;

Moreover, as these are in a header file, they will be global by default. While it is not such a bad thing since they cannot be modified, it would be safer to have them contained within a namespace or a class (only if it belongs in a class).

namespace vals
{
    const int WORLD_HEIGHT = 4;
    // ...
}

The constant can then be accessed via the scope operator (::):

int height = vals::WORLD_HEIGHT;

(Note: The name vals is only an example, so feel free to give a more relevant name. As a user may be using it frequently (unless using is used), try to make it short.)

If it does belong in a class, then make it a static const type (it must be static as it belongs to the class itself and not the individual objects). If it's an integer type, it can be initialized inside the class declaration as public, otherwise it'll have to be done in the implementation file.

The scope access concept still applies, except you'll use the class name:

int height = Class::WORLD_HEIGHT;

-
You can omit the ChunkManager constructor since you aren't already overwriting it. The compiler will provide a default one for you.

Also, void parameters are not needed in C++, unlike in C. The compiler will already know that such call takes no arguments.

-
It's a little confusing to have multiple public and private sections. Just group them into one.

Also, centerLoaded should be private as it is a data member. That should not be exposed to the public interface.

Implementation file:

-
Do not keep srand() anywhere but in main(). It should only be called once so that the seed will not be reset at each call, giving you the same random values. Keeping it in the constructor is still not sufficient as you'll be creating multiple objects thus calling the function multiple times. By having it in main() only, you know that it will only be called once in the entire program.

-
Instead of using a constructor, you can use an initializer list:

ChunkManager::ChunkManager()
    : renderDistance(8) //[5, 6, 7, 8, 12, 13, 14, 15, 20, 22]
    , centerLoaded(false)
    , timer(0)
    , timer2(0)
    , tickTimer(0)
    , removeTimer(0)
{
    noises.push_back(new Noise(0, 0, 0, rand()));
    noises.push_back(new Noise(1, 0, 0, rand()));
    noises.push_back(new Noise(2, 0, 0, rand()));
}

Moreover, you don't need to allocate with new (this isn't Java). Leaving them out will still work as noises will then be adding anonymous objects (those initialized without a name). All it needs are the constructed objects, which you already have.

StackExchange Code Review Q#63278, answer score: 11