0
\$\begingroup\$

I'm building a small space shooter and want my player to have multiple weapons available. I'm using an array and a vector for the weapons (both initialized when the player is initialized). Both the weapon array and vector are members of the player.

    CWeapon     m_aWeapons[3];  // Array for unlockable weapons
    vector<CWeapon> m_vWeapons; // Vector for unlocked weapons

The weapons that can be unlocked are initialized in an array at the beginning.

// Set unlockable weapons
m_aWeapons[0].Init("LaserOrange.bmp", 0.2f, 400.0f, 20.0f);
m_aWeapons[1].Init("LaserBlue.bmp", 0.2f, 600.0f, 20.0f);
m_aWeapons[2].Init("LaserRed.bmp", 0.2f, 800.0f, 20.0f);

I save the weapons the player has available at current in the vector.

After each level the next weapon is unlocked, if possible, by pushing the next element of the array into the vector with the public method CPlayer::UnlockWeapon ().

// Unlock
//
// Task: Unlock next weapon
//
bool CPlayer::UnlockWeapon()
{
// Unlocked new weapon?
bool bUnlocked = false;

// Temp for size of weapon vector and array
int VectorSize = static_cast<int> (m_vWeapons.size ());
int ArraySize = sizeof(m_aWeapons) / sizeof(CWeapon);

// Not unlocked every weapon?
if (VectorSize < ArraySize)
{
    // Yes, then unlock next
    m_vWeapons.push_back(m_aWeapons[VectorSize]);

    // Unlocked new weapon
    bUnlocked = true;
}

return (bUnlocked);

} // UnlockWeapon

I save the index of the currently selected weapon in a variable and when the player shoots, I create a shot and initialize it with the speed and shotsprite of the vector element at the index of the currently selected weapon variable.

// ProcessShooting
//
// Aufgabe: Shoot weapon
//
void CPlayer::ProcessShooting ()
{
// Hasn't shot until now
m_bHasShot = false;

// Pressed "Space" and can shoot?
if (g_pFramework->KeyDown (SDL_SCANCODE_SPACE) && m_bShotLock == false)
{
    // Has shot
    m_bHasShot = true;

    // Create and initialize new shot
    CShot Shot;

    // Temp for ease
    CWeapon Temp = m_vWeapons.at(m_CurrentWeapon);

    Shot.Init(Temp.GetSprite(), m_fXPos, m_fYPos, Temp.GetSpeed ());

    // Insert shot into shotlist
    m_ShotList.push_back (Shot);

    // Shoot again, when space released
    m_bShotLock = true;

}

// Released space, so allow shooting
if (g_pFramework->KeyDown (SDL_SCANCODE_SPACE) == false)
    m_bShotLock = false;

} // ProcessShooting

Is that a strange approach or can I stick with it?

\$\endgroup\$
  • \$\begingroup\$ If you do not pop the weapon from m_aWeapons when unlocking, isn't the code unlocking the same weapon all the time? \$\endgroup\$ – realUser404 Jan 17 '17 at 9:19
  • \$\begingroup\$ @realUser404 you can't "pop" from an array. But that's not an issue: the code takes the next weapon each time via m_aWeapons[VectorSize]. \$\endgroup\$ – Quentin Jan 17 '17 at 9:20
  • \$\begingroup\$ Oh right I got confused between VectorSize and ArraySize. And guess why? Because calling a vector size VectorSize is as usefull as calling a string s_string :) \$\endgroup\$ – realUser404 Jan 17 '17 at 9:25
  • \$\begingroup\$ @realUser404 what name would you propose as a more appropriate name for VectorSize? \$\endgroup\$ – zelda hyrule Jan 17 '17 at 12:42
  • \$\begingroup\$ unlockableWeaponsSize ?? \$\endgroup\$ – realUser404 Jan 17 '17 at 12:49
0
\$\begingroup\$

It looks strange to me that you're copying the CWeapon objects everywhere: they look very much like the typical Type Object pattern, where an instance is immutable and represents an in-game type of weapon. In fact, I suggest renaming CWeapon to WeaponType and making it non-copiable. Then you pass around pointers to weapon types to use them, no copies needed.

You also apparently don't use constructors, which would be a good habit to get into: Init functions are neither needed nor practical in C++.

Refactoring your snippets, with some more inline tweaks, leads to:

Player.h

class Player {

    // ...

    // std::array rather than C-style array
    // Also consider std::vector, even if the size doesn't change,
    // for you then don't need to update it when adding weapons.
    // Also made the member static (no need to duplicate it in various players)
    // and const (no apparent need to tweak weapon types on-the-fly).
    static std::array<WeaponType, 3> const unlockableWeapons;

    // Clear naming makes comments redundant
    std::vector<WeaponType const*> m_unlockedWeapons;

};

Player.cpp

std::array<WeaponType, 3> const Player::unlockableWeapons{{
    {"LaserOrange.bmp", 0.2f, 400.0f, 20.0f},
    {"LaserBlue.bmp",   0.2f, 600.0f, 20.0f},
    {"LaserRed.bmp",    0.2f, 800.0f, 20.0f}
}};

bool Player::unlockWeapon() {

    auto const alreadyUnlocked = m_unlockedWeapons.size();

    // Simplify your control flow by returning early
    if(alreadyUnlocked == unlockableWeapons.size())
        // Everything is already unlocked
        return false;

    // Unlock the next weapon
    m_unlockedWeapons.push_back(&unlockableWeapons[alreadyUnlocked]);
    return true;
}

void Player::processShooting() {

    m_hasShot = false;

    if(!g_framework->keyDown(SDL_SCANCODE_SPACE)) {
        m_shotLock = false;
        return;
    }

    auto *const currentWeapon = m_unlockedWeapons[m_selectedWeapon];

    // Using emplace_back to construct the shot directly into the vector
    m_firedShots.emplace_back(
        currentWeapon.getSprite(),
        m_xPos, m_yPos,
        currentWeapon.getSpeed()
    );

    m_shotLock = true;
}

I've also tweaked the naming convention, which looked like it came from MFC. Usual (in my experience -- moderately broad but not a hard truth) C++ naming is:

  • PascalCase with no prefix for classes;
  • camelCase for functions;
  • camelCase with prefix for non-public member variables. m_ and _ are two popular prefixes.
  • No systems hungarian notation: C++ has types, no need to repeat them in the variable's name.

Yes, the standard library is a notable exception, which uses snake_case everywhere.

\$\endgroup\$
  • \$\begingroup\$ The naming convention is based on a very popular german games programming book, as well as some of the variable names you renamed, I didn't know it was so of topic. \$\endgroup\$ – zelda hyrule Jan 17 '17 at 12:56
  • \$\begingroup\$ I do not agree with the camelCase for functions; line. I believe PascalCase is most commonly used for functions. \$\endgroup\$ – realUser404 Jan 17 '17 at 15:15
  • \$\begingroup\$ @realUser404 I've personally only seen PascalCase functions in C# and ol' MFC. But as always with conventions, they are subjective -- which I'll emphasise in my answer. \$\endgroup\$ – Quentin Jan 17 '17 at 15:18
  • \$\begingroup\$ I found this document (I do not know what it is worth) : github.com/OpenXRay/xray-16/wiki/… \$\endgroup\$ – realUser404 Jan 17 '17 at 15:21
  • \$\begingroup\$ @realUser404 There is no consensus on code style in C++ (as opposed to, for example, Python). So every team has its own. What I've listed above are the most common choices I've encountered when reading C++ code (and that I use myself). \$\endgroup\$ – Quentin Jan 17 '17 at 15:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.