stellarium
stellarium copied to clipboard
Stellarium
Refactor Observability plugin to use signals for reacting to location state changes
Previous to this PR, the Observability plugin would track the location state during draw loops. This is error prone as the developer must ensure to reset its state in the appropriate places.
What this PR does is let StelLocationMgr be the sole owner of the state by using signals and slots to react to the location change event. This is the first PR in a series of PRs that are derived from a branch I'm working on. I'll try to keep these PRs as focused as possible.
There are A LOT of whitespace/formatting changes. I used the requested formatting configuration, and I guess it hasn't been run on those files for a long time.
Type of change
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] This change requires a documentation update
How Has This Been Tested?
The changes in this PR are relatively simple. No intentional changes have been made to the calculation logic.
Testing consisted of running Stellarium, enabling the Observability plugin, clicking an object, and ensuring that a report showed up in the bottom left hand side of the screen.
Application logs were also checked to ensure that connections were made and closed at the appropriate times. They were also checked to see if new errors were introduced.
Test Configuration:
- Operating system: macOS Catalina v10.15.7
- Graphics Card: Intel Iris 1536 MB
Checklist:
- [x] My code follows the code style of this project.
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation (header file)
- [ ] I have updated the respective chapter in the Stellarium User Guide
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream modules
This pull request introduces 2 alerts when merging e4ef01aaa2894faa19f08f58d2d26404c01432c3 into 2d8b4cc2de7c2d777b8a1577a2f1f2216d72eabe - view on LGTM.com
new alerts:
- 2 for Multiplication result converted to larger type
![lgtm-com[bot] avatar](https://avatars.githubusercontent.com/in/17324?v=4 "Published by a pub.dev verified publisher"){kind=small}
@alex-w Can you tell me what steps you took for report to not show information? I set up stellarium with the same parameters, but the data is available in my setup:
- Run Stellarium and enable the plugin
- Select any object
- Open Location dialog and change the location (you can do it twice or more times)
- Press "Return to default location"
@alex-w Both the language issue and report issue are fixed. Can you verify that it works for you as well?
@colossatr0n It's stop working after you click the Moon. Select some stars -> select the Moon -> select a star.
@worachate001 Just pushed a fix. Thanks for finding that one.
It works as expected now, thank you.
Important note: our .clang* file are not completed and rules didn't fully follow our coding style at the moment
@alex-w Is there a TODO for me regarding your last comment?
Probably you should re-check the coding style in this PR (did you used our settings file for QtCreator?)
@alex-w Is there a TODO for me regarding your last comment?
Probably you should re-check the coding style in this PR (did you used our settings file for QtCreator?)
Yes. I used the provided XML file. Maybe I did it incorrectly? I selected all the code > Edit > Advanced > Auto format selection.
Yes. I used the provided XML file. Maybe I did it incorrectly? I selected all the code > Edit > Advanced > Auto format selection.
Good question! I've tried apply auto format in QtCreator 8 (macOS) with active "Stellarium" C++ style and he is formatted the code as described in Qt style. :-/
This pull request has conflicts, please resolve those before we can evaluate the pull request.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Conflicts have been resolved. A maintainer will review the pull request shortly.
@colossatr0n please rebase this PR from current master. The online diff tool does not work properly. I don't know what the conflicts are, but they should be minimal.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
Pushed and cleaned up the messed up formatting. I need to retest, but am having troubles building for some reason. I'm getting build issues saying that the QtScript file can't be found. This was before rebasing, and afterward as well. Building master to see if I can at least get that to build.
EDIT: Can't build master either.
/Users/user/code/stellarium/src/scripting/StelScriptMgr.cpp:52: error: 'QtScript' file not found
/Users/user/code/stellarium/src/scripting/StelScriptMgr.cpp:52:10: fatal error: 'QtScript' file not found
#include <QtScript>
^~~~~~~~~~
Any ideas?
QtScript is a deprecated part of Qt5. When installing Qt5 libraries, you must tick a separate box to install it. When you are building with Qt6, you must for now configure ENABLE_SCRIPTING=false in cmake. I am in the process of re-enabling scripting in Qt6 with the newer scripting engine.
Hello @colossatr0n! Please check the fresh version (development snapshot) of Stellarium: https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot
Hello @colossatr0n!
Please check the fresh version (development snapshot) of Stellarium: https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot
Hello @colossatr0n!
Please check the latest stable version of Stellarium: https://github.com/Stellarium/stellarium/releases/latest