Use Size when it should be used.

[simonmika]

Use size for translations not points. For example in FloatEuclidTransform. See https://github.com/cogneco/ooc-kean/pull/662 for details.

[simonmika]

Let us also change the properties of all size covers into x and y.

[bettinaselig]

Change FloatSize2D to a more intuitive name. @niklasborwell @sebastianbaginski @marcusnaslund @davidhesselbom

[bettinaselig]

Suggestions: FloatVector2D, FloatDirection2D

[simonmika]

What about FloatDisplacement2D?

[davidhesselbom]

Are we keeping FloatSize2D? A FloatBox2D consisting of a FloatPoint2D and a FloatDisplacement2D doesn't seem very intuitive.

[bettinaselig]

Just brain storming... FloatTranslation2D

[bettinaselig]

Can collect first and then make pick the best, please?

[marcusnaslund]

This has probably been brought up before but I'd like to point (hehe) out that the Point covers have some properties that are generally associated (mathematically) to vectors, not points. So having a FloatVector2D and a FloatPoint2D would imply some consistency there which we don't have.

[davidhesselbom]

FloatDimensions2D should cover both size and translations, as should the already suggested FloatVector2D.

[bettinaselig]

Ok. We can also discuss if FloatPoint2D can get a better name.

[niklasborwell]

If we're gonna make this change, I would prefer to clean up the things @marcusnaslund mentioned. Otherwise we're just exchanging a half-weird system for another.

[bettinaselig]

In computer graphics the terms are location vector and direction vector.

[bettinaselig]

Position: https://en.wikipedia.org/wiki/Position_%28vector%29
Direction: https://en.wikipedia.org/wiki/Direction_vector

[niklasborwell]

So I would suggest, remove all the things that are actually vector-related and move them into a general vector class, FloatVector2D. Keep the FloatPoints, but more specific.
I think anybody would feel comfortable using a FloatVector2D, as it is a standard concept. A FloatDisplacement or FloatTranslation would just seem like special cases that would cause more confusion.

[marcusnaslund]

"Real" Point and Vector covers sound good to me, I am honestly quite confused by the division we have now.
There's probably merit to what @bettinaselig says as well, maybe that's more close to what we need in other projects.

[davidhesselbom]

I thought a point was a vector... 😕

[bettinaselig]

Yes... well... it depends. In my world everything are tupels :-)

[bettinaselig]

Personally, I can live with point and vector. But I think to remember that @simonmika did not like this concept that much.

[tomasmahlberg]

If we are changing the properties in size to x and y and so on I will have to agree that vector would be the more intuitive and general naming. Then we would have one cover for positions (point) and one for displacements (vector) and they could also be used for more general cases, so my vote right now would also be on this set of covers. translation, dimension, displacement and so on all sound too specific as has been mentioned and are likely to cause some confusion, which I guess is what we're trying to avoid here.

[niklasborwell]

Talked with Simon today. He agreed with the idea of vectors + points, with the width, height, depth renamed to x, y and z. That means we are ready to fix this, but it is quite a big project. We should discuss who is going to to what. Volunteers welcome!

[marcusnaslund]

Haha, how fun for the person who has to review that PR! Oh wait..

[davidhesselbom]

Just wanted to say that I tried this, and it works, with no warnings and no errors from valgrind:

Vector: cover {
    x, y: Int
    init: func@
}
Point: cover from Vector extends Vector {
    init: func@
    scalarProduct: func (other: This) -> Int { this x * other x + this y * other y }
}
Size: cover from Vector extends Vector {
    init: func@
    width: Int { get { this x } }
    height: Int { get { this y } }
    area ::= this width * this height
}

vector := Vector new()
vector x = 2
vector y = 3

size := vector as Size
size width toString() println() // prints 2, yay
size height toString() println() // prints 3, yay
size area toString() println() // prints 6, yay

point := size as Point
point scalarProduct(point) toString() println() // prints 13, yay

I haven't figured out how to set a width and height, unfortunately.

[davidhesselbom]

Fixed it:

Vector: cover {
    x, y: Int
    init: func@
}
Point: cover from Vector extends Vector {
    init: func@
    scalarProduct: func (other: This) -> Int { this x * other x + this y * other y }
}
Size: cover from Vector extends Vector {
    init: func@
    width: extern (x) Int
    height: extern (y) Int
    area ::= this width * this height
}

vector := Vector new()
vector x = 2
vector y = 3
size := vector as Size
size width toString() println() // prints 2
size width = 4
size width toString() println() // prints 4
size height toString() println() // prints 3
size area toString() println() // prints 12
point := size as Point
point scalarProduct(point) toString() println() // prints 25, yay

So that means we can treat covers like objects with inheritance (sort of), and we can access x and y in Vector with the variables width and height in Vectors cast to Size, as if the Vector were a Size all along, then back to Vector or Point, all without conversion constructors and the overhead they cause. This also means we can call a Vector a Size where it makes sense to do so, and when cast to a Size, it will have functions that make sense for a Size but that are otherwise not accessible.

Pretty neat, huh?

[niklasborwell]

Can you access point x and point y? And what about functions in Vector, are they accessible in Point and Size?

[davidhesselbom]

I added

Vector: cover {
    x, y: Int
    init: func@
    foo: func -> Int { this x + this y }
}

and

    point x toString() println() // prints 4, as expected
    point foo() toString() println() // prints 7, as expected

and it works, so think the answer is yes.

[davidhesselbom]

I don't know about you, but ooc (as a programming language) suddenly looks a whole lot better to me.

[niklasborwell]

If it works properly, this would make this issue a whooole lot easier, and improve the code we have today a lot too. Let's hope we don't encounter any serious issues with it. I'm still a bit suspicious about the width: extern (x) Int part..

[davidhesselbom]

I'm not sure about the extern either, but that's how we use covers from external libraries already. Without it, we wouldn't be able to use the X11 window, for instance. I'm not 100% sure it's needed here, since Vector isn't... external, but if all else fails, we know it'll work if we use that keyword.

EDIT: I checked with Amos:

an extern name is the underlying name of a field declared somewhere else

so we should use extern when "inheriting" from another cover, and it's the only way to do it.

[bettinaselig]

That is awesome!

So what about the naming problem? Since Vector is now taken, I guess we need to have a new discussion about Size. I still think we need a more intuitive name for that cover.

I suggest Direction and some adjustments in Box:

• Box is defined by the point leftTop and a direction direction, which will span the box starting from leftTop.
• The properties width and height will be exchanged by

width ::= this direction x abs()
height ::= this direction y abs()

What do you think?

[davidhesselbom]

FloatComplex could also be a cover from FloatVector2D, if we want:

real: extern (x) Float
imaginary: extern (y) Float

[bettinaselig]

Sounds reasonable.

[tomasmahlberg]

+1 for this latest suggestion.

And this new-found feature looks awesome, so I had to try it out for a bit. It seems you have to declare getters and setters for variables in the base cover, otherwise things like init: func@ (=x, =y) in a derived cover gives this familiar error:

/home/tmahlberg/versioned/ooc/ooc-kean/cover_test.ooc: In function ‘cover_test__Point_init’:
/home/tmahlberg/versioned/ooc/ooc-kean/cover_test.ooc:17:37: error: lvalue required as left operand of assignment
   init: func@ (=x, =y)
                                     ^
/home/tmahlberg/versioned/ooc/ooc-kean/cover_test.ooc:17:37: error: lvalue required as left operand of assignment
   init: func@ (=x, =y)
                                     ^
C compiler failed on module cover_test from cover_test, bailing out

[bettinaselig]

Can someone make some issues, so we can start working on these changes?

[davidhesselbom]

@tomasmahlberg That's... a big problem. I reported it here: ooc-lang/rock#949

[davidhesselbom]

Still, it seems to be a fixable bug, and not a limitation in the language itself. I have no ETA, unfortunately, but perhaps we could lay low with this for a while until we know? I would prefer keeping our current, broken way of doing things over changing everything everywhere and then doing it all over again because the bug was fixed a few days or weeks later.

[bettinaselig]

Once more to the original issue: Transforms can only be applied to Points.

[simonmika]

I am still not convinced of the benefit of introducing yet another cover. Why would we need Vector and Size?

[davidhesselbom]

If you don't think we need it, okay, fine. Point and Complex can still "inherit" from Vector, though... well, if it weren't for the bug.

[niklasborwell]

I agree. We can have a property in box: size: FloatVector2D. I don't think there is a need for a specific size cover.

[niklasborwell]

Accidentally pressed close...sorry.

[bettinaselig]

Ok. My thoughts I have written here. I will exit this discussion now.

[simonmika]

This is what I think:

1. Rename size to vector.
2. Rename width and height to x and y.
3. Move all points, vectors, boxes, shells, etc into a geometry component.
4. No weird casts aka inheritance.
5. Kill FloatPoint4D. There is no such thing in our code.
6. And for complex numbers: Not everything with two values is a vector.

And in the context of the geometry component this is what is meant:

• Points are locations in space.
• Vectors are displacements in space.

[emilwestergren]

I need FloatPoint4D to be replaced with a 4 float vector.

[emilwestergren]

I'll take care of FloatPoint4D since it's my hack. https://github.com/cogneco/ooc-kean/issues/830

[simonmika]

This is how we will make the changes:

• Remove the need for FloatPoint4D. Replace FloatPoint4D #830
• Rename width and height to x and y.
• Rename Size to Vector.
• Create geometry namespace.
• Ensure right use of Vector and Point everywhere.

@emilwestergren is doing the first one. Who wants to continue?

[marcusnaslund]

What's left to do in terms of "Ensure right use of Vector and Point everywhere."?

[niklasborwell]

Since it is hard to know when it is finished, this issue is probably not very useful anymore. If we find places where it should be changed, we can open new issues. I suggest closing this.

[marcusnaslund]

Agreed. An issue like "Do X wherever needed" doesn't quite fit our definition of a github issue.

[simonmika]

I was thinking about translation it FloatEuclidTransform. I nobody has any more ideas I will close tis now.