Adds projection type to CCamera to control usages of projection dependent properties
ClosedPublic
Actions

Authored by vladislavbelov on Oct 3 2019, 4:22 PM.

Details

Reviewers

None

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP23036: Adds projection type to CCamera to control usages of projection dependent…

Summary

Subj.

Test Plan

Apply the patch and compile the game
Run the game and make sure that no assertions during a game and big screenshots work as before
Run tests and make sure that all tests are passed as before the patch.

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.Oct 3 2019, 4:22 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/904/display/redirect

Harbormaster failed remote builds in B9705: Diff 10062!Oct 3 2019, 4:22 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/389/display/redirect

Not particularly a fan of ENSURE, usually it reads like TODO and it has the issue that the user can click on "continue" (thats what the average player does if he triggers one).
Better the code is written in such a way that it becomes impossible to trigger, or that it is caught and reported and runtime continues.
(Wondering if one could use a custom class or pass a functor instead of hardcoding the choices in the enum)
Otherwise no objections.

source/graphics/Camera.cpp
64 ↗	(On Diff #10062)	Should `m_FOV` be set in the custom case too (perhaps cleared)?

In D2351#98299, @elexis wrote:

Not particularly a fan of ENSURE, usually it reads like TODO and it has the issue that the user can click on "continue" (thats what the average player does if he triggers one).

Yeah, I can call it TODO, because I want to remove it after new projection type will be added.

(Wondering if one could use a custom class or pass a functor instead of hardcoding the choices in the enum)

One could, but it seems a much more complex code. For ex, if we hides projection types, then the projection object should know about how to calculate refraction/reflection camera.