Pitfalls of my current component implementation

So I sat down today with the intention of adding some sort of component to handle sprite animations. After deciding that I should probably make an AnimatedSprite component that inherited from Sprite I started to think about how to add it to my GameScreen class with the rest of my components.

For those who haven’t been keeping up with my blogs, I keep all my components together in memory in arrays – one array for each type of component. So now I needed to make a new array for my new AnimatedSprite component type.

I have GameObjects which then store references to the relevant component instances in an array. I have Player, Enemy and Bullet classes which all inherit from the GameObject class, and it was looking likely that I’ll need an Explosion class that does the same.

At the moment all my GameObjects are created in my GameScreen class. However this new Explosion needs to be created due to logic in the Player and Enemy classes. Should I then create some sort of ExplosionManager like I have the BulletManager so that I can have a finite number of Explosions that can be active at any one time?

The idea doesn’t appeal to me. I’d really like to be able to create GameObjects during the Update loop instead of having to make sure they’re all created during the GameScreen setup.

Surely there must be a better way? Perhaps I should try to emulate Unity even more and aim for some kind of “Prefab” GameObject that can be instantiated from anywhere within my code? How do I do that and maintain cache coherence? Do I reserve space for X number of each possible Component type and grab one when I need it? How do Unity do this? They have so many component types, as well as the “custom” script components.

I’m also not happy with how each of my GameObjects must be set up – there seem to be too many steps to adding a Component to a GameObject. This could easily lead to errors if you forget a step (which will happen). Perhaps some implementation of the Factory Pattern could help here?

Hmm, I have a feeling I’m heading towards implementing some actual Object Pooling system, with a pool for each component type as suggested by Cameron earlier this year. Then perhaps a Factory for each type of GameObject that encapsulates the setup a bit more? (I guess a Prefab is kind of a Factory) Someone, please stop me if I’m heading down the completely wrong path here.

If you’d like to have a look at the code for my current setup for a better idea of my dilemma then please have a look here. It is a bit messy because some of the old code from my pre-component version is still hanging around as reference for myself, but it hopefully should be readable.

(PS – I am reading more and more that OOP is hell and design patterns are the devil, but I cannot understand how to solve some of the problems I’m having without them. Do I need to go through some phase of better understanding how to solve these problems with OOP before I can properly understand how to solve them better with DOD or some other method?)

 

 

Advertisements

Fixing my collision errors + cleaning up

So I’ve made a promise to myself to write a post once a week. I actually started this 3 weeks ago then promptly went to Spain on holidays and missed two weeks. And I’m still not quite in the habit of doing it yet, so I seem to be leaving things to the last minute and writing my posts up on Sunday evening. In fact with my last post I managed to not finish before a house guest arrived, then frantically tried to get it done while the house guest was out one night seeing the sights of Amsterdam.

I’m happy that I do at least end up motivating myself to do some work once a week (in fact once I do get going I really enjoy myself) but I really need to work on not leaving it until the last minute. Because when I do that I rush and make mistakes.

Which brings me to today’s post. It seems my attempt at collisions was not quite perfect. For some reason the bullets were not being removed on collision, and in my search to find out why I discovered other minor bugs and ended up tidying up a bit.

Firstly I noticed that ships were being hurt by their own bullets as they were shooting them (because the bullet and shooting ship overlap for a brief time). To solve this I decided that the bullet should store a reference to the ship that shot it, and when checking collisions I should just discount any bullets that come from my own ship. I added a ShipObject pointer to bullet and the following code to the ShipObject Update function:

else if (collisionTag == BULLET_COLLIDER) {
    BulletObject* bullet = dynamic_cast<BulletObject*>(m_colliderComponent->m_pOtherCollider->m_pGameObject);
    if (bullet->m_pOwner != nullptr && bullet->m_pOwner != this) {
        m_healthComponent->TakeDamage(bullet->GetPower());
}

I also added a “power” variable to the bullets, so that I can change how much damage is done by enemy and player bullets. So in the above code the power of the bullet is obtained and passed to the health component.

I then noticed that some components of the game objects remain active even when the game object itself is deactivated, and my tests to see if game objects or components are active were just all over the place. So I added the following function to GameObject:

void GameObject::SetActive(bool a_bValue)
{
    if (m_bActive != a_bValue) {
        m_bActive = a_bValue;
        for (unsigned int i = 0; i < m_components.size(); i++) {
            m_components[i]->m_bActive = m_bActive;
        }
    }
}

This way the components are all set accordingly when a GameObject becomes active or inactive. I also made sure I was checking the active boolean in all Updates and other relevant functions. Also I added some code to the AddComponent class so that newly added components are set up with the active flag set the same as it is on the GameObject they are being added to at the time.

After all this my bullets were STILL not being removed after hitting a ship. I spent an embarrassingly long amount of time trying to work out why. In the end it turned out to be the first line of the IsCollidingWith function in the ColliderComponent. I was setting the pointer to the “other collider” (which I’m also using as a flag to determine whether there has been a collision) to be null.

What an idiotic thing to do – since I check every collider against every other collider right now then the other collider pointer is being overwritten with the very next collider I check. Really it only worked if the last collider I check against a particular collider results in a collision. To fix it I just reset all the “other collider” pointers before going into my collision checking loops.

The last thing I changed was to cache the references to the components I need in each class. Previously I was calling GetComponent multiple times per Update loop, which seemed like a not very good decision performance wise (since I’d have to check references to each component in the component list for that game object, and these are not stored very closely together in memory.

So in each of my derived GameObject classes I added a pointer to the Components it uses – then created a “setComponentPointers” private function, that is called at the end of the AddComponent function. The GetComponent call is done in here, like so:

void ShipObject::setComponentPointers()
{
    m_spriteComponent = dynamic_cast<SpriteComponent*>(GameObject::GetComponent(SPRITE));
    m_colliderComponent = dynamic_cast<ColliderComponent*>(GameObject::GetComponent(COLLIDER));
    m_healthComponent = dynamic_cast<HealthComponent*>(GameObject::GetComponent(HEALTH));
}

Next week I think I might take a look at trying to separate out the game logic code from the engine code. Also perhaps try to make it a bit easier to use the engine code, to foolproof it a bit somewhat. It would be great for the gameplay coder to not have to write the setComponentPointers function – right now they have to kind of add components in two places (using the AddComponent function as well as setting up the pointers in the GameObject class and making sure they are filled properly by the setComponentPointers function). It would be awesome if, after calling AddComponent, a gameplay coder never had to worry about doing anything else…

Feel free to have a closer look at my code on GitHub and let me know any suggestions you might have in the comments below.

Adding collisions to my little engine

Wow, it’s been a whole month since I last posted. Somehow it feels like so much longer. I’ve been so busy because my game was released on PS4 and Steam, and we’ve been so busy promoting it (which included a trip to Germany).

But, finally I’ve been able to do a bit of work for myself again. I’ve decided to add collisions to my project. They were already there in my previous iteration, but the existing implementation does not work at all with the component based system I’ve been working on. So, time for a refactor.

Before I started I moved the position, rotation and scale variables into the base GameObject class. There were way too many components that needed this information, and having to retrieve this from one component and pass it to the others was just getting ridiculous. The Unity game engine actually has a Transform component that stores this information and every GameObject includes a Transform component by default. I thought that was overkill for my little project, but I might change that again later on because it might be good to keep all the transform information together in memory.

Continuing with the theme of Unity being my main inspiration, I decided to create a ColliderComponent. Right now this is really an AABB collider, but eventually I think this will just be the base class for a series of colliders. I incorporated my existing AABB implementation into this component.

bool ColliderComponent::IsCollidingWith(ColliderComponent *a_pOtherCollider)
{
    m_pOtherCollider = nullptr;
    if (!m_pGameObject->IsActive()) {
        return false;
    }
    if (m_collisionTag == a_pOtherCollider->m_collisionTag) {
        return false;
    }
    if (max.x < a_pOtherCollider->min.x || min.x > a_pOtherCollider->max.x)
        return false;
    if (max.y < a_pOtherCollider->min.y || min.y > a_pOtherCollider->max.y)
        return false;
    //store the other collider
    m_pOtherCollider = a_pOtherCollider;
    return true;
}

There are a few things going on in this function. The lower half should look familiar – it’s basically just the existing AABBvAABB code from my previous implementation, just checking that the corners of the bounding box intersect.
Above that I’m checking to see if m_pGameObject is active. This is a variable I added to the base Component class – it’s a reference to the GameObject that the component is attached to. This made it easier to share information between Components (especially the transform information I mentioned earlier).

The CollisionComponent also has a collisionTag. I only have 3 tags right now: BULLET, PLAYER and ENEMY. No Bullets should collide with one another, and enemies should also pass through one another (for now), so I’m simply checking that nothing with the same tag should collide. Ideally there would be some sort of lookup table here to see what is allowed to collide with what, but that’s definitely a task for another time.

If there has been a collision I store the other collider, currently as a public variable so that I can check it in the gameplay scripts.

To call this function I created a PhysicsManager. The PhysicsManager stores references to all the CollisionComponents and has the following Update function:

void PhysicsManager::Update()
{
    //check all colliders against all other colliders. 
    //Is there a faster way to do this?? 
    //Obviously methods exist, should do some research
    for (unsigned int i = 0; i < m_colliders.size(); i++) {
        for (unsigned int j = 0; j < m_colliders.size(); j++) {
            //process collisions
            m_colliders[i]->IsCollidingWith(m_colliders[j]);
        }
    }
}

So basically it just runs through all the colliders and sets the “other” collider if there has been a collision. This collider is then checked in the GameObject gameplay code like so:

//check collisions here?
ColliderComponent* colliderComponent = 
    dynamic_cast<ColliderComponent*>(GameObject::GetComponent(COLLIDER));
if (colliderComponent->m_pOtherCollider != nullptr) {
    //ok, this means there has been a collision
    //check if collision is ship object or bullet
    if (dynamic_cast<ShipObject*>(
        colliderComponent->m_pOtherCollider->m_pGameObject)) {
        //inflict damage
        healthComponent->TakeDamage(10);
    }
    else if (dynamic_cast<BulletObject*>(
        colliderComponent->m_pOtherCollider->m_pGameObject)) {

        //inflict damage
        healthComponent->TakeDamage(10); 
    }
}

So here you can see I’m checking if the other collider is set and then checking to see what the game object is that we’ve collided with. In hindsight I should probably be checking the physics tag at this point, rather than doing a bit of a hacky dynamic cast. Also the damage is hardcoded in right now, but really I should be retrieving the weapon power of the bullet for the last line and applying that in place of “10”.

The last thing to note is that I am now calculating the bounding boxes of the objects every frame in the update function of the ColliderComponent instead of calculating this twice a frame and having to process the result in the main Update loop in the GameScreen.

Here is the ColliderComponent Update function:

void ColliderComponent::Update(const double a_dDeltaTime)
{
    min.x = max.x = m_pGameObject->m_position.x;
    min.y = max.y = m_pGameObject->m_position.y;

    glm::mat4 globalTransform = 
        glm::translate(glm::mat4(1), glm::vec3(m_pGameObject->m_position.x, 
                       m_pGameObject->m_position.y, 
                       m_pGameObject->m_position.z)) *
                       glm::rotate(glm::mat4(1), 
                       m_pGameObject->m_fRotationAngle, 
                       glm::vec3(0, 0, 1)) * 
                       glm::scale(glm::mat4(1), 
                       glm::vec3(m_pGameObject->m_fScale, 
                       m_pGameObject->m_fScale, 1));

    for (unsigned int i = 0; i < 4; ++i)
    {
        glm::vec4 temp = globalTransform * m_corners[i];
        if (temp.x < min.x)
            min.x = temp.x;
        if (temp.y < min.y)
            min.y = temp.y;
        if (temp.x > max.x)
            max.x = temp.x;
        if (temp.y > max.y)
            max.y = temp.y;
    }
}

So, here’s the recalculation of the bounding box each frame. Having to recalculate the globalTransform once per object per frame seems like a silly idea. Especially since the SpriteComponent is also making this calculation. It seems like the globalTransform should also be a part of the GameObject (or Transform) so it’s only calculated once per frame. And possibly it might only get recalculated if the position, rotation or scale have changed.

So that brings me to the end of my Collision refactor. I was happy to get this all up and running in only a few hours, but I definitely feel that I made some poor decisions, especially in terms of performance. I think my next job will be to fix up some of the issues I’ve already mentioned, as well as do a little research into a better way to process the collisions.

If you have any suggestions then please let me know in the comments, and as always, you can find my code on github.