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

Save Scene #128

Merged
merged 81 commits into from
Apr 27, 2020
Merged

Save Scene #128

merged 81 commits into from
Apr 27, 2020

Conversation

Schneegans
Copy link
Member

@Schneegans Schneegans commented Mar 24, 2020

This PR adds initial support for saving the current scene. The idea is to extend our settings file to store the scene state (including the state of each and every plugin). There are now basically four setting types in the Settings class mapping to elements in the JSON file:

std::optional< T >: Settings which have a std::optional type can be defined in the JSON file but do not have to. If they are not present in the JSON file, they will be set to std::nullopt. When settings were reloaded, you have to check whether the value has changed. When saving, it will only be written if it is not currently set to std::nullopt.
cs::utils::Property< T >: This is a mandatory element; omitting it in the JSON file will lead to an error. You can connect to the onChange() signal in order to be notified when the value changes. This, for example, could be caused by by modifing a corresponding widget in the user interface or by reloading the settings.
cs::utils::DefaultProperty< T >: Similar to the Property but not mandatory. When reading from file, it will be set to its default state if it's not present in the file. On save, it will not be written to file when it's currently in its default state.
Everything else: Everything else is considered to be mandatory.

The following has been implemented:

  • The Settings class has been extended to store all properties describing the current scene state. This includes most of the properties which previously were public members of the GraphicsEngine, such as pEnableLighting.
  • Refactoring of the de-serialization of the JSON file. Now there is only a (overloaded) deserialize method instead of a parseSection, parseMap, parseOptional, parseOptionalSection, parseOptionalVector, ....
  • Implementation of the cs::utils::DefaultProperty template class.
  • Implementation of the from_json and to_json of all members of the Settings class.
  • Implementation of the from_json and to_json of all members of the plugin settings classes.
  • A new JavaScript callback: CosmoScout.callbacks.core.load(settingsFile)
  • A new JavaScript callback: CosmoScout.callbacks.core.save(settingsFile)
  • Reloading works for all core settings except for Events on the timeline. This will be implemented in another PR refactoring the Bookmark / Event-System.
  • Reloading works for the csp-atmospheres plugin.

The following is still missing:

  • Reloading for other plugins (only csp-atmospheres is implemented for now)
  • Saving and loading of the current simulation time.

Related PRs:

Feel free to experiment and add some comments!

@Schneegans Schneegans added new feature A feature request or a general improvement idea discussion needed Further information is requested labels Mar 24, 2020
@Schneegans Schneegans added this to the Version 1.3.0 milestone Mar 24, 2020
@Schneegans Schneegans self-assigned this Mar 24, 2020
@github-actions
Copy link

Pull Request Test Coverage Report for Build c82b21d82ab779a2e14649411f11febaa619d4c8-PR-128

  • 6 of 504 (1.19%) changed or added relevant lines in 15 files are covered.
  • 736 unchanged lines in 25 files lost coverage.
  • Overall coverage increased (+0.02%) to 4.816%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/cs-core/PluginBase.cpp 0 1 0.0%
src/cs-core/tools/MultiPointTool.cpp 0 1 0.0%
src/cs-utils/AnimatedValue.hpp 0 1 0.0%
src/cs-core/tools/Mark.cpp 0 4 0.0%
src/cs-utils/Property.hpp 1 6 16.67%
src/cs-graphics/MouseRay.cpp 0 6 0.0%
src/cosmoscout/main.cpp 0 7 0.0%
src/cs-utils/DefaultProperty.hpp 5 12 41.67%
src/cs-core/SolarSystem.cpp 0 11 0.0%
src/cs-core/TimeControl.cpp 0 16 0.0%
Files with Coverage Reduction New Missed Lines %
plugins/csp-atmospheres/src/Plugin.hpp 1 33.33%
src/cs-graphics/MouseRay.hpp 1 0%
src/cs-core/TimeControl.cpp 2 1.82%
src/cs-core/GraphicsEngine.cpp 3 0.86%
src/cs-core/SolarSystem.cpp 3 0.4%
src/cosmoscout/Application.cpp 5 1.84%
src/cs-core/GuiManager.cpp 5 0.53%
src/cs-utils/Property.hpp 7 18.52%
src/cs-core/Settings.cpp 9 0.4%
src/cs-core/Settings.hpp 11 0%
Totals Coverage Status
Change from base Build 8eb9e8e54efccffefd0eec3e2572967533be7cd0: 0.02%
Covered Lines: 610
Relevant Lines: 12666

💛 - Coveralls

config/base/scene/simple_desktop.json Show resolved Hide resolved
resources/gui/js/apis/gui.js Outdated Show resolved Hide resolved
resources/gui/js/apis/gui.js Outdated Show resolved Hide resolved
src/cosmoscout/Application.cpp Outdated Show resolved Hide resolved
src/cosmoscout/Application.cpp Outdated Show resolved Hide resolved
src/cs-core/tools/MultiPointTool.cpp Outdated Show resolved Hide resolved
src/cs-graphics/MouseRay.cpp Outdated Show resolved Hide resolved
src/cs-utils/DefaultProperty.hpp Outdated Show resolved Hide resolved
src/cs-utils/DefaultProperty.hpp Outdated Show resolved Hide resolved
src/cs-utils/DefaultProperty.hpp Outdated Show resolved Hide resolved
src/cs-core/TimeControl.cpp Outdated Show resolved Hide resolved
src/cs-core/TimeControl.cpp Outdated Show resolved Hide resolved
src/cs-core/TimeControl.cpp Outdated Show resolved Hide resolved
src/cs-core/TimeControl.cpp Outdated Show resolved Hide resolved
src/cs-core/TimeControl.cpp Outdated Show resolved Hide resolved
src/cs-core/TimeControl.hpp Outdated Show resolved Hide resolved
@Schneegans Schneegans marked this pull request as ready for review April 24, 2020 08:23
@Schneegans Schneegans changed the title [WIP] Save Scene Save Scene Apr 24, 2020
src/cs-core/tools/MultiPointTool.cpp Outdated Show resolved Hide resolved
src/cs-core/Settings.hpp Outdated Show resolved Hide resolved
src/cs-core/Settings.cpp Outdated Show resolved Hide resolved
src/cs-core/GuiManager.cpp Outdated Show resolved Hide resolved
src/cs-graphics/MouseRay.cpp Outdated Show resolved Hide resolved
src/cs-core/tools/MultiPointTool.cpp Outdated Show resolved Hide resolved
src/cs-core/tools/MultiPointTool.cpp Outdated Show resolved Hide resolved
src/cs-core/tools/MultiPointTool.cpp Outdated Show resolved Hide resolved
@Schneegans
Copy link
Member Author

As I said before, for csp-satellites, I would like to wait until #142 is resolved, and for csp-fly-to-locations I would like to wait until #144) is resolved. But it's pretty useful already so once all CI-issues are resolved, we could merge this I think.

Next I will try to implement that the user interface is updated when settings are reloaded. This shouldn't be so complicated but this PR is already huge enough...

@Schneegans
Copy link
Member Author

I have no idea what's happening to clang there... I cannot even reproduce it with the same version of clang...

@Schneegans
Copy link
Member Author

Finally. GitHub's clang insisted that

struct Trail {
    double mLength{};
    int32_t mSamples{};
    std::string mParent;
};

is not default-constructible. Even for

struct Trail {
    Trail() = default;
    double mLength{};
    int32_t mSamples{};
    std::string mParent;
};

the assert failed. However

struct Trail {
    Trail() {}
    double mLength{};
    int32_t mSamples{};
    std::string mParent;
};

works?!

@Schneegans
Copy link
Member Author

So... any open issues?

@Schneegans Schneegans merged commit 74d7eed into develop Apr 27, 2020
@Schneegans Schneegans deleted the feature/save-scene branch April 27, 2020 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion needed Further information is requested new feature A feature request or a general improvement idea
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants