Cleanups BoundingBoxAxisAligned before refactoring
ClosedPublic
Actions

Authored by vladislavbelov on Jun 3 2019, 9:58 PM.

Details

Reviewers

wraitii

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP22372: Cleanups BoundingBoxAxisAligned and fixes coding styles a bit.

Summary

Subj.

Test Plan

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

vladislavbelov created this revision.Jun 3 2019, 9:58 PM

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

Harbormaster completed remote builds in B7757: Diff 8297.Jun 3 2019, 10:27 PM

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. I don't see what sane 'client' code would need this offhand.

vladislavbelov added inline comments.Jun 5 2019, 8:36 PM

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.