Problem With Virtual Function & Abstract Class

[devious dm]

It is now, yes

[ne555]

> The code compiles and executes, but crashes when it makes a call to "atk()",
> so I can only guess that the virtual function isn't being overridden.
your code compiles, the function is being overridden.
you may be trying to access an invalid pointer when calling the function.

also, atk() is a terrible name, just write attack()
an you have Monster::atk(Character*, Monster*, Weapon*, float) ¿why are you asking for another monster instead of using the caller object?
hydra.attack(player) instead of hydra.attack(player, hydra);

    Monster* pMonster1 = &Goblin(); //error: taking address of rvalue
    Weapon mw = Unarmed(); Weapon* MonWpn = &mw; //convoluted

    // Character and Character Weapon Variables
    Character* Fighter = new Character(); //ugly, error prone and inefficient 

[Thomas1965]

What did you do to get a crash?
I did the following and it worked:

Would you like to load a previous game, or start a new game? (l/n)     n
What kind of Fighter do you want to be?

1: Slow Dealer     2: Glass Dealer     3: Slow Tank
4: Glass Dancer    5: Light Tank       6: Light Dancer
7: Balanced

Enter the number for the Fighter of your choice:     1
What is your fighter's name?
NN
Hi NN!

Character Info:
Name: NN         Type: Slow Dealer
Level: 1         To Next Level: 500XP
Max HP: 22       Current HP: 22          Gold: 5
Strength: 12     Dexterity: 8    Constitution: 10

--- Equipped ---
Weapon: Unarmed
Damage Bonus: 0

What would you like to do?
1: Fight a Monster   2: Sleep        3: Shop     4: Swap Weapons
5: View Stats        6: Save Game    7: Quit Playing

1
You are fighting a goblin!
Your turn!

What action would you like to take?
1: Attack     2: Defend     3: Ready     4: Swap Weapons       3

You prepared to counter!
goblin's turn!
You parried the goblin's attack!
You scored a hit!
You dealt 7 damage to the goblin. (6)
Your turn!
What action would you like to take?
1: Attack     2: Defend     3: Ready     4: Swap Weapons       1
You missed!
goblin's turn!
This crap still isn't working.
Your turn!
What action would you like to take?
1: Attack     2: Defend     3: Ready     4: Swap Weapons       1
You scored a hit!
You dealt 5 damage to the goblin. (1)
goblin's turn!
This crap still isn't working.
Your turn!
What action would you like to take?
1: Attack     2: Defend     3: Ready     4: Swap Weapons       1
You scored a hit!
You dealt 1 damage to the goblin. (0)
You killed the goblin!!
You found 3 gold.

[devious dm]

I put some output text in the parent function to see what was happening, and no matter which monster it was, it was always outputting the text in the parent function rather than overriding with the derived class's function.

I know it's not great for "Monster::atk(..)" to call for a Monster pointer, that's just how the program evolved. It was easiest to do it that way because I had already set up "monAtk()", which isn't a member function and calls for the pointer, and replaced it with calls to "pMonster1->atk(...)" so that i could call "monAtk()" in different ways for different monsters. It's messy, but this is my first personal project and I didn't do any pseudocode or other forward planning, I'm just writing it as I get ideas. I know that's not a good way to do it, but I'm new to C++, and this will be a learning experience.

If you'd like to look through the code to see how everything is set up, there's a link to a DropBox in a previous post ^^^ that should have the most recent code. If there's a better way to do this than to use virtual, I'd be happy to try it.

[ne555]

your dropbox link still has the faulty code Monster *pMonster1 = &Goblin();
upload the updated code

[ne555]

a correction on a previous post
Weapon mw = Unarmed(); Weapon* MonWpn = &mw;
is just plain wrong, check out object slicing

the proper convoluted way would be Unarmed mw; Weapon *monster_weapon = &mw;

[devious dm]

@Thomas1965 To get the crash, convert "Monster::atk()" to a pure virtual in "classList.h" and comment out the definition in "classList.cpp". In the output you posted, you can see where the error is where it outputs on the goblin's turn the text "this crap still isn't working". That text is in the base class member function definition, which shows that the "Goblin::atk()" overrider isn't being called.

@ne555 That is the current code. I do it that way because if "Monster::atk()" is pure virtual, I can't instantiate a "Monster" object to overwrite with other info, but i can still initialize a "Monster" pointer and initialize the object with whichever monster. So when I initialize the pointer, i give it a goblin just so the pointer is actually pointing to valid data rather than empty/random memory. I've watched this one in the "autos" and "locals" windows in Visual Studio just to make sure it works, and as far as I can see, the data members are properly overwritten when assigned a different monster object. But I don't have any way of knowing if that also works for the member functions. What is it about that code that's faulty, and how can it be improved? EDIT: I didn't see your edit until now. Frankly, I don't exactly know what I'm doing, I don't know the cleanest way to initialize the pointers & objects. I'm working on just a couple months (at best) of teaching myself to code, and writing this program is a little over my head, but I'm doing that on purpose to teach myself. So I'm not sure why the code you picked out is bad (though I can't disagree with you), so an explanation would be fantastic.

[ne555]

Consider this

Monster* create_goblin(){
   Goblin g;
   return &g;
} //here `g' dies

Monster *m = create_goblin();
m->atk(/**/); //`g' no longer exists, `m' is invalid and it's an error to dereference it 

here the code is returning the memory address of a variable local to the function
the thing is, once the function ends that variable doesn't exist ¿so what would happen if you try to access it?
because c++ makes no checks for valid pointers the behaviour is actually undefined. The program may crash if you are lucky, or may execute as normal, or the variable now has garbage..., whatever.

that scenario is equivalent to your Monster *pMonster1 = &Goblin();
there is no Goblin object because it died just after that line, so you can't actually do pMonster->atk() and you end up seeing "This crap still isn't working."

¿what does work?

Goblin g;
Monster *ptr = &g; //note, same scope 

here dereferencing `ptr' is fine, because `g' still exists and will continue to exists along `ptr'
an alternative is to use dynamic allocation, the object will exists till it's deleted

Monster *m;
//...
m = new Goblin();
//...
m->atk();
//...
delete m;

(use of smart pointers is recommended)


now, about the weapons
first a simplification

class Weapon{
	std::string name;
	int damage;
};

a weapon has two members: name and damage.
you went and do

class Dagger: public Weapon{
	Dagger(){
		this->name = "Dagger";
		this->damage = 5;
	}
};

Weapon w = Dagger();
w.name; //Dagger
w.damage; //5 

so far so good

then later you create a magical weapon, it has mana that it must recharge

class Magic_blade: public Weapon{
	int mana;
};

Weapon w = Magic_blade();
w.mana; //¿? 

`Weapon' only knows about its members, that is, name and damage
it does not know about `mana' so that variable is lost, that's object slicing.

[devious dm]

Okay, I follow so far. (I realized that what I followed in debugging wasn't what I thought it was, it was actually the line above it that had been commented out... my bad.) Thanks for the good explanation for that.

So, if that's the case, then it looks like dynamic allocation is my only option, because when it comes time to generate a new monster, say a hydra, in place of the goblin, then the new attack function gets sliced unless the goblin is deleted and a new pointer is created that points to a new object of the Hydra type, correct?

EDIT: A problem i can see with that though, is that if I do use dynamic allocation and smart pointers, then when the object and pointer are created (in this case, in the "random_monster()" function), isn't the object just deleted when the function's scope is closed, leaving a pointer that points to empty memory?

[devious dm]

@ne555 Thank you very much. I took a closer look at smart pointers and discovered the "shared_ptr", and that combined with proper scoping has solved all of my problems. The code now compiles and executes perfectly! I really appreciate the time you took to explain all that ^^ to me!