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

Rename vectors methods length and lengthSquared into norm and normSquared #2795

Merged
merged 1 commit into from
Oct 31, 2022

Conversation

10110111
Copy link
Contributor

When porting C++ code to GLSL the length() method can lead to hard to diagnose bugs (and I have been bitten by this), because this method in GLSL returns number of elements of a vector, while we supposed to get the norm of the vector. This patch renames the length() method of all VecMath's vectors, and for consistency also lengthSquared(), to say norm instead of length.

The old names are explicitly marked as deleted so that those who write new code don't get surprised by unavailability of these functions.

@github-actions
Copy link

Great PR! Please pay attention to the following items before merging:

Files matching src/**/*.cpp:

  • Are possibly unused includes removed?

This is an automatically generated QA checklist based on modified files

@10110111 10110111 force-pushed the master branch 2 times, most recently from e95a5df to 670ba7d Compare October 30, 2022 22:20
…ared

When porting C++ code to GLSL the length() method can lead to hard to
diagnose bugs, because this method in GLSL returns number of elements of
a vector, while we supposed to get the norm of the vector. This patch
renames the length() method of all VecMath's vectors, and for
consistency also lengthSquared(), to say norm instead of length.

The old names are explicitly marked as deleted so that those who write
new code don't get surprised by unavailability of these functions.
Copy link
Member

@gzotti gzotti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while length() is much better readable in terms of what is intended, comments and disable are sufficient. If you really need it, I shall not object.

@alex-w alex-w added this to the 1.2 milestone Oct 31, 2022
@alex-w alex-w merged commit c85aa79 into Stellarium:master Oct 31, 2022
@alex-w alex-w added the state: published The fix has been published for testing in weekly binary package label Nov 20, 2022
@github-actions
Copy link

Hello @10110111!

Please check the fresh version (development snapshot) of Stellarium:
https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot

@alex-w alex-w removed the state: published The fix has been published for testing in weekly binary package label Dec 25, 2022
@github-actions
Copy link

Hello @10110111!

Please check the latest stable version of Stellarium:
https://github.com/Stellarium/stellarium/releases/latest

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

Successfully merging this pull request may close these issues.

3 participants