olc::vi2d multiplication with double is not commutative

[joshinils]

In this example:

olc::vi2d a{100,100};
olc::vi2d b = a * 0.7;
olc::vi2d c = 0.7 * a;

b will not equal c.
This is because the two operators are not doing the same mathematical operation: https://github.com/OneLoneCoder/olcPixelGameEngine/blob/b3759234b251bf674cbc3287621992eba5d9ad92/olcPixelGameEngine.h#L338 This one takes the argument of the same type as the template argument, for olc::vi2d that is an int, hence it will first cast the 0.7 (double) to 0 (int), then multiply.

https://github.com/OneLoneCoder/olcPixelGameEngine/blob/b3759234b251bf674cbc3287621992eba5d9ad92/olcPixelGameEngine.h#L350 This one on the other hand, takes the argument specifically as a double and multiplies the double with the template type and then casts to the template type.

These two operators offer a different functionality than is expected.

why doesnt there exist only one version which takes a different template argument type for the scalar, and one operator for the "lhs * vector" order which swaps the two arguments and calls the other operator?

[joshinils]

of course this also happens with the other types of operators

[joshinils]

oh and the current implementation also means that adding an olc::vi2d to an olc::vd2d is possible and results in an olc::vi2d, but adding an olc::vd2d to an olc::vi2d is not possible, whereas adding an int to a double and ading a double to an intis both possible and both result in a double if i do: auto v = 100 + 3.14 or auto v = 3.14 + 100. It would make sense to add the underlying types and then cast to the template type of one of the two vectors.

[OneLoneCoder]

Good observations as usual. I'll address in next update.

[OneLoneCoder]

Hi Josh, Ive been working through this with the imminent release of PGE2, and it works well until you have the scenario olc::vf2d * olc::vi2d.

template<class T, class A> inline v2d_generic<T> operator * (const v2d_generic<T>& lhs, const A& rhs)
{ return v2d_generic<T>((T)(rhs * (A)lhs.x), (T)(rhs * (A)lhs.y)); } // Thanks Joshinils
template<class A, class T> inline v2d_generic<T> operator * (const A& lhs, const v2d_generic<T>& rhs)
{ return v2d_generic<T>((T)(lhs * (A)rhs.x), (T)(lhs * (A)rhs.y)); }

Now I need to handle explicitly vec * vec, because the type A above wont discriminate. That's ok, I'll do this:

template<class T, class A> inline v2d_generic<T> operator * (const v2d_generic<T>& lhs, const v2d_generic<A>& rhs)
{ return v2d_generic<?>((?)(rhs.x * (?)lhs.x), (?)(rhs.y * (?)lhs.y)); }

But now I have an issue regarding which combination is correct? If both vecs are same type, no problem, but if they aren't which is right?

olc::vi2d b = { 3, 3 };
olc::vf2d c = { 0.5f, 0.5f };
olc::vf2d d = b * c; // d = {1.0, 1.0}
olc::vf2d e = c * b; // e = {1.5, 1.5}

[joshinils]

I'm not exactly sure what multiplication you want to implement. The dot product of two vectors results in a scalar, not a vector, the cross product is only defined for vectors of length 3.

Since this is what the underlying integral types do:

int   b = 3;
float c = 0.5f;
float d = b * c; // d == 1.5
float e = c * b; // e == 1.5

It could be wise to mimic the same behaviour. Either through template specialization or use of enable_if, but I don't think I'm qualified to aswer how. You could also only allow the type of A to be an integral type, so you would ban v2d from being a valid template argument for scalar multiplication with A.

As a mathematics student I'd argue neither are correct and the compiler should not let olc::vf2d d = b * c; compile, however float f = b * c; is fine and f should equal 3.

Could this work:?

template<class T> inline T operator * (const v2d_generic<T>& lhs, const v2d_generic<T>& rhs)
{ return lhs.x * rhs.x + lhs.y * rhs.y; }

Then the multiplication should be done on the integral type and work like what I first mentioned, right? Does a copy constructor for v2d exist? Can this be used to implicitly convert a vi2d to a vf2d and vice versa?

[OneLoneCoder]

It would be an element wise operation, which is quite customary though I appreciate the syntax makes it look like a cross product

[MyGoodSir]

the type_traits library can help with this

#include<type_traits>
template<class T, class A>
inline v2d_generic<typename std::common_type<T,A>::type>  operator *  (const v2d_generic<T>& lhs, const v2d_generic<A>& rhs) 
{ return v2d_generic<typename std::common_type<T,A>::type>(lhs.x * rhs.x, lhs.y * rhs.y); }

that makes this:

int main(){
    vd2d dvec(1.5, 1.5);
    vi2d ivec(2, 2);

    auto di_vec = dvec * ivec;
    auto id_vec = ivec * dvec;

    std::cout << "d * i: "<< di_vec << "\ni * d: " << id_vec;
}

output this:

d * i: (3.000000,3.000000)
i * d: (3.000000,3.000000)