Polling versus events

In the update function you're polling the state. This means that every single frame, you're requesting state. In this case, it's about a few keys but if you have support for multiple players and controllers, this method is going to get computationally expensive. Responding on events is a little more complex, but worth considering for larger projects. Have a look at events in sdl. The same happens in the render function, where you set the material every single frame. Why not set the material when the material changes, so in the update? It is weird that a render function changes materials!

Structure

It may be that copy/pasting your code may have changed it a little, but at certain places you have more spaces than others, sometimes the { is on a newline, sometimes it is on the same, the destructor is in the headers while the constructor is not, ...

C versus C++

The style of CreateShader that returns a shader struct is a C-style construct. The C++ way of doing the same would be to give the Shader class a constructor that does the same as CreateShader.

Const references & move operators

If you pass a parameter by value it is copied over. So that means that every time someone calls SetMaterial, a copy of a Material object is created. The same for parameters in setters and constructors. To prevent useless copying, use const references or move operators where possible.

Remove dead/useless code

There is an end function, but it is never called nor implemented. So just remove it, and perhaps rename begin. (There is no use in creating functions because you think it may be useful in the future).

Virtual/Override

Mark functions you are going to overload as such. Mark the function in the base class virtual and add the override keyword in the derived class. This will help the compiler catch errors.

virtual void ShaderBatch::Begin();
virtual void LightBatch::Begin() override;

ShaderBatch

The naming seems to suggest a collection of meshes to be rendered with the same shader. Currently it is used to store the settings. For me personally, it would sound more logical if there is a list of items to render in that batch, and a call to a render function does all logic required to set-up and render all registered items. (e.g. store box1 (that's a bit of an odd name for a terrain, isn't it?) inside of a list inside of the ShaderBatch)

Polling versus events

In the update function you're polling the state. This means that every single frame, you're requesting state. In this case, it's about a few keys but if you have support for multiple players and controllers, this method is going to get computationally expensive. Responding on events is a little more complex, but worth considering for larger projects. Have a look at events in sdl. The same happens in the render function, where you set the material every single frame. Why not set the material when the material changes, so in the update? It is weird that a render function changes materials!

Structure

It may be that copy/pasting your code may have changed it a little, but at certain places you have more spaces than others, sometimes the { is on a newline, sometimes it is on the same, the destructor is in the headers while the constructor is not, ...

C versus C++

The style of CreateShader that returns a shader struct is a C-style construct. The C++ way of doing the same would be to give the Shader class a constructor that does the same as CreateShader.

Const references & move operators

If you pass a parameter by value it is copied over. So that means that every time someone calls SetMaterial, a copy of a Material object is created. The same for parameters in setters and constructors. To prevent useless copying, use const references or move operators where possible.

Remove dead/useless code

There is an end function, but it is never called nor implemented. So just remove it, and perhaps rename begin. (There is no use in creating functions because you think it may be useful in the future).

Virtual/Override

Mark functions you are going to overload as such. Mark the function in the base class virtual and add the override keyword in the derived class. This will help the compiler catch errors.

virtual void ShaderBatch::Begin();
virtual void LightBatch::Begin() override;

ShaderBatch

The naming seems to suggest a collection of meshes to be rendered with the same shader. Currently it is used to store the settings. For me personally, it would sound more logical if there is a list of items to render in that batch, and a call to a render function does all logic required to set-up and render all registered items. (e.g. store box1 (that's a bit of an odd name for a terrain, isn't it?) inside of a list inside of the ShaderBatch)

Polling versus events

In the update function you're polling the state. This means that every single frame, you're requesting state. In this case, it's about a few keys but if you have support for multiple players and controllers, this method is going to get computationally expensive. Responding on events is a little more complex, but worth considering for larger projects. Have a look at events in sdl. The same happens in the render function, where you set the material every single frame. Why not set the material when the material changes, so in the update? It is weird that a render function changes materials!

Structure

It may be that copy/pasting your code may have changed it a little, but at certain places you have more spaces than others, sometimes the { is on a newline, sometimes it is on the same, the destructor is in the headers while the constructor is not, ...

C versus C++

The style of CreateShader that returns a shader struct is a C-style construct. The C++ way of doing the same would be to give the Shader class a constructor that does the same as CreateShader.

Const references & move operators

If you pass a parameter by value it is copied over. So that means that every time someone calls SetMaterial, a copy of a Material object is created. The same for parameters in setters and constructors. To prevent useless copying, use const references or move operators where possible.

Remove dead/useless code

There is an end function, but it is never called nor implemented. So just remove it, and perhaps rename begin. (There is no use in creating functions because you think it may be useful in the future).

Virtual/Override

Mark functions you are going to overload as such. Mark the function in the base class virtual and add the override keyword in the derived class. This will help the compiler catch errors.

virtual void ShaderBatch::Begin();
virtual void LightBatch::Begin() override;

ShaderBatch

The naming seems to suggest a collection of meshes to be rendered with the same shader. Currently it is used to store the settings. For me personally, it would sound more logical if there is a list of items to render in that batch, and a call to a render function does all logic required to set-up and render all registered items. (e.g. store box1 (that's a bit of an odd name for a terrain, isn't it?) inside of a list inside of the ShaderBatch)

Polling versus events

In the update function you're polling the state. This means that every single frame, you're requesting state. In this case, it's about a few keys but if you have support for multiple players and controllers, this method is going to get computationally expensive. Responding on events is a little more complex, but worth considering for larger projects. Have a look at events in sdl. The same happens in the render function, where you set the material every single frame. Why not set the material when the material changes, so in the update? It is weird that a render function changes materials!

Structure

It may be that copy/pasting your code may have changed it a little, but at certain places you have more spaces than others, sometimes the { is on a newline, sometimes it is on the same, the destructor is in the headers while the constructor is not, ...

C versus C++

The style of CreateShader that returns a shader struct is a C-style construct. The C++ way of doing the same would be to give the Shader class a constructor that does the same as CreateShader.

Const references & move operators

If you pass a parameter by value it is copied over. So that means that every time someone calls SetMaterial, a copy of a Material object is created. The same for parameters in setters and constructors. To prevent useless copying, use const references or move operators where possible.

Remove dead/useless code

There is an end function, but it is never called nor implemented. So just remove it, and perhaps rename begin. (There is no use in creating functions because you think it may be useful in the future).

Virtual/Override

Mark functions you are going to overload as such. Mark the function in the base class virtual and add the override keyword in the derived class. This will help the compiler catch errors.

virtual void ShaderBatch::Begin();
virtual void LightBatch::Begin() override;

ShaderBatch

The naming seems to suggest a collection of meshes to be rendered with the same shader. Currently it is used to store the settings. For me personally, it would sound more logical if there is a list of items to render in that batch, and a call to a render function does all logic required to set-up and render all registered items. (e.g. store box1 (that's a bit of an odd name for a terrain, isn't it?) inside of a list inside of the ShaderBatch)