ImVec2/ImVec4: add basic algebra

[gucio321]

close #12

methods:

1. for vec2:

• Add
• Sub
• Mul
• Div

2. for vec4:

• Add
• Sub

let me know if anything else could be added
CC @neclepsio

[gucio321]

sadly, can't say if it compiles, because of #43 😢

[gucio321]

well, lets check it now

[gucio321]

shall I add some tests for these methods or you trust them ;-)

[eliasdaler]

shall I add some tests for these methods or you trust them ;-)

Let's add them if you don't mind.

Btw, are these operations in Dear ImGui's C++ code? I think we should only add operators which ImGui defines.

[gucio321]

Btw, are these operations in Dear ImGui's C++ code? I think we should only add operators which ImGui defines.

Which operators do you exactly mean?

[gucio321]

ok, @eliasdaler unittest is ready

[eliasdaler]

Btw, are these operations in Dear ImGui's C++ code? I think we should only add operators which ImGui defines.

Which operators do you exactly mean?

For some reason I believed that Dear ImGui defined some operator+/- etc. for you, but I was mistaken. Disregard my comment.

[eliasdaler]

Just make a Vec2 and b Vec2. fields and args is a bad name.
The test names are noisy - I'd say it's better to drop them and make them comments. This applies to all the tests.

[gucio321]

I'd leave them as is because that template is automatically generated by goland

[eliasdaler]

"that template is automatically generated by goland"
And? This doesn't mean it's good code.

[gucio321]

"that template is automatically generated by goland"
And? This doesn't mean it's good code.

ofc it doesn't, I just think that it is commonly used 😄
but let's deiscuss it:

The test names are noisy - I'd say it's better to drop them and make them comments. This applies to all the tests.

I can't agree, lets imagine that one of tests fails. If test didn't have a name, you wouldn't know which exactly failed. The name (it could be anything unique e.g. "red fox" or so...) helps you identify the faulty place, doesn't it?

Just make a Vec2 and b Vec2. fields and args is a bad name.

You're probably right. It isn't the best of names, but it makes sense after considering it:
"fields" stands from struct (reciver's) fields
"args" are arguments of method.

If you insist, I can change it, however IMO it doesn't really matter.

[eliasdaler]

"ofc it doesn't, I just think that it is commonly used 😄"

1. I didn't see it much in real code. And if I did, I would get really annoyed
2. I think it's better to do what Go's standard library does with tests - do minimal amount of "magic" for tests. If the test is not trivial, it should be moved to a new unit test. If it's trivial, then it's easy to see which input caused which output and which case failed. "'add positive' test failed!" has much less meaning than "test failed: Vec{10,20}.Add(Vec2{40,30}) - expected Vec2{50,50}, got Vec2{10, 10}"

See here for example.

Note that "expected X, got Y" string should include input arguments as well.

Add() - expected X got Y is not as good as Vec2{X,Y}.Add(Vec2{X2,Y2}) - expected X, got Y

[gucio321]

ok, I've removed argss/fields and replaced with simple p/q.
However, after looking on the code I've noticed thet name has one more reason for keeping them there.
the script runs a sub-test wit t.Run and it needs test's name.

[eliasdaler]

Looks good now, thanks!