From 40684e83d33eea7bfc91efb1e89d77d7e6ce8f14 Mon Sep 17 00:00:00 2001
From: David Fioramonti
Date: Thu, 7 Sep 2017 04:29:21 -0700
Subject: TITANIC: Better naming for Viewport function

I differentiated getRelativePosCentering() and
getRelativePosCentering2() since one was using
the raw Pose and one was using the regular Pose.
---
 engines/titanic/star_control/star_camera.cpp | 2 +-
 engines/titanic/star_control/viewport.cpp    | 2 +-
 engines/titanic/star_control/viewport.h      | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

(limited to 'engines')

diff --git a/engines/titanic/star_control/star_camera.cpp b/engines/titanic/star_control/star_camera.cpp
index cab29a99f4..4edc05bbf7 100644
--- a/engines/titanic/star_control/star_camera.cpp
+++ b/engines/titanic/star_control/star_camera.cpp
@@ -246,7 +246,7 @@ FVector CStarCamera::proc30(int index, const FVector &v) {
 }
 
 FVector CStarCamera::proc31(int index, const FVector &v) {
-	return _viewport.getRelativePosCentering2(index, v);
+	return _viewport.getRelativePosCenteringRaw(index, v);
 }
 
 void CStarCamera::setViewportAngle(const FPoint &angles) {
diff --git a/engines/titanic/star_control/viewport.cpp b/engines/titanic/star_control/viewport.cpp
index d277f4a2c5..07a6dcc36c 100644
--- a/engines/titanic/star_control/viewport.cpp
+++ b/engines/titanic/star_control/viewport.cpp
@@ -261,7 +261,7 @@ FVector CViewport::getRelativePosCentering(int index, const FVector &src) {
 }
 
 // TODO: Identical to getRelativePosCentering, was this meant to be different?
-FVector CViewport::getRelativePosCentering2(int index, const FVector &src) {
+FVector CViewport::getRelativePosCenteringRaw(int index, const FVector &src) {
 	FVector dest;
 	FPose pose = getRawPose();
 	FVector tv = src.matProdRowVect(pose);
diff --git a/engines/titanic/star_control/viewport.h b/engines/titanic/star_control/viewport.h
index d5c35b6317..082d063233 100644
--- a/engines/titanic/star_control/viewport.h
+++ b/engines/titanic/star_control/viewport.h
@@ -125,7 +125,7 @@ public:
 	FPose getRawPose();
 	FVector getRelativePosNoCentering(int index, const FVector &src);
 	FVector getRelativePosCentering(int index, const FVector &src);
-	FVector getRelativePosCentering2(int index, const FVector &src);
+	FVector getRelativePosCenteringRaw(int index, const FVector &src);
 
 	/**
 	 * All arguments are return values
-- 
cgit v1.2.3


From d69404d5de6fded94803c36bd0d5fe012d705c0a Mon Sep 17 00:00:00 2001
From: David Fioramonti
Date: Thu, 7 Sep 2017 05:08:38 -0700
Subject: TITANIC: Make use of CCameraAutoMover::setPath() more clear

There was a setPath() adn setPath2() function took in
different arguments and were doing the same thing, but
not using the different arguments. I made it into
one function that only takes in the arguments it uses.

Also it was marked virtual, but all the derived classes,
CMarkedAutoMover, and CUnmarkedAutoMover were just doing
there own thing and then calling this base class implementation.
Therefore, I made it be not virtual and the derived classes can do
there own thing and then call this, but since they are doing slightly
different things it makes sense to differentiate the names and not have
them all be called setPath. I.e., the derived classes also change
the orientation so that is included in their function names to reflect
that.
---
 engines/titanic/star_control/camera_auto_mover.cpp | 29 +++-------------------
 engines/titanic/star_control/camera_auto_mover.h   |  9 ++++---
 engines/titanic/star_control/marked_auto_mover.cpp |  4 +--
 engines/titanic/star_control/marked_auto_mover.h   |  2 +-
 .../titanic/star_control/marked_camera_mover.cpp   |  3 +--
 .../titanic/star_control/unmarked_auto_mover.cpp   |  4 +--
 engines/titanic/star_control/unmarked_auto_mover.h |  4 +--
 .../titanic/star_control/unmarked_camera_mover.cpp |  2 +-
 8 files changed, 17 insertions(+), 40 deletions(-)

(limited to 'engines')

diff --git a/engines/titanic/star_control/camera_auto_mover.cpp b/engines/titanic/star_control/camera_auto_mover.cpp
index fcd1e9e0ee..1b016bc549 100644
--- a/engines/titanic/star_control/camera_auto_mover.cpp
+++ b/engines/titanic/star_control/camera_auto_mover.cpp
@@ -41,25 +41,6 @@ CCameraAutoMover::CCameraAutoMover() : _srcPos(0.0, 1000000.0, 0.0) {
 	_transitionPercentInc = 0.0;
 }
 
-// TODO: same as setPath also orientations are not used
-void CCameraAutoMover::setPath2(const FVector &oldPos, const FVector &newPos,
-	const FMatrix &oldOrientation, const FMatrix &newOrientation) {
-	_srcPos = oldPos;
-	_destPos = newPos;
-	_posDelta = _destPos - _srcPos;
-
-	float temp = 0.0;
-	_posDelta.normalize(temp); // Do the normalization, put the scale amount in temp
-	_distance = temp;
-	_active = false;
-	_field34 = false;
-	_transitionPercent = 1.0;
-	_field40 = -1;
-	_field44 = -1;
-	_field48 = -1;
-	_field4C = 0;
-}
-
 // TODO: same as proc2 also orientations are not used
 void CCameraAutoMover::setOrientations(const FMatrix &srcOrient, const FMatrix &destOrient) {
 	_srcPos.clear();
@@ -70,18 +51,14 @@ void CCameraAutoMover::setOrientations(const FMatrix &srcOrient, const FMatrix &
 	_field34 = false;
 }
 
-// TODO: same as setPath2 also orientations are not used
-void CCameraAutoMover::setPath(const FVector &srcV, const FVector &destV, const FMatrix &orientation) {
+void CCameraAutoMover::setPath(const FVector &srcV, const FVector &destV) {
 	_srcPos = srcV;
 	_destPos = destV;
 	_posDelta = _destPos - _srcPos;
 
 	float temp = 0.0;
-	if (!_posDelta.normalize(temp)) {
-		// Do the normalization, put the scale amount in temp,
-		// but if it is unsuccessful, crash
-		assert(temp);
-	}
+	_posDelta.normalize(temp); // normalization won't happen if _posDelta is zero vector
+								// and that is okay
 
 	_distance = temp;
 	_active = false;
diff --git a/engines/titanic/star_control/camera_auto_mover.h b/engines/titanic/star_control/camera_auto_mover.h
index 25384632fa..d83f15013c 100644
--- a/engines/titanic/star_control/camera_auto_mover.h
+++ b/engines/titanic/star_control/camera_auto_mover.h
@@ -59,14 +59,15 @@ public:
 	CCameraAutoMover();
 	virtual ~CCameraAutoMover() {}
 
-	virtual void setPath2(const FVector &oldPos, const FVector &newPos,
-		const FMatrix &oldOrientation, const FMatrix &newOrientation);
-
 	/**
 	 * Clear src and dest orientation and set some default values for other fields
 	 */	
 	virtual void setOrientations(const FMatrix &srcOrient, const FMatrix &destOrient);
-	virtual void setPath(const FVector &srcV, const FVector &destV, const FMatrix &orientation);
+
+	/**
+	 * Setup a transition to from one position to another
+	 */		
+	void setPath(const FVector &srcV, const FVector &destV);
 
 	/**
 	 * Applys speeds to the mover. More than one application is usually done for several transitions
diff --git a/engines/titanic/star_control/marked_auto_mover.cpp b/engines/titanic/star_control/marked_auto_mover.cpp
index 0787c48de3..ae987aa8fc 100644
--- a/engines/titanic/star_control/marked_auto_mover.cpp
+++ b/engines/titanic/star_control/marked_auto_mover.cpp
@@ -26,9 +26,9 @@
 
 namespace Titanic {
 
-void CMarkedAutoMover::setPath2(const FVector &oldPos, const FVector &newPos,
+void CMarkedAutoMover::setPathOrients(const FVector &oldPos, const FVector &newPos,
 	const FMatrix &oldOrientation, const FMatrix &newOrientation) {
-	CCameraAutoMover::setPath2(oldPos, newPos, oldOrientation, newOrientation);
+	CCameraAutoMover::setPath(oldPos, newPos);
 
 	double distance = _distance;
 	_active = true;
diff --git a/engines/titanic/star_control/marked_auto_mover.h b/engines/titanic/star_control/marked_auto_mover.h
index 856df99c03..ca7fbf3b7f 100644
--- a/engines/titanic/star_control/marked_auto_mover.h
+++ b/engines/titanic/star_control/marked_auto_mover.h
@@ -41,7 +41,7 @@ private:
 public:
 	virtual ~CMarkedAutoMover() {}
 
-	virtual void setPath2(const FVector &oldPos, const FVector &newPos,
+	void setPathOrients(const FVector &oldPos, const FVector &newPos,
 		const FMatrix &oldOrientation, const FMatrix &newOrientation);
 
 	/**
diff --git a/engines/titanic/star_control/marked_camera_mover.cpp b/engines/titanic/star_control/marked_camera_mover.cpp
index ff9c055abb..df6edbec5f 100644
--- a/engines/titanic/star_control/marked_camera_mover.cpp
+++ b/engines/titanic/star_control/marked_camera_mover.cpp
@@ -32,13 +32,12 @@ CMarkedCameraMover::CMarkedCameraMover(const CNavigationInfo *src) :
 		CCameraMover(src) {
 }
 
-
 void CMarkedCameraMover::transitionBetweenPosOrients(const FVector &oldPos, const FVector &newPos,
 		const FMatrix &oldOrientation, const FMatrix &newOrientation) {
 	if (isLocked())
 		decLockCount();
 
-	_autoMover.setPath2(oldPos, newPos, oldOrientation, newOrientation);
+	_autoMover.setPathOrients(oldPos, newPos, oldOrientation, newOrientation);
 	incLockCount();
 }
 
diff --git a/engines/titanic/star_control/unmarked_auto_mover.cpp b/engines/titanic/star_control/unmarked_auto_mover.cpp
index c84fbbab00..1120690b0e 100644
--- a/engines/titanic/star_control/unmarked_auto_mover.cpp
+++ b/engines/titanic/star_control/unmarked_auto_mover.cpp
@@ -36,8 +36,8 @@ void CUnmarkedAutoMover::setOrientations(const FMatrix &srcOrient, const FMatrix
 	_active = true;
 }
 
-void CUnmarkedAutoMover::setPath(const FVector &srcV, const FVector &destV, const FMatrix &orientation) {
-	CCameraAutoMover::setPath(srcV, destV, orientation);
+void CUnmarkedAutoMover::setPathOrient(const FVector &srcV, const FVector &destV, const FMatrix &orientation) {
+	CCameraAutoMover::setPath(srcV, destV);
 
 	if (_distance > 8000.0) {
 		_active = true;
diff --git a/engines/titanic/star_control/unmarked_auto_mover.h b/engines/titanic/star_control/unmarked_auto_mover.h
index b7fb4e3c66..41c13311e0 100644
--- a/engines/titanic/star_control/unmarked_auto_mover.h
+++ b/engines/titanic/star_control/unmarked_auto_mover.h
@@ -37,9 +37,9 @@ public:
 	virtual void setOrientations(const FMatrix &srcOrient, const FMatrix &destOrient);
 
 	/**
-	 * Sets the path to animate movement between
+	 * Sets the path and starting and ending orientations to animate movement between
 	 */
-	virtual void setPath(const FVector &srcV, const FVector &destV, const FMatrix &orientation);
+	void setPathOrient(const FVector &srcV, const FVector &destV, const FMatrix &orientation);
 
 	virtual MoverState move(CErrorCode &errorCode, FVector &pos, FMatrix &orientation);
 };
diff --git a/engines/titanic/star_control/unmarked_camera_mover.cpp b/engines/titanic/star_control/unmarked_camera_mover.cpp
index 200d549ce1..c879dc25e8 100644
--- a/engines/titanic/star_control/unmarked_camera_mover.cpp
+++ b/engines/titanic/star_control/unmarked_camera_mover.cpp
@@ -41,7 +41,7 @@ void CUnmarkedCameraMover::moveTo(const FVector &srcV, const FVector &destV, con
 
 	debugC(DEBUG_BASIC, kDebugStarfield, "Starfield move %s to %s", srcV.toString().c_str(),
 		destV.toString().c_str());
-	_autoMover.setPath(srcV, destV, orientation);
+	_autoMover.setPathOrient(srcV, destV, orientation);
 }
 
 // TODO: v3 is unused
-- 
cgit v1.2.3


From 4bdea384c30586a861ccbc102039258faafe3d90 Mon Sep 17 00:00:00 2001
From: David Fioramonti
Date: Thu, 7 Sep 2017 05:16:26 -0700
Subject: TITANIC: make sure of setOrientations more clear

Made camera automover setOrientations not virtual and reduced
arguments also changed name since to differentiate it from
behavior of derived classes.
---
 engines/titanic/star_control/camera_auto_mover.cpp   | 3 +--
 engines/titanic/star_control/camera_auto_mover.h     | 2 +-
 engines/titanic/star_control/unmarked_auto_mover.cpp | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

(limited to 'engines')

diff --git a/engines/titanic/star_control/camera_auto_mover.cpp b/engines/titanic/star_control/camera_auto_mover.cpp
index 1b016bc549..71f7de85b2 100644
--- a/engines/titanic/star_control/camera_auto_mover.cpp
+++ b/engines/titanic/star_control/camera_auto_mover.cpp
@@ -41,8 +41,7 @@ CCameraAutoMover::CCameraAutoMover() : _srcPos(0.0, 1000000.0, 0.0) {
 	_transitionPercentInc = 0.0;
 }
 
-// TODO: same as proc2 also orientations are not used
-void CCameraAutoMover::setOrientations(const FMatrix &srcOrient, const FMatrix &destOrient) {
+void CCameraAutoMover::clear() {
 	_srcPos.clear();
 	_destPos.clear();
 	_transitionPercent = 1.0;
diff --git a/engines/titanic/star_control/camera_auto_mover.h b/engines/titanic/star_control/camera_auto_mover.h
index d83f15013c..9b7eb1b7ce 100644
--- a/engines/titanic/star_control/camera_auto_mover.h
+++ b/engines/titanic/star_control/camera_auto_mover.h
@@ -62,7 +62,7 @@ public:
 	/**
 	 * Clear src and dest orientation and set some default values for other fields
 	 */	
-	virtual void setOrientations(const FMatrix &srcOrient, const FMatrix &destOrient);
+	void clear();
 
 	/**
 	 * Setup a transition to from one position to another
diff --git a/engines/titanic/star_control/unmarked_auto_mover.cpp b/engines/titanic/star_control/unmarked_auto_mover.cpp
index 1120690b0e..b8cd042e9c 100644
--- a/engines/titanic/star_control/unmarked_auto_mover.cpp
+++ b/engines/titanic/star_control/unmarked_auto_mover.cpp
@@ -28,7 +28,7 @@
 namespace Titanic {
 
 void CUnmarkedAutoMover::setOrientations(const FMatrix &srcOrient, const FMatrix &destOrient) {
-	CCameraAutoMover::setOrientations(srcOrient, destOrient);
+	CCameraAutoMover::clear();
 	_orientationChanger.load(srcOrient, destOrient);
 	_transitionPercentInc = 0.1;
 	_transitionPercent = 0.0;
-- 
cgit v1.2.3


From d3a0a487be01f14e8a81f2d82fd4d78f203607e2 Mon Sep 17 00:00:00 2001
From: David Fioramonti
Date: Thu, 7 Sep 2017 04:27:50 -0700
Subject: TITANIC: star control, removed todos and improved some comments

---
 engines/titanic/star_control/star_camera.cpp | 9 ++++++---
 engines/titanic/star_control/star_camera.h   | 2 +-
 engines/titanic/star_control/viewport.cpp    | 2 +-
 3 files changed, 8 insertions(+), 5 deletions(-)

(limited to 'engines')

diff --git a/engines/titanic/star_control/star_camera.cpp b/engines/titanic/star_control/star_camera.cpp
index 4edc05bbf7..bcfc9ea232 100644
--- a/engines/titanic/star_control/star_camera.cpp
+++ b/engines/titanic/star_control/star_camera.cpp
@@ -391,7 +391,8 @@ void CStarCamera::setViewportAngle(const FPoint &angles) {
 		break;
 	}
 
-	// TODO: should three stars locked do anything in this function? Error?
+	// All three stars are locked on in this case so the camera does not move
+	// in response to the users mouse movements
 	case THREE_LOCKED:
 		break;
 	}
@@ -505,7 +506,9 @@ bool CStarCamera::lockMarker1(FVector v1, FVector firstStarPosition, FVector v3)
 
 	FMatrix matrix = _viewport.getOrientation();
 	const FVector &pos = _viewport._position;
-	_mover->transitionBetweenOrientations(v3, tempV, pos, matrix); // TODO: pos does not get used in this function
+	_mover->transitionBetweenOrientations(v3, tempV, pos, matrix); // TODO: pos does not get used in this function, 
+																// i.e., _mover has CUnmarkedCameraMover handle which means
+																// CUnmarkedCameraMover::transitionBetweenOrientations gets called
 
 	CStarVector *sv = new CStarVector(this, firstStarPosition);
 	_mover->setVector(sv);
@@ -613,7 +616,7 @@ bool CStarCamera::lockMarker3(CViewport *viewport, const FVector &thirdStarPosit
 	FMatrix newOr = viewport->getOrientation();
 	FMatrix oldOr = _viewport.getOrientation();
 	FVector newPos = viewport->_position;
-	FVector oldPos = _viewport._position;
+	//FVector oldPos = _viewport._position;
 
 	// WORKAROUND: set old position to new position (1st argument), this prevents 
 	// locking issues when locking the 3rd star. Fixes #9961.
diff --git a/engines/titanic/star_control/star_camera.h b/engines/titanic/star_control/star_camera.h
index 3ffb74950a..281abb9d30 100644
--- a/engines/titanic/star_control/star_camera.h
+++ b/engines/titanic/star_control/star_camera.h
@@ -49,7 +49,7 @@ private:
 	FMatrix _lockedStarsPos; // Each row represents the location of a locked star
 	CCameraMover *_mover;
 	CViewport _viewport;
-	bool _isMoved; // TODO: determine if this is being used
+	bool _isMoved; // Used in CPetStarfield to determine if a star destination can be set
 	bool _isInLockingProcess; // The mover/view is homing in on a new star
 private:
 	/**
diff --git a/engines/titanic/star_control/viewport.cpp b/engines/titanic/star_control/viewport.cpp
index 07a6dcc36c..e368dfa317 100644
--- a/engines/titanic/star_control/viewport.cpp
+++ b/engines/titanic/star_control/viewport.cpp
@@ -260,7 +260,7 @@ FVector CViewport::getRelativePosCentering(int index, const FVector &src) {
 	return dest;
 }
 
-// TODO: Identical to getRelativePosCentering, was this meant to be different?
+// Similar to getRelativePosCentering, but uses the raw/transpose version of Pose
 FVector CViewport::getRelativePosCenteringRaw(int index, const FVector &src) {
 	FVector dest;
 	FPose pose = getRawPose();
-- 
cgit v1.2.3


From 93d9ac926c466f39670073f60166630c1fd65539 Mon Sep 17 00:00:00 2001
From: David Fioramonti
Date: Thu, 7 Sep 2017 05:51:16 -0700
Subject: TITANIC: star camera better handler function naming

The functions that dealt with the mover handling only
had handler in the name so I added mover and type to the
name to reflect that it involves the mover handler.
---
 engines/titanic/star_control/star_camera.cpp | 18 +++++++++---------
 engines/titanic/star_control/star_camera.h   | 12 +++++++-----
 2 files changed, 16 insertions(+), 14 deletions(-)

(limited to 'engines')

diff --git a/engines/titanic/star_control/star_camera.cpp b/engines/titanic/star_control/star_camera.cpp
index bcfc9ea232..e4fcdcdead 100644
--- a/engines/titanic/star_control/star_camera.cpp
+++ b/engines/titanic/star_control/star_camera.cpp
@@ -41,7 +41,7 @@ FMatrix *CStarCamera::_newOrientation;
 
 CStarCamera::CStarCamera(const CNavigationInfo *data) :
 		_starLockState(ZERO_LOCKED), _mover(nullptr), _isMoved(false), _isInLockingProcess(false) {
-	setupHandler(data);
+	setMoverType(data);
 }
 
 CStarCamera::CStarCamera(CViewport *src) :
@@ -69,7 +69,7 @@ bool CStarCamera::isNotInLockingProcess() {
 }
 
 CStarCamera::~CStarCamera() {
-	deleteHandler();
+	removeMover();
 }
 
 void CStarCamera::proc2(const CViewport *src) {
@@ -404,12 +404,12 @@ bool CStarCamera::addLockedStar(const FVector v) {
 
 	CNavigationInfo data;
 	_mover->copyTo(&data);
-	deleteHandler();
+	removeMover();
 
 	FVector &row = _lockedStarsPos[(int)_starLockState];
 	_starLockState = StarLockState((int)_starLockState + 1);
 	row = v;
-	setupHandler(&data);
+	setMoverType(&data);
 	return true;
 }
 
@@ -419,10 +419,10 @@ bool CStarCamera::removeLockedStar() {
 
 	CNavigationInfo data;
 	_mover->copyTo(&data);
-	deleteHandler();
+	removeMover();
 
 	_starLockState = StarLockState((int)_starLockState - 1);
-	setupHandler(&data);
+	setMoverType(&data);
 	return true;
 }
 
@@ -438,7 +438,7 @@ void CStarCamera::save(SimpleFile *file, int indent) {
 	_viewport.save(file, indent);
 }
 
-bool CStarCamera::setupHandler(const CNavigationInfo *src) {
+bool CStarCamera::setMoverType(const CNavigationInfo *src) {
 	CCameraMover *mover = nullptr;
 
 	switch (_starLockState) {
@@ -457,7 +457,7 @@ bool CStarCamera::setupHandler(const CNavigationInfo *src) {
 	}
 
 	if (mover) {
-		assert(!_mover);
+		assert(!_mover); // removeMover() is usually called before this function so _mover is null
 		_mover = mover;
 		return true;
 	} else {
@@ -465,7 +465,7 @@ bool CStarCamera::setupHandler(const CNavigationInfo *src) {
 	}
 }
 
-void CStarCamera::deleteHandler() {
+void CStarCamera::removeMover() {
 	if (_mover) {
 		delete _mover;
 		_mover = nullptr;
diff --git a/engines/titanic/star_control/star_camera.h b/engines/titanic/star_control/star_camera.h
index 281abb9d30..f2d27212fe 100644
--- a/engines/titanic/star_control/star_camera.h
+++ b/engines/titanic/star_control/star_camera.h
@@ -47,20 +47,22 @@ private:
 private:
 	StarLockState _starLockState;
 	FMatrix _lockedStarsPos; // Each row represents the location of a locked star
-	CCameraMover *_mover;
+	CCameraMover *_mover; // A marked or unmarked camera mover, contains an automover
 	CViewport _viewport;
 	bool _isMoved; // Used in CPetStarfield to determine if a star destination can be set
 	bool _isInLockingProcess; // The mover/view is homing in on a new star
 private:
 	/**
-	 * Set up a handler
+	 * Set Mover type to be unmarked or marked camera mover based on 
+	 * the number of stars currently locked (_starLockState)
+	 * The CNavigationInfo data is used to initialize the mover
 	 */
-	bool setupHandler(const CNavigationInfo *src);
+	bool setMoverType(const CNavigationInfo *src);
 
 	/**
-	 * Deletes any previous handler
+	 * Deletes the previous mover handle
 	 */
-	void deleteHandler();
+	void removeMover();
 
 	/**
 	 * Return whether the handler is locked
-- 
cgit v1.2.3