Page MenuHomeWildfire Games

Cleanups BoundingBoxAxisAligned before refactoring
ClosedPublic

Authored by vladislavbelov on Mon, Jun 3, 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
  1. Apply the patch and the game with tests
  2. Run tests
  3. Make sure that all related tests are passed
  4. Run the game
  5. 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.Mon, Jun 3, 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

wraitii added a subscriber: wraitii.Wed, Jun 5, 7:09 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.Wed, Jun 5, 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.

wraitii accepted this revision.Wed, Jun 5, 9:52 PM

Seems fine, passes tests, gets in-game with no changes that I can notice.

source/maths/Vector3D.h
36 ↗(On Diff #8297)

Fair enough.

This revision is now accepted and ready to land.Wed, Jun 5, 9:52 PM
This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Wed, Jun 12, 10:23 PM