Merges two camera JS interface functions into single function
ClosedPublic
Actions

Authored by vladislavbelov on Dec 14 2019, 12:37 PM.

Details

Reviewers

None

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP23264: Merges two camera JS interface functions into the single one.

Summary

We used two queries to get coordinates of camera pivot. It seems useless, as we always use both coordinates.

Test Plan

Apply the patch and compile the game
Run the game on some map
Make sure that camera jumping works

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

vladislavbelov created this revision.Dec 14 2019, 12:37 PM

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

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

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

Linter detected issues:
Executing section Source...

source/graphics/scripting/JSInterface_GameView.h
|   1| /*·Copyright·(C)·2018·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2018"

source/graphics/scripting/JSInterface_GameView.h
|  30| namespace·JSI_GameView
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceJSI_GameView{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

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

Harbormaster completed remote builds in B10295: Diff 10579.Dec 14 2019, 12:45 PM

elexis added a subscriber: elexis.Dec 15 2019, 1:41 AM

elexis added inline comments.

source/graphics/scripting/JSInterface_GameView.cpp
80 ↗	(On Diff #10579)	ToJSVal<Vector3D>?

elexis added inline comments.Dec 15 2019, 4:16 PM

source/graphics/scripting/JSInterface_GameView.cpp
168 ↗	(On Diff #10579)	Perhaps this even works, I didn't try: `scriptInterface.RegisterFunction<CVector3D, &GetCameraPivot>("GetCameraPivot");`

vladislavbelov added inline comments.Dec 15 2019, 4:18 PM

source/graphics/scripting/JSInterface_GameView.cpp
168 ↗	(On Diff #10579)	Thx, I will try.

@elexis It seems that we don't have conversion for CVector3D, but we have for CFixedVector3D.

We don't have JS conversion for CVector3D. Having it may lead to bugs that someone passed CVector3D to the simulation. So I'm leaving the code as is, as it doesn't change the behaviour.

We might want to refactor such cases later.

This revision was not accepted when it landed; it landed in state Needs Review.Dec 20 2019, 12:41 AM

Closed by commit rP23264: Merges two camera JS interface functions into the single one. (authored by vladislavbelov). · Explain Why

This revision was automatically updated to reflect the committed changes.

vladislavbelov added a commit: rP23264: Merges two camera JS interface functions into the single one..

Owners added a subscriber: Restricted Owners Package.Dec 20 2019, 12:41 AM