-1

Okay so I'm making my own Entity Component System, and I want to be able to set an Entity to another, here is how it looks:

Entity& operator=(const Entity& other) {
    this->Name = other.Name;
    this->Tag = other.Tag;
    this->IsStatic = other.IsStatic;
    return Entity(this->Name, this->Tag, true, this->IsStatic);
}

an Entity also has an ID which has to be unique from other Entities, but when i set an Entity to another, the ID also gets set:

'main.cpp'

Entity a = Entity("a", "tag of a"); // A automatically gets ID: 0 because its the first Entity created
Entity b = Entity("b", "tag of b"); // B gets ID: 1 because its the second Entity created
a.PrintAll(); // This is a function which prints the name, tag, and ID of an Entity, this prints out: " "a" has "tag of a" as a tag, and its ID = "0" "
// but after i set a to b, things get a little messy
a = b;
a.PrintAll(); // This now prints out: " "b" has "tag of b" as a tag, and its ID = "1" ", that should not happen, why did the ID of a change ?

the way IDs work is that when an Entity is constructed its ID is set to a global variable which increments by 1, like so:

'Public.h' // this is included everywhere, has all the global variables

int IDcounter = 0;
int GetNewID(){
 return IDcounter;
 IDcounter++;
}

And then inside the Entity constructor:

'Entity.cpp'

Entity::Entity(string name, string tag){
 this->name = name;
 this->tag = tag;
 this->ID = GetNewID(); // Everything fine, the problem is the operator=
}

EDIT:

I tried what you guys told me, here is how i tried it:

Entity* leshko;
Entity* ent2;
leshko = new Entity("leshko", "lesh"); 
ent2 = new Entity("ent2", "presh");
leshko->PrintAll(); // ID = 0
leshko = ent2;
leshko->PrintAll(); // ID = 1

I think the problem may be that I'm using pointer 'Entities' and not regular 'Entities', but I cannot change that.

kooldart
  • 61
  • 11
  • Recommend a read of [What is the copy-and-swap idiom?](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) for what is often a better way to do `operator=`. – user4581301 Nov 17 '16 at 15:58
  • _"I tried what you guys told me, here is how i tried it:"_ Who told you to do that? I don't see it anywhere. – Lightness Races in Orbit Nov 17 '16 at 18:39
  • @LightnessRacesinOrbit ok i'm pretty sure users: NathanOliver and Sam Varshavchik suggested that i use 'return *this', if i'm wrong please correct me. – kooldart Nov 17 '16 at 19:06
  • At no point did they instruct you to rewrite your code to use dynamic allocation everywhere. And there is no `return *this` in your "here is how i tried it" modified code snippet. – Lightness Races in Orbit Nov 17 '16 at 19:19
  • _"I think the problem may be that I'm using pointer 'Entities' and not regular 'Entities', but I cannot change that."_ No, you weren't using pointer 'Entities' (but now you are). I'm very confused by this question, and so are you! – Lightness Races in Orbit Nov 17 '16 at 19:20

2 Answers2

2

The issue here is you are trying to return a reference to a local variable. In your assignment operator you have

return Entity(this->Name, this->Tag, true, this->IsStatic);

Which creates a temporary. You cannot return that by reference.

What you want to do instead is return a reference to the object you just assigned to. You do that with

return *this;

Note that in your code leshko = ent2; is pointer assignment not object assignment. If you want to assign the underlying objects you need *leshko = *ent2;.

NathanOliver
  • 150,499
  • 26
  • 240
  • 331
  • It worked, I did some tests using integers and knew It was pointer assignment but didnt know how to fix it, thanks. – kooldart Nov 17 '16 at 17:36
1

Your operator= needs to simply return this.

Entity& operator=(const Entity& other) {
    this->Name = other.Name;
    this->Tag = other.Tag;
    this->IsStatic = other.IsStatic;
    return *this;
}

After all, *this is what you've just assigned to, and that's the result of the = operator, a reference to *this.

Sam Varshavchik
  • 84,126
  • 5
  • 57
  • 106
  • Yes that was how i had it the first time, then i changed it to the one shown up there, it doesn't work. – kooldart Nov 17 '16 at 16:00