Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.length() functions in types are confusing with length(x) functions. #245

Closed
Groovounet opened this issue Oct 5, 2014 · 7 comments
Closed
Assignees
Milestone

Comments

@Groovounet
Copy link
Member

Add GLM_FORCE_SIZE_FUNC to use size_t size() instead of length_t length().

@richardeakin
Copy link

I noticed that some function implementations are using the length() method (such as here). Won't this cause compile errors if GLM_FORCE_SIZE_FUNC is used, making length() undefined?

Apologies for the noise if you're already aware, and thanks again for change.

@Groovounet Groovounet reopened this Oct 11, 2014
@Groovounet
Copy link
Member Author

Damn, stupid overlook.
I'll fix it.

Groovounet added a commit that referenced this issue Oct 11, 2014
… constructor to quaternion. Fixed lack of conscistency or quaternion constructors with other types. Various uninitilized constructor optimizations
@Groovounet
Copy link
Member Author

This should be fixed.

Thanks for reporting,
Christophe

@richardeakin
Copy link

Awesome, nice solution.

@richardeakin
Copy link

Sorry to be the carrier of bad news always, but I found some other places that are still directly using length(), they look to be mainly in the debug asserts, which are using it like this->length().

/Users/r/code/graphics/glm/glm-git/glm/detail/type_vec2.inl:58:30: error: no member named 'length' in 'glm::tvec2<unsigned int, 0>'
                assert(i >= 0 && i < this->length());
                                     ~~~~  ^
...

Other thing is, is there any way to make use of some of the tests with this flag on? I see .length() is used in there, but then you possibly don't want to use detail::component_count(x) directly in the tests..

@Groovounet
Copy link
Member Author

It should be fixed now, until the next one (:p), and I have added a test.

Thanks,
Christophe

andrewfb added a commit to cinder/Cinder that referenced this issue Feb 4, 2015
@nickeb96
Copy link

fyi you will have issues with vec.length() and length(vec) in C++17 when the uniform call syntax is implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants