-
-
Notifications
You must be signed in to change notification settings - Fork 815
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
Refactor: getAngularSize->getAngularRadius #2180
Conversation
- rename to clarify that these are radii, not sizes - also fixed a few typecasting warnings
Great PR! Please pay attention to the following items before merging: Files matching
This is an automatically generated QA checklist based on modified files |
@alex-w several classes have small arbitrary but distinctly non-zero sizes. The size for stars is zero. Why not also for these classes? A non-zero value implies we could enlarge it to see more than a dot. |
@alex-w I have added 2 TODO annotations in the StelObject::getInfoMap(): I don't understand the test for negative values. (Just add an assert if in doubt?) And why do we need size in radians? |
I don't remember details, probably this part was added into map to backward compatibility. I think we may delete this data. |
I've refactor this stuff. The most of these non-zero values was added a long time ago (around series 0.11-0.12) and probably on some step of refactoring we just forgot the fix this points. |
I think I caught all cases now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Hello @gzotti! Please check the fresh version (development snapshot) of Stellarium: |
Hello @gzotti! Please check the latest stable version of Stellarium: |
It occurred to me that we need to rename this method to clarify that these values are in fact radii, not sizes
See the documentation in StelObject.h
Description
Fixes # (issue)
Screenshots (if appropriate):
Type of change
TODOs
How Has This Been Tested?
Test Configuration:
Checklist: