Aimbot for open-source FPS game Assault Cube

Question

I am actually quite new to C++, although I have previously programmed in Java. Do you see any ways I can improve readability, maintainability and performance, and make it more object-oriented?

/*
    Control + 0 = enable aimbot
*/

#include "stdafx.h"
#include <math.h>

const float pi = 3.14159265358979f;

/*
  * I am using int* as if it were the same thing as int**, int***, etc.
  * Another more elegant looking method is to use int as if it were int*, int**, etc
  * but it is more prone to undefined behaviour and I am more likely to get away with the first method.
  * See: http://stackoverflow.com/questions/16256158/dereferencing-a-double-level-pointer-results-in-different-behaviour-from-derefer
*/
int* getClosestPointer(int** basePointer, int offsets[], int levels) {
    for (int i = 0; i < levels; i++) {
        basePointer = (int**) (*basePointer + offsets[i] / sizeof(int));
    }
    return (int*) basePointer;
}

/*
  [Player base] = A
  [A + 34] = X
  [A + 38] = Y
  [A + 3C] = Z
  [A + 40] = Crosshair X
  [A + 44] = Crosshair Y
  [A + F4] = HP
  [A + 154] = Grenades
*/
int playerBaseXOffsets[] = { 0x34 };
int playerBaseYOffsets[] = { 0x38 };
int playerBaseZOffsets[] = { 0x3C };
int playerBaseCXOffsets[] = { 0x40 };
int playerBaseCYOffsets[] = { 0x44 };
int playerBaseHPOffsets[] = { 0xF4 };
int playerBaseAmmoOffsets[] = { 0x368, 0x14, 0 };
int playerBaseGrenadesOffsets[] = { 0x154 };

typedef struct Player {
    int** basePointer;
    float* xPointer;
    float* yPointer;
    float* zPointer;
    float* cXPointer;
    float* cYPointer;
    int* hpPointer;
    int* ammoPointer;
    int* grenadesPointer;

    Player() {
    }

    Player(int** basePtr) {
        basePointer = basePtr;
        xPointer = (float*) getClosestPointer(basePointer, playerBaseXOffsets, sizeof(playerBaseXOffsets) / sizeof(playerBaseXOffsets[0]));
        yPointer = (float*) getClosestPointer(basePointer, playerBaseYOffsets, sizeof(playerBaseYOffsets) / sizeof(playerBaseYOffsets[0]));
        zPointer = (float*) getClosestPointer(basePointer, playerBaseZOffsets, sizeof(playerBaseZOffsets) / sizeof(playerBaseZOffsets[0]));
        cXPointer = (float*) getClosestPointer(basePointer, playerBaseCXOffsets, sizeof(playerBaseCXOffsets) / sizeof(playerBaseCXOffsets[0]));
        cYPointer = (float*) getClosestPointer(basePointer, playerBaseCYOffsets, sizeof(playerBaseCYOffsets) / sizeof(playerBaseCYOffsets[0]));
        hpPointer = getClosestPointer(basePointer, playerBaseHPOffsets, sizeof(playerBaseHPOffsets) / sizeof(playerBaseHPOffsets[0]));
        ammoPointer = getClosestPointer(basePointer, playerBaseAmmoOffsets, sizeof(playerBaseAmmoOffsets) / sizeof(playerBaseAmmoOffsets[0]));
        grenadesPointer = getClosestPointer(basePointer, playerBaseGrenadesOffsets, sizeof(playerBaseGrenadesOffsets) / sizeof(playerBaseGrenadesOffsets[0]));
    }
} Player;

Player players[32] = { };

/* Pythagorean's theorem (flavour for 3D) */
float getDistanceBetween(Player one, Player two) {
    return sqrt(
        (*(one.xPointer)-*(two.xPointer))*(*(one.xPointer)-*(two.xPointer))
        + (*(one.yPointer)-*(two.yPointer))*(*(one.yPointer)-*(two.yPointer))
        + (*(one.zPointer)-*(two.zPointer))*(*(one.zPointer)-*(two.zPointer))
        );
}

int getNumberOfPlayers() {
    return *((int*) ((UINT) GetModuleHandleW(0) + 0xE4E10));
}

Player* getClosestTarget() {
    float smallestDistance;
    int index = -1;
    for (int i = 1; i < getNumberOfPlayers(); i++) {
        if (*(players[i].hpPointer) > 0) {
            float tempDistance = getDistanceBetween(players[0], players[i]);
            if (index == -1 || tempDistance < smallestDistance) {
                smallestDistance = tempDistance;
                index = i;
            }
        }
    }
    if (index == -1) {
        return NULL;
    } else {
        return &players[index];
    }
}

/* Gets the crosshair horizontal angle in degrees. */
float getCX(Player me, Player target) {
    float deltaX = *(target.xPointer) - *(me.xPointer);
    float deltaY = *(me.yPointer) - *(target.yPointer);
    if (*(target.xPointer) > *(me.xPointer) && *(target.yPointer) < *(me.yPointer)) {
        return atanf(deltaX/deltaY) * 180.0f / pi;
    } else if(*(target.xPointer) > *(me.xPointer) && *(target.yPointer) > *(me.yPointer)) {
        return atanf(deltaX/deltaY) * 180.0f / pi + 180.0f;
    } else if(*(target.xPointer) < *(me.xPointer) && *(target.yPointer) > *(me.yPointer)) {
        return atanf(deltaX/deltaY) * 180.0f / pi - 180.0f;
    } else {
        return atanf(deltaX/deltaY) * 180.0f / pi + 360.0f;
    }
}

/* Gets the crosshair vertical angle in degrees. */
float getCY(Player me, Player target) {
    float deltaZ = *(target.zPointer) - *(me.zPointer);
    float dist = getDistanceBetween(me, target);
    return asinf(deltaZ/dist) * 180.0f / pi;
}

int main() {
    bool aimbotEnabled = false;
    Player* closestTargetPointer = NULL;

    // [Base + DF73C] = Player 1 base
    players[0] = Player((int**) ((UINT) GetModuleHandleW(0) + 0xDF73C));
    int** extraPlayersBase = *((int***) ((UINT) GetModuleHandleW(0) + 0xE5F00));

    while (true) {
        // [Base + E5F00] = A
        // [A + 0,4,8...] = Player 2/3/4... base
        for (int i = 0; i < getNumberOfPlayers() - 1; i++) {
            players[i + 1] = Player(extraPlayersBase + i * 4 / sizeof(int*));
        }

        if (GetAsyncKeyState(VK_CONTROL) && GetAsyncKeyState('0')) {
            aimbotEnabled = !aimbotEnabled;
            Sleep(500);
        }

        if (aimbotEnabled) {
            closestTargetPointer = getClosestTarget();

            if (closestTargetPointer != NULL) {
                *(players[0].cXPointer) = getCX(players[0], *closestTargetPointer);
                *(players[0].cYPointer) = getCY(players[0], *closestTargetPointer);
            }
        }

        Sleep(10);
    }
}

DWORD WINAPI Main(LPVOID lpParam) {
    main();
    return S_OK;
}

BOOL APIENTRY DllMain(HMODULE hModule, DWORD  ul_reason_for_call, LPVOID lpReserved) {
    if (ul_reason_for_call == DLL_PROCESS_ATTACH) {
        DisableThreadLibraryCalls(hModule);
        CreateThread(NULL, 0, &Main, NULL, 0, NULL);
    }
    return TRUE;
}

Answer 1

Your C++ is very C-like. I would start by taking advantage of C++:

• You will almost always want to use std::vector instead of arrays.

  For example:

  std::vector<Player> players;
  players.reserve(32);
  

  This will create a vector large enough to hold 32 Players. If you ever need room for more, the vector will grow automatically when you add more players. By accessing elements using players.at(index), you will get range-checking.

  To get the number of elements in a vector, you can use players.size() instead of using the sizeof array division hack.

• Your struct typedef-ing is a common idiom in C, because in C you need to qualify struct identifiers with struct in order to use them: void foo(struct Bacon* bacon). In C++ you don't have to do this. I would therefore change your definition of Player to:

  class Player {
  public:
      // contents
  };
  

• The variables in Player should normally be encapsulated. This means they should be defined in the private section of the class (above public:). If you need access to the variables from other functions or classes, then you can provide accessor functions:

  class Player {
      int** basePointer;
  public:
      int** getBasePointer() const { return basePointer; }
  };
  

• If you really want to use your struct as POD, I recommend making the members const and making a copy every time your need to. This isn't a must, but reduces the chances of unintentional changes.

• In short, your class (a struct is a class) could and probably should be designed like a class, and not like POD.

• Like @Jamal points out, you can use enum for your offsets:

  enum Offsets { playerBaseXOffset = 0x34, playerBaseYOffset = 0x38 /* ... */ };
  

  If you are expecting to have more offsets of a kind later, use a std::vector instead:

  std::vector<std::size_t> playerBaseXOffsets{ 0x34 }; // C++11 syntax.
  

  Note the { and } in the last example. Using ( and ) instead compiles, but means something entirely different! (It will default-construct 0x34 = 52 objects.)

• I'm not sure if all your pointer fiddling is really necessary. If it's not, I recommend you avoid it. It is error prone, and some of the potential errors can be really hard to find. You risk forgetting to dereference a pointer if you're lucky; you get undefined behavior from violating the strict aliasing rule if you're not.

• Use symbolic, rather than literal, constants. For example:

  const std::size_t player_one_base_offset = 0xDF73C;
  players[0] = Player((int**) ((UINT) GetModuleHandleW(0) + player_one_base_offset));
  

  I'm sure you agree player_one_base_offset is much more readable and understandable than 0xDF73C.

• I would normally recommend C++-style static_cast<type>(object) instead of your C-style (type)object casting, but in your case I think it would just reduce readability even more without giving much benefit.

And finally, not directly code related: It's either Pythagoras' theorem, or the Pythagorean theorem.

Answer 2

• This is C++, not C, so use <cmath> instead of <math.h>.

• This would look and function better with more classes, especially since it's a game. For instance, Player can become a class instead of a struct so that it can be kept private. There should also be a Game class to keep a collection of Players and handle the gameplay. You will simply initiate the game in main() by creating a Game object. Note that main() should not have access to anything within the game beyond the ability to start it.

• I agree with @Lstor about making an std::vector of Players. This will also allow you to access each Player using the container's iterators. If you have C++11, you have two options, depending on whether or not your compiler supports range-based for-loops:

  If it does...

  for (auto const& player : players)
  {
      std::cout << player;
  }
  

  If it does not...

  for (auto iter = players.cbegin(); iter != players.cend(); ++iter)
  {
      std::cout << *iter;
  }
  

  In your game, you would be operating on this vector inside the Game class only. This is important because it shouldn't be exposed through the interface. You're free to define a display function (or overload operator<<), but you mustn't break encapsulation.

• The parameters in getCX() and getCY() should be const-refs since they're not modified:

  float getCX(Player const& me, Player const& target) {}
  float getCY(Player const& me, Player const& target) {}
  

• The playerBaseOffsets look like they should be const. I'd also prefer something more concise than single arrays, such as an enum. It could also be put into a namespace to avoid name collisions.

• The STL should especially be utilized to take advantage of smart pointers in place of raw pointers (the latter is more C-like). Beyond that, using the STL as much as possible will clean up your code considerably. As for pointers in general, use nullptr instead of NULL if you're using C++11.

• Regarding @Lstor's answer, include <cstddef> in order to use std::size_t.