Cleanup and simple refactoring of SkyManager. It makes more clear a way to improve SkyManager.
Details
- Reviewers
Imarok - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rP21952: Cleanup and simple refactor the SkyManager.
- Apply & compile the patch
- Run the game and check that the sky still works, and that it's toggleable.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... source/tools/atlas/GameInterface/ActorViewer.cpp | 33| #include·"graphics/Terrain.h" | | [MAJOR] CPPCheckBear (syntaxError): | | Invalid number of character ({) when these macros are defined: ''. source/tools/atlas/GameInterface/ActorViewer.cpp | 33| #include·"graphics/Terrain.h" | | [MAJOR] CPPCheckBear (syntaxError): | | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'. source/tools/atlas/GameInterface/ActorViewer.cpp | 33| #include·"graphics/Terrain.h" | | [MAJOR] CPPCheckBear (syntaxError): | | Invalid number of character ({) when these macros are defined: '_MSC_VER'. Executing section JS...
Link to build: https://jenkins.wildfiregames.com/job/differential/777/
source/renderer/SkyManager.h | ||
---|---|---|
64–72 ↗ | (On Diff #6979) | They are used in Atlas below. |
source/renderer/SkyManager.h | ||
---|---|---|
64–72 ↗ | (On Diff #6979) | Sure, but why do we need a Getter and a Setter instead of raw access? |
source/renderer/SkyManager.h | ||
---|---|---|
64–72 ↗ | (On Diff #6979) | Because C++ doesn't have any simple access monitoring (because low-level). So we don't allow to access class members (especially private) to modify without notifying the parent class (by notifying means the methods calling). So it's the good practice to know when someone modifies your class state. |
source/renderer/SkyManager.h | ||
---|---|---|
64–72 ↗ | (On Diff #6979) | But why do we want to know it here? |
source/renderer/SkyManager.h | ||
---|---|---|
64–72 ↗ | (On Diff #6979) | We use the principe: class members aren't public, only struct ones are. We use classes like a black box, we don't want to show people how they work inside. We share only interfaces. Besides it, we'd need to do some staff here on the state changes, because I wanna add a more complex sky rendering technique. |
source/renderer/SkyManager.h | ||
---|---|---|
64–72 ↗ | (On Diff #6979) | Then it's totally ok. |