CODEWITHSUNDEEP
c++
WideEyedNewbie
WideEyedNewbie

Reputation: 11

C++ operator= overload works selectively

I appear to have an assignment operator that works circumstatially. I have a Vector2 class which contains two float's, x and y. Here is the operator= method:

Vector2 Vector2::operator=(const Vector2 &right)
{
    Vector2 result;
    result.x = right.x;
    result.y = right.y;
    return result;
}

Within an onMove method for an entity I have:

Vector2 newPosition = position + (speed * getSpeedFactor());

if (posValid(newPosition))
{
    position.x = newPosition.x;
    position.y = newPosition.y;
    //position = newPosition;
}

Where speedFactor depends on framerate and posValid is a check for whether a point is in a wall or something. position and speed are also Vector2's. The first assignment:

Vector2 newPosition = position + (speed * getSpeedFactor());

works, and with the code as is I get the expected/intended behaviour, however

position = newPosition;

has no effect, whether on it's own or before or after the .x & .y assignments.

Upvotes: 1

Views: 122

Answers (2)

Attila
Attila

Reputation: 28762

Your operator=() is wrong, it should modify the current object, not the extra you create

Vector2& Vector2::operator=(const Vector2 &right) 
{
  x = right.x; 
  y = right.y; 
  return *this; 
}

Note: the return type is a reference and you are returning the reference to the current object, not a copy of it (or of a new one like in your code).

The reason it worked for 

Vector2 newPosition = ...;

is because this is not default-construct + assignment but a copy-construct call.

Upvotes: 14

QuantumMechanic
QuantumMechanic

Reputation: 13946

You aren't actually assigning to position. You are making a temporary Vector2 object, assigning to it, then implicitly copying it and returning the copy. You are never modifying the actual object.

You need something like:

Vector2& Vector2::operator=(const Vector2 &right)
{
    x = right.x;
    y = right.y;
    return *this;
}

Upvotes: 1

Related Questions