• 9
• 9
• 9
• 10
• 10

Basic RPG Attempt

This topic is 399 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

Recommended Posts

Hey guys, first post here!

I've checked out some of the posts here for solutions to some of the problems I've had while learning c++. Now that I have a project running decently, I'd like to know how I am doing! :)

I'm currently going to school for software design, but for the most part my classes have been in Java. I'm trying to teach myself C++ because I want to go into game development.

I have my project posted publicly here on GitHub:

https://github.com/joeydubs/cpp-playground

Please let me know what you think, and thank's in advance for any feedback!

Joey Dubs

Share on other sites
Overall it looks pretty good. You do a good job separating code.

your play function in main.cpp is getting a little bloated.

You are passing by pointer in many places
void fight(Player *player) {
...
}

Where you could and probably should be passing by reference
void fight(Player &player) {
...
}

I try to only pass by pointer whenever NULL is a valid value to pass.

You item inheritance tree could be problematic. Suppose you eventually want to have an item that you can both equip and consume. Inheritance doesn't solve this problem well. An alternate solution to polymorphic items is applying polymorphism to individual concerns of an item.


// forward declaration
class Item;

class IUseBehavior {
public:
virtual void useOn(Item& item, Player& on) = 0;
}

class Item {
protected:
string name;
int levelReq;
IUseBehavior* useBehavior;
public:
Item(string name, IUseBehavior* useBehavior;, int levelReq);
void useOn(Player& player);
...
}

...
void Item::useOn(Player& player) {
if (useBehavior) {
useBehavior->useOn(*this, player);
}
}

You can then have your standard equip and consume behavior

void Equipment::useOn(Item& item, Player& on) {
// pass the item with the stats so the player can save the item for when it gets unequipped
on.equip(item, equipmentStats);
}

void Consumable::useOn(Item& item, Player& on) {
on.consume(consumeStats);
}


void EditbleEquipment::useOn(Item& item, Player& on) {

int pChoice;

cout << "Use Item --> Would you like to equip it or eat it?" << endl;
cout << "[1:Equip], [2:Eat]" << endl;
cin >> pChoice;

if (pChoice == 1) {
on.equip(item, equipmentStats);
} else if (pChoice == 2) {
on.consume(consumeStats);
} else {
cout << "Invalid choice" << endl;
}
}


You have some "using namespace" statements in header files. This is considered bad practice since it pollutes the global namespace for any files that directly or indirectly include it.

Share on other sites

Thank you so much for your response! I'm excited to know I'm doing pretty well!

Excellent point about the play() function, I was beginning to feel that way. I guess I should break each part of the switch statement into its own function. Maintain the switch, but just have it jump to a function for the execution. At which point, I should probably also create a main.h file, right?

Spring break is over, and my next class is starting, so this will probably be on hold while I jump back to Java for the remainder of the course (unless I have some free time). As it stands, it's a complete (albeit relatively simple) program, so I figured it would be a great time to get some feedback before continuing! :) I do definitely want to continue expanding it though. Might branch it on GitHub for the time being so that the base stay a complete project. Just need to come up with some more ideas. Wanted to keep it relatively high level to start before diving into the details too much (like expanding weaponry and more advanced combat -- thus the "some sword" item names, haha). Combat abilities is a big one I want to add later. :D

Interesting with the pointers, thank you for bringing that up. That part is still a little confusing to me, I'll need to look into it more closely. Honestly I thought I was passing by reference. With, for example, my function "void fight(Player *player) {...}". When I call the function I would say "fight(&player)". I thought this was passing the reference to the player to the function. So I would still call the function this way, but the function parameter should also contain an ampersand. How would I then use the reference inside the function?

Still trying to understand the last part. Gotta love those candy necklaces, am I right? So each item (ie. Equipment, Consumable, and EdibleEquipment) would still inherit the Item class. I hadn't thought about items that may need to be consumed, whether eaten or otherwise (thinking of RuneScape here -- jewelry with charges to, say, teleport). I'm just not sure where the IUseBehavior class comes in. Would it also have a child class for each item type to provide this definition? That was the only reason I went with inheritance, so each Item could be stored in the inventory, but each would have it's own set of stats and behaviors. I do like the useOn function, instead of the Player class handling the item type, the item gives an affect to the player, and if the item has multiple options like equip or eat, it presents those options before proceeding like you showed in the EdibleEquipment::useOn function. However, I kind of see this as the same thing as the IUseBehavior so I think I'm misunderstanding what you were trying to say with that portion.

Thank you again! :D :D

P.S. Also going to check out the global namespace part too. :) So in headers just spell it out, i.e. std::string?

Share on other sites

Interesting with the pointers, thank you for bringing that up. That part is still a little confusing to me, I'll need to look into it more closely. Honestly I thought I was passing by reference. With, for example, my function "void fight(Player *player) {...}". When I call the function I would say "fight(&player)". I thought this was passing the reference to the player to the function. So I would still call the function this way, but the function parameter should also contain an ampersand. How would I then use the reference inside the function?

The terminology can be confusing. You do indeed use the reference operator to get a pointer. To pass by reference you would remove the reference operator, or deference if you already have a pointer. If you look up passing by reference vs pointer in c++ you will get a lot more information.

I'm just not sure where the IUseBehavior class comes in.

Sorry the original wasn't super clear. Equipment, Consumable, and EdibleEquipment would extend IUseBehavior and you construct one and pass it into the item when you create it. Right now, the item only has one action. To use it on the player. If that were always the case then there would be no difference between extending IUseBehavoir and Item. However, you may decide you want to add a on acquire behavior or a on player death behavior. It is much easier to have an Item class you don't extend and simply compose with many different combinations behaviors than it is to extend Item for each combination of actions. Edited by HappyCoder

Share on other sites

If you look up passing by reference vs pointer in c++ you will get a lot more information.

Will do!

Equipment, Consumable, and EdibleEquipment would extend IUseBehavior and you construct one and pass it into the item when you create it.

So each item extends IUseBehavior and Item, and the Item class drops off the equipment type. So the header for an item class would be "class Equipment : Item, IUseBehavior {...}". Can the IUseBehavior::useOn() be defined in the instantiation for each item? or would it have to be defined in it's own file for each item.

Share on other sites

So each item extends IUseBehavior and Item, and the Item class drops off the equipment type. So the header for an item class would be "class Equipment : Item, IUseBehavior {...}". Can the IUseBehavior::useOn() be defined in the instantiation for each item? or would it have to be defined in it's own file for each item.

Here is some example code.

// forward declaration
class Item;

class IUseBehavior {
public:
virtual void useOn(Item& item, Player& on) = 0;
}

class Item {
public:
Item(string name, IUseBehavior* useBehavior, int levelReq);
}

...
class Equipment : public IUseBehavior {
public:
void useOn(Item& item, Player& on);
}

class Consumable : public IUseBehavior {
public:
void useOn(Item& item, Player& on);
}
...
// Then to create an item
Item potion("Sword", new Equipment(...), 1);
Item potion("Health Potion", new Consumable(...), 1);

I think I may be over engineering this specific example, but supposed we wanted to add an on aquire action for items.

class Item {
public:
Item(string name, IUseBehavior* useBehavior, IAquireBehavior* aquireBehavior, int levelReq);
}
...
class Curse : public IAquireBehavior {
...
}
...
// Then to create items
// Then to create an item
Item potion("Sword", new Equipment(...), null, 1);
Item potion("Health Potion", new Consumable(...), null, 1);
// applies a curse when this sword is added the players inventory
Item potion("Cursed Sword", new Equipment(...), new Curse(...), 1);

Edited by HappyCoder

Share on other sites

Okay, I think I am finally understanding! Everything falls under Item class because that class it just a name and behavior, and the behavior is defined by what kind of item you want it to be and how it acts.

Item potion("Sword", new Equipment(...), null, 1);

This is also an interesting point, passing null, which I believe you mention earlier in regards to pointers.  I used null from time to time in Java in this way when something didn't apply to the specific object. I was trying to apply this to my C++ program when I first started and what I was seeing on StackOverflow was that a specific null pointer didn't exist in C++, and was highly unpredictable when things were assigned null. Is this a feature of newer C++ versions (like 11, 14, or 17)? or something that gets defined by the programmer.