Simplify logic of UpdateMissilePos using helpers from 4620 (#4621)

Source/engine/displacement.hpp

@@ -1,6 +1,9 @@
 #pragma once
 
 #include <cmath>
+#ifdef BUILD_TESTING
+#include <ostream>
+#endif
 
 #include "direction.hpp"
 #include "size.hpp"
@@ -120,11 +123,42 @@ struct Displacement {
 	 *
 	 * @return A representation of the original displacement in screen coordinates.
 	 */
-	constexpr Displacement WorldToScreen() const
+	constexpr Displacement worldToScreen() const
 	{
 		return { (deltaY - deltaX) * 32, (deltaY + deltaX) * -16 };
 	}
 
+	/**
+	 * @brief Returns a new Displacement object in world coordinates.
+	 *
+	 * This is an inverse matrix of the worldToScreen transformation.
+	 *
+	 * @return A representation of the original displacement in world coordinates.
+	 */
+	constexpr Displacement screenToWorld() const
+	{
+		return { (2 * deltaY + deltaX) / -64, (2 * deltaY - deltaX) / -64 };
+	}
+
+	/**
+	 * @brief Missiles flip the axes for some reason -_-
+	 * @return negated world displacement, for use with missile movement routines.
+	 */
+	constexpr Displacement screenToMissile() const
+	{
+		return -screenToWorld();
+	}
+
+	constexpr Displacement screenToLight() const
+	{
+		return { (2 * deltaY + deltaX) / 8, (2 * deltaY - deltaX) / 8 };
+	}
+
+	constexpr Displacement operator>>(size_t factor)
+	{
+		return Displacement(deltaX >> factor, deltaY >> factor);
+	}
+
 	constexpr Displacement Rotate(int quadrants)
 	{
 		constexpr int Sines[] = { 0, 1, 0, -1 };
@@ -137,6 +171,19 @@ struct Displacement {
 		return Displacement { deltaX * cosine - deltaY * sine, deltaX * sine + deltaY * cosine };
 	}
 
+#ifdef BUILD_TESTING
+	/**
+	 * @brief Format displacements nicely in test failure messages
+	 * @param stream output stream, expected to have overloads for int and char*
+	 * @param offset Object to display
+	 * @return the stream, to allow chaining
+	 */
+	friend std::ostream &operator<<(std::ostream &stream, const Displacement &offset)
+	{
+		return stream << "(x: " << offset.deltaX << ", y: " << offset.deltaY << ")";
+	}
+#endif
+
 private:
 	static constexpr Displacement fromDirection(Direction direction)
 	{

Source/lighting.cpp

@@ -532,8 +532,8 @@ void DoLighting(Point position, int nRadius, int lnum)
 
 	if (lnum >= 0) {
 		Light &light = Lights[lnum];
-		xoff = light.position.offset.x;
-		yoff = light.position.offset.y;
+		xoff = light.position.offset.deltaX;
+		yoff = light.position.offset.deltaY;
 		if (xoff < 0) {
 			xoff += 8;
 			position -= { 1, 0 };
@@ -975,7 +975,7 @@ void ChangeLightXY(int i, Point position)
 	UpdateLighting = true;
 }
 
-void ChangeLightOffset(int i, Point position)
+void ChangeLightOffset(int i, Displacement offset)
 {
 	if (DisableLighting || i == NO_LIGHT) {
 		return;
@@ -985,7 +985,7 @@ void ChangeLightOffset(int i, Point position)
 	light._lunflag = true;
 	light.position.old = light.position.tile;
 	light.oldRadius = light._lradius;
-	light.position.offset = position;
+	light.position.offset = offset;
 	UpdateLighting = true;
 }
 

Source/lighting.h

@@ -24,7 +24,7 @@ namespace devilution {
 struct LightPosition {
 	Point tile;
 	/** Pixel offset from tile. */
-	Point offset;
+	Displacement offset;
 	/** Prevous position. */
 	Point old;
 };
@@ -62,7 +62,7 @@ int AddLight(Point position, int r);
 void AddUnLight(int i);
 void ChangeLightRadius(int i, int r);
 void ChangeLightXY(int i, Point position);
-void ChangeLightOffset(int i, Point position);
+void ChangeLightOffset(int i, Displacement offset);
 void ChangeLight(int i, Point position, int r);
 void ProcessLightList();
 void SavePreLighting();

Source/loadsave.cpp

@@ -860,8 +860,8 @@ void LoadLighting(LoadHelper *file, Light *pLight)
 	pLight->position.old.x = file->NextLE<int32_t>();
 	pLight->position.old.y = file->NextLE<int32_t>();
 	pLight->oldRadius = file->NextLE<int32_t>();
-	pLight->position.offset.x = file->NextLE<int32_t>();
-	pLight->position.offset.y = file->NextLE<int32_t>();
+	pLight->position.offset.deltaX = file->NextLE<int32_t>();
+	pLight->position.offset.deltaY = file->NextLE<int32_t>();
 	pLight->_lflags = file->NextBool32();
 }
 
@@ -1571,8 +1571,8 @@ void SaveLighting(SaveHelper *file, Light *pLight)
 	file->WriteLE<int32_t>(pLight->position.old.x);
 	file->WriteLE<int32_t>(pLight->position.old.y);
 	file->WriteLE<int32_t>(pLight->oldRadius);
-	file->WriteLE<int32_t>(pLight->position.offset.x);
-	file->WriteLE<int32_t>(pLight->position.offset.y);
+	file->WriteLE<int32_t>(pLight->position.offset.deltaX);
+	file->WriteLE<int32_t>(pLight->position.offset.deltaY);
 	file->WriteLE<uint32_t>(pLight->_lflags ? 1 : 0);
 }
 

Source/missiles.cpp

@@ -141,18 +141,15 @@ void PutMissile(Missile &missile)
 
 void UpdateMissilePos(Missile &missile)
 {
-	int mx = missile.position.traveled.deltaX >> 16;
-	int my = missile.position.traveled.deltaY >> 16;
-	int dx = mx + 2 * my;
-	int dy = 2 * my - mx;
-	int lx = dx / 8;
-	dx = dx / 64;
-	int ly = dy / 8;
-	dy = dy / 64;
-	missile.position.tile = missile.position.start + Displacement { dx, dy };
-	missile.position.offset.deltaX = mx + (dy * 32) - (dx * 32);
-	missile.position.offset.deltaY = my - (dx * 16) - (dy * 16);
-	ChangeLightOffset(missile._mlid, { lx - (dx * 8), ly - (dy * 8) });
+	Displacement pixelsTravelled = missile.position.traveled >> 16;
+
+	Displacement tileOffset = pixelsTravelled.screenToMissile();
+	missile.position.tile = missile.position.start + tileOffset;
+
+	missile.position.offset = pixelsTravelled + tileOffset.worldToScreen();
+
+	Displacement absoluteLightOffset = pixelsTravelled.screenToLight();
+	ChangeLightOffset(missile._mlid, absoluteLightOffset - tileOffset * 8);
 }
 
 /**
@@ -185,7 +182,7 @@ void MoveMissilePos(Missile &missile)
 	auto target = missile.position.tile + moveDirection;
 	if (IsTileAvailable(Monsters[missile._misource], target)) {
 		missile.position.tile = target;
-		missile.position.offset += Displacement(moveDirection).WorldToScreen();
+		missile.position.offset += Displacement(moveDirection).worldToScreen();
 	}
 }
 

Source/player.cpp

@@ -206,8 +206,8 @@ void PmChangeLightOff(Player &player)
 	y = (y / 8) * (y < 0 ? 1 : -1);
 	int lx = x + (l->position.tile.x * 8);
 	int ly = y + (l->position.tile.y * 8);
-	int offx = l->position.offset.x + (l->position.tile.x * 8);
-	int offy = l->position.offset.y + (l->position.tile.y * 8);
+	int offx = l->position.offset.deltaX + (l->position.tile.x * 8);
+	int offy = l->position.offset.deltaY + (l->position.tile.y * 8);
 
 	if (abs(lx - offx) < 3 && abs(ly - offy) < 3)
 		return;

Source/scrollrt.cpp

@@ -616,8 +616,8 @@ void DrawObject(const Surface &out, Point tilePosition, Point targetBufferPositi
 	Point screenPosition = targetBufferPosition - Displacement { CalculateWidth2(objectToDraw._oAnimWidth), 0 };
 	if (objectToDraw.position != tilePosition) {
 		// drawing a large or offset object, calculate the correct position for the center of the sprite
-		Displacement screenOffset = objectToDraw.position - tilePosition;
-		screenPosition -= screenOffset.WorldToScreen();
+		Displacement worldOffset = objectToDraw.position - tilePosition;
+		screenPosition -= worldOffset.worldToScreen();
 	}
 
 	byte *pCelBuff = objectToDraw._oAnimData;

test/CMakeLists.txt

@@ -24,6 +24,7 @@ set(tests
   file_util_test
   inv_test
   lighting_test
+  math_test
   missiles_test
   pack_test
   path_test

test/math_test.cpp

@@ -0,0 +1,31 @@
+#include <gtest/gtest.h>
+
+#include "engine/displacement.hpp"
+
+namespace devilution {
+
+TEST(MathTest, WorldScreenTransformation)
+{
+	Displacement offset = { 5, 2 };
+	// Diablo renders tiles with the world origin translated to the top left of the screen, while the normal convention
+	// has the screen origin at the bottom left. This means that we end up with negative offsets in screen space for
+	// tiles in world space where x > y
+	EXPECT_EQ(offset.worldToScreen(), Displacement(-96, -112));
+
+	// Transformation should be reversable (as long as it's not truncating)
+	EXPECT_EQ(offset.worldToScreen().screenToWorld(), offset);
+
+	// Tiles with y >= x will still have a negative y coordinate in screen space
+	offset = { 2, 5 };
+	EXPECT_EQ(offset.worldToScreen(), Displacement(96, -112));
+
+	// Most screen to world transformations will have a further displacement applied, this is a simple case of
+	// selecting a tile on the edge of the world with the default origin
+	Displacement cursorPosition = { 342, -150 };
+	EXPECT_EQ(cursorPosition.screenToWorld(), Displacement(0, 10));
+
+	// Screen > World transforms lose information, so cannot be reversed exactly using ints
+	EXPECT_EQ(cursorPosition.screenToWorld().worldToScreen(), Displacement(320, -160));
+}
+
+} // namespace devilution