I got to the statemanager chapter. A few constructive remarks:
1- You tend to pass very small data-items bij const ref: e.g. void f (const bool&). I don't see what's the merit in that. A bool or an int is probably even smaller than a pointer. Just pass these things by value and use const refs where a copy is (potentially) expensive.
2- You have a tendency of declaring empty destructors. Probably power of habit, but in C++11 this prevents the generation of move constructor and assignment. This will of course not be a problem with classes like eventmanager and statemanager because they will not be moved around anyway, but this is not a good habit.
3- In the draw and update functions of the statemanager, you traverse back and forth to find the last non-transparent state. In the past I used to be a C programmer, so traversing pointer structures is nothing new for me, but the C++ Committee gave us the algorithms library to be used. This code can be rewritten to be a lot more obvious:
#include <algorithm>
using State_elem = std::pair<StateType, BaseState*>>; // convenience
void StateManager::Draw()
{
if (m_states.empty() {return;}
if (m_states.back().second->IsTransparent() && m_states.size() > 1)
{
// find last non-transparent state. find_if normally finds the first element that satisfies its predicate,
// but if we feed it reverse iterators, it searches backwards.
// I use find_if_not to avoid confusing negative logic.
auto r_itr = std::find_if_not (m_states.rbegin(), m_states.rend(), [](State_elem e) {return e.second->isTransparent();});
// we gave find_if_not reverse iterators, it also gives us a reverse iterator. So we have to trick it:
for (; r_itr != m_states.rbegin(); --r_itr)
{
r_itr -> second -> Draw();
}
}
else
{
m_states.back().second->Draw();
}
}
IMHO this code is self-explanatory, except for the little trick with the reverse iterators.
It might be possible to just say
auto r_itr = std::find_if_not (m_states.rbegin(), m_states.rend(), [](auto e) {return e.second -> isTransparent();});
and let the compiler figure out the type of the argument of the predicate.
The algorithm library works brilliantly with lambdas. Every new C++ programmer should learn about that. This also goes for the HasState function: it can be simplified a lot by using find_if. Use find_if to find the state in m_states and use find to find it in m_toRemove. If the state is found in m_states and not found in m_toRemove, the result is true. Otherwise the result is false.
bool StateManager::HasState(const StateType type) // type is an int, just pass by value
{
auto state = std::find_if (m_states.begin(), m_states.end(), [type](State_elem e) {return e.first == type;});
auto remove = std::find(m_toRemove.begin(), m_toRemove.end(), type);
if (state != m_states.end() && remove == m_toRemove.end()) // found in m_states, not found in m_toRemove
{
return true;
}
else
{
return false;
}
}
4- The draw and update functions are almost identical. Duplicate code is Not Very Nice. The only differences are the functions calles (IsTransparent vs IsTranscendent, Draw vs Update). It should be possible to make a private helper function that does the searching and that accepts the functions that are used. I haven't worked this out yet. Probably it isn't worth the trouble in this simple case.
