Dungeon Exercise - open for comments

[Tzarls]

Hi. I wanted to show you guys my version of the Dungeon Crawl exercise. It´s still not finished (like 80%), but all the main features work. I´d like to hear your opinions on what could be done better and why. This has been like a crash course for me, and I know some things could have been done better from the beginning, but hey, it´s been a great learning experience.

In my version:

- The number of traps and monsters is relative to the size of the dungeon.
- The monsters move, but won´t share the same spot.
- A monster that steps on a trap dies. The trap is then disabled.
- The player moves and can be killed, but there´s no way to win (haven´t created the treasure yet).
- Traps are visible.

To Do:

- Put only functions prototypes at the beginning and move definitions to be after the main function. (I know, I know...)
- replace "system("cls")" with something better
- Create the winning situation (treasure ans such).
- Monsters can carry and drop the treasure, changing it´s position.
- Traps shouldn´t be visible, but when a player is near a trap, he will get a warning.

Even though the messages in my game are in spanish, all comments in the code are in english. (Sorry, but I want my kids to play this game, and they only speak spanish!).

So, here´s my code, almost 300 lines...:S

//
// Template - provides a template to be used as the starting
// point
//
// the following include files define the majority of
// functions that any given program will need
#include <cstdio>
#include <cstdlib>
#include <iostream>
#include <time.h>
#include <string>

using namespace std;

class Monster
{
    public:
    Monster ()  // Class constructor
    {
        Alive = true;
        ID = 2; // 2 = monster
        XYPos = 0; // top left
        oldXYPos = 0;
    }

    bool Alive;
    int ID;
    int XYPos;
    int oldXYPos;

};

class Player
{
    public:
    Player()
    {
        Alive = true;
        ID = 4; // 4 = player
        XYPos = 0;
        oldXYPos = 0;
        Win = false;
    }

    bool Alive;
    int ID;
    int XYPos;
    int oldXYPos;
    bool Win;
};

void SetTraps (int* pCalabozo, int nAmount, int sizeX, int sizeY)
{
    // Set nAmount traps in the nCalabozo Array.
    // More than 1 trap can be in the same spot

    int x;
    int y;

    for (int i = 0; i<nAmount; i++)
    {
        x = rand() % sizeX;
        y = rand() % sizeY - 2;
        pCalabozo [x + (sizeX*y)] = 1; // 1 = trap
    }
}

void SetMonsters (int* pCalabozo, int nAmount, Monster* Mons, int sizeX)
{
    int x;
    int y;

    for (int i = 0; i<nAmount; i++) // Clear all old positions for living monsters
    {
        if (Mons[i].Alive == true) // and only for alive monsters
        {
            x = Mons[i].oldXYPos / 100;
            y = Mons[i].oldXYPos - (x * 100);
            pCalabozo [x + (sizeX*y)] = 0;
        }
    }

    for (int i = 0; i<nAmount; i++)
    {
        x = Mons[i].XYPos / 100;
        y = Mons[i].XYPos - (x * 100);

        if (pCalabozo [x + (sizeX*y)] == 2) // Already a monster there?
        {
            Mons[i].XYPos = Mons[i].oldXYPos; // Then don´t move
            x = Mons[i].XYPos / 100;
            y = Mons[i].XYPos - (x * 100);
        }

        if (pCalabozo [x + (sizeX*y)] == 1) // stepped on a trap?
        {
            Mons[i].ID = 3; // 3 = dead monster
            Mons[i].Alive = false;
        }

        if (pCalabozo [x + (sizeX*y)] != 2) // Avoid displaying dead monsters over alive ones
        pCalabozo [x + (sizeX*y)] = Mons[i].ID;

    }
}

void SetPlayer (int* pCalabozo, Player &fnHero, int sizeX)
{
    int x;
    int y;

    x = fnHero.oldXYPos / 100;
    y = fnHero.oldXYPos - (x * 100);

    if (pCalabozo [x + (sizeX*y)] == fnHero.ID) // If leaving a trail
        pCalabozo [x + (sizeX*y)] = 0; // clear trail

    x = fnHero.XYPos / 100;
    y = fnHero.XYPos - (x * 100);

    if ((pCalabozo [x + (sizeX*y)] == 1) or (pCalabozo [x + (sizeX*y)]) == 2) // Stepped on a trap or monster?

        {
            fnHero.ID = 5; // 5 = dead hero
            fnHero.Alive = false;
        }

        pCalabozo [x + (sizeX*y)] = fnHero.ID; // Put hero on new position
}

void PrintCalabozo (int* pCalabozo, int sizeX, int sizeY)
{
    char szTexturas [] = " OMXP++p+";

    for (int x=0; x<sizeX + 2; x++)
            {
                cout<<"-";
            }
    cout<<"\n";

    for (int y=0; y<sizeY; y++)
    {
        cout<<"|";
        for (int x=0; x<sizeX; x++)
        {
            cout<<szTexturas [pCalabozo[x + (sizeX*y)]];
        }
    cout<< "|\n";
    }

    for (int x=0; x<sizeX + 2; x++)
                {
                    cout<<"-";
                }
        cout<<"\n\n";
}

void MovePlayer (Player &fnHero, int sizeX, int sizeY)
{
    string direccion;
    int x;
    int y;

    fnHero.oldXYPos = fnHero.XYPos;

    x = fnHero.XYPos / 100;
    y = fnHero.XYPos - (x * 100);

    cout << "Indique direccion para mover al jugador:" << endl; // Get input from user
    cin >> direccion;

    if (direccion == "u") y--; // up
    if (direccion == "d") y++; // down
    if (direccion == "l") x--; // left
    if (direccion == "r") x++; // right

    x = max (0,x);
    x = min (sizeX-1,x);
    y = max (0,y);
    y = min (sizeY-1,y);

    fnHero.XYPos = (x*100) + y;
}

void MoveMonsters (Monster* Mons, int Amount, int sizeX, int sizeY)
{
    int x;
    int y;

    int pasos;
    int direccion;

    for (int i = 0; i<Amount; i++)
    {
        if (Mons[i].Alive == true)  // Move only living monsters
        {
            x = Mons[i].XYPos / 100;
            y = Mons[i].XYPos - (x * 100);

            pasos = (rand()%3)-1; // Random number (-1, 0, 1)
            direccion = rand()%2;

            switch (direccion) // Decide wether to move l-r or u-d
            {
                case (0) : x += pasos; break;
                case (1) : y += pasos; break;
            }

            x = max (0,x);
            x = min (sizeX-1,x);
            y = max (0,y);
            y = min (sizeY-1,y);

            Mons[i].oldXYPos = Mons[i].XYPos;
            Mons[i].XYPos = (x*100) + y;
        }

    }
}

int main(int nNumberofArgs, char* pszArgs[])
{
    srand ( time(NULL) ); // Rand seed

    int nSizeX;
    int nSizeY;
    int nNumberOfMonsters;

    // Get Calabozo size

    cout<<"Ingrese numero de celdas horizontales\n";
    cin>>nSizeX;
    cout<<"Ingrese numero de celdas verticales\n";
    cin>>nSizeY;

    system ("CLS"); // to be replaced with better method

    // Create Calabozo, fill with "0" , 0 = floor

    int nCalabozo [nSizeX * nSizeY];
    for (int i=0; i < (nSizeX * nSizeY); i++)
    {
        nCalabozo[i] = 0;
    }

    // Create Player and Player´s initial position.

    Player Hero;
    Hero.XYPos = (((nSizeX / 2) - 1) * 100) + (nSizeY - 1);

    // Create Monsters Objects

    nNumberOfMonsters = (nSizeX * nSizeY) / 20;
    Monster M[nNumberOfMonsters];

    for (int i = 0; i<nNumberOfMonsters; i++) // Set Monsters´ initial positions randomly
    {
        M[i].XYPos = ((rand() % nSizeX)*100) + (rand() % (nSizeY - 2));
    }

    // Put traps in dungeon
    SetTraps (nCalabozo, nNumberOfMonsters, nSizeX, nSizeY);

    // Put monsters in dungeon;
    SetMonsters (nCalabozo, nNumberOfMonsters, M, nSizeX);

    // Place Player in dungeon
    SetPlayer (nCalabozo, Hero, nSizeX);

    // Print dungeon
    PrintCalabozo (nCalabozo, nSizeX, nSizeY);

    // Begin Game

while ((Hero.Alive == true) and (Hero.Win == false))
{
    MovePlayer (Hero, nSizeX, nSizeY); // Ask for input to move Hero
    MoveMonsters (M, nNumberOfMonsters, nSizeX, nSizeY); // Move monsters randomly
    SetMonsters (nCalabozo, nNumberOfMonsters, M, nSizeX);
    SetPlayer (nCalabozo, Hero, nSizeX); // update player´s position & check for traps or monsters

    system ("CLS"); // to be replaced with  better method

    PrintCalabozo (nCalabozo, nSizeX, nSizeY); // print map
}

    // Move Monsters

    // Check if user won or not.

// wait until user is ready before terminating program
// to allow the user t;o see the program results
system("PAUSE");
return 0;
}

[Tzarls]

No one? C´mon guys, my code can´t be that bad!

[Mythios]

This doesn't compile. Has a bunch of errors - etc you have 'or' instead of '||' and 'and' instead of '&&'. What are using to compile this?

I changed a couple of things to get it to compile. For you're first dungeon crawl it's not to bad. Still would like to know what you're compiling with you to make some of this stuff work.

[Tzarls]

I´m using Code::Blocks and it compiles without problems, I´ve read about the usage of "||" and "&&", but when I tried using "or" and "and" those words were accepted so I used them instead of the other symbols. I´m more used to those because of my previous experience in other languages, but if the standard in C++ are "||" and "&&" symbols then I´ll adopt them no question asked.

So....no too bad for my first dungeon crawl??? It took me about three days to have it in its present form!!! - Just kidding. What do you think I could do to improve it(apart from the and/or thing)?

Thanks!

[Mythios]

For improvements now I'd try something like using 'new' and 'delete' for the creation of the map. And using a std::vector<> to store the monsters in etc. After that you could then try breaking apart your project into multiple .h and .cpp files.

[firedraco]

Yes, || and && are the standard. 'and' and 'or' do exist, but I have never really seen them used.

int nCalabozo [nSizeX * nSizeY];
This code is illegal in C++. You would need to use new.

Anyway, besides that, it seems fine. I would suggest making a "randint()" function so you don't have to do that rand() % stuff every time you want a random number. Also, I'm not really sure why a bunch of your variables have stuff like n and p attached to the front of them and some of them don't. Being consistent is usually helpful to the readers.

[Tzarls]

Thanks for your answers.

@firedraco:

Yes, those "n" and "p" n front of the variable names were an attempt to stick to the "tradition" of using n for "int", p for"pointer", etc., but when trying to make up new names for new variables sometimes I forgot to add those letters.

And about:

int nCalabozo [nSizeX * nSizeY];
This code is illegal in C++. You would need to use new.

Wow! And why in the world does my IDE let me use this kind of code? Gotta check the complie options, maybe there´s some kind of "lazy pogrammer" mode or something like that that I have to turn off in order to learn to use the language appropriately.

@Mythios: About using vectors to store monsters.... could you please elaborate a little more so I can better understand that approach?

Many thanks for you words people, they´re greatly welcomed.

[firedraco]

Wow! And why in the world does my IDE let me use this kind of code?

It is probably because it is legal C code, and most C++ compilers will just use a C compiler to compile certain parts. Like you said though, it is probably an option you can turn off.

I would help you with the vector thing, but it's kind of late here. XD

[Tzarls]

Don´t worry.... it´s late here too, so I´m better off too bed.

Thanks again, and when you feel like explaining the vector stuff for this particular case, don´t hesitate to write! :D

[Mythios]

Heres something small to get you started: (I'm going out to grab some tea ill be back in an hour if you need help)

#include <iostream>
#include <vector>
#include <ctime>

struct Monster
{
	int x, y;
};

void CreateMonsters(std::vector<Monster>& rMonsters, int numMonsters, int mapSizeX, int mapSizeY)
{
	for( int i = 0; i < numMonsters; i++ )
	{
		Monster temp;
		temp.x = rand() % mapSizeX;
		temp.y = rand() % mapSizeY;
		rMonsters.push_back(temp);
	}
}

void DrawMap(const std::vector<Monster>& rMonsters, char* myMap, int mapSizeX, int mapSizeY)
{
	// For this example as we are only drawing it once - set the map here.
	for( int y = 0; y < mapSizeX; y++ )
	{
		for( int x = 0; x < mapSizeX; x++ )
		{
			myMap[x + y * mapSizeX] = char(176); // char(176) to use the extended ascii chart
		}
	}

	// Set the monsters to the map
	for( unsigned int i = 0; i < rMonsters.size(); i++ )
	{
		myMap[rMonsters[i].x + rMonsters[i].y * mapSizeX] = 'M';
	}

	for( int y = 0; y < mapSizeX; y++ )
	{
		for( int x = 0; x < mapSizeX; x++ )
		{
			std::cout << myMap[x + y * mapSizeX];
		}
		std::cout << "\n";
	}
}

int main()
{
	int sizeX = 20;
	int sizeY = 20;
	char* map = new char[sizeX * sizeY];

	std::vector<Monster> monsters;
	const int numberOfMonsters = 10;
	
	CreateMonsters(monsters, numberOfMonsters, sizeX, sizeY);
	DrawMap(monsters, map, sizeX, sizeY);

	delete map;
	map = NULL;

	return 0;
}

[Tzarls]

Thanks for the code, I´ll analyze it and let you know if I have any trouble understanding it.

[Mythios]

No problem, let me know how you go.

[Duthomhas]

The or and and are standard, but you need to #include <ciso646>.

[Mythios]

Ah that answers a bit. How come he didn't need to include anything then. Does one of the headers he used include it? Or was it from using codeblocks?

[Duthomhas]

Some compilers do it for you automatically.

[Mythios]

I see, thanks for that Duoas :)

[Tzarls]

Thanks from me too, Duoas..... now I can continue using "and" and "or" without feelling guilty. :D

Just one more question (haven´t got time to analyze your code yet, Mythios, today was a busy day) - regarding compiler options, what does "-wall" is supposed to do? I used -pedantic and that actually showed me that:

int nCalabozo [nSizeX * nSizeY];

isn´t legal, which is a good thing I guess. But -wall (without -pedantic set) shows me no error, but according to code::blocks, it means "enable all errors" or something like that. Any ideas?

Thank you guys!

[Mythios]

Sorry, thats possibly a Duoas question. I'm not to sure what they do.

[Duthomhas]

-Wall means "show all warnings"
-pedantic means "be a stickler for what is a warning and what is not"
-ansi means "stick to the C++ standard -- no [GCC] extensions"

So when compiling, use -Wall -ansi -pedantic.

[chrisname]

When compiling a release version (as in, one that is not just a test) I tend to use
-Wall -Wextra -Werror -pedantic-errors -ansi
-Wextra shows extra warnings
-Werror makes all warnings into errors (I sometimes leave this one out when I need to do something evil to make my code work)
-pedantic-errors makes all of the warnings given by -pedantic into errors