Subj.
Details
- Reviewers
wraitii - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rP22372: Cleanups BoundingBoxAxisAligned and fixes coding styles a bit.
- Apply the patch and the game with tests
- Run tests
- Make sure that all related tests are passed
- Run the game
- Make sure that everything is rendered as before
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 Source... source/tools/atlas/GameInterface/ActorViewer.cpp | 213| » void·UpdatePropListRecursive(CModelAbstract*·model); | | [MAJOR] CPPCheckBear (syntaxError): | | Invalid number of character '{' when these macros are defined: 'MESSAGESSETUP_NOTFIRST'. Executing section JS... Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/differential/1596/display/redirect
This looks good.
source/maths/Vector3D.cpp | ||
---|---|---|
38 ↗ | (On Diff #8297) | Needless comment imo? When we've had inline hints like that we generally wrapped them such /* */anyways. |
source/maths/Vector3D.h | ||
36 ↗ | (On Diff #8297) | I think those should be private and matrix should probably be our friend - or alternatively just define these inline in BoundingBoxAligned.cpp if that's the only place where it's used. |
source/maths/Vector3D.cpp | ||
---|---|---|
38 ↗ | (On Diff #8297) | I don't mind, it's more my habit. |
source/maths/Vector3D.h | ||
36 ↗ | (On Diff #8297) | I don't see any reason to hide it from others. It's the same as FLT_MAX. But yes, it could be inlined. It can be used in any kind of search, where we have undefined value before start. |
Seems fine, passes tests, gets in-game with no changes that I can notice.
source/maths/Vector3D.h | ||
---|---|---|
36 ↗ | (On Diff #8297) | Fair enough. |