From 1047e408bda059c10b38f90ceb9fef09dd477bb0 Mon Sep 17 00:00:00 2001 From: Andrew James Date: Tue, 31 May 2022 14:58:47 +1000 Subject: [PATCH] Simplify logic of UpdateMissilePos using helpers from 4620 (#4621) --- Source/engine/displacement.hpp | 49 +++++++++++++++++++++++++++++++++- Source/lighting.cpp | 8 +++--- Source/lighting.h | 4 +-- Source/loadsave.cpp | 8 +++--- Source/missiles.cpp | 23 +++++++--------- Source/player.cpp | 4 +-- Source/scrollrt.cpp | 4 +-- test/CMakeLists.txt | 1 + test/math_test.cpp | 31 +++++++++++++++++++++ 9 files changed, 104 insertions(+), 28 deletions(-) create mode 100644 test/math_test.cpp diff --git a/Source/engine/displacement.hpp b/Source/engine/displacement.hpp index 0a0c29da5..7aac6dbda 100644 --- a/Source/engine/displacement.hpp +++ b/Source/engine/displacement.hpp @@ -1,6 +1,9 @@ #pragma once #include +#ifdef BUILD_TESTING +#include +#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) { diff --git a/Source/lighting.cpp b/Source/lighting.cpp index 5699c2a5b..5354220a8 100644 --- a/Source/lighting.cpp +++ b/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; } diff --git a/Source/lighting.h b/Source/lighting.h index daadbcccd..a4a80ca58 100644 --- a/Source/lighting.h +++ b/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(); diff --git a/Source/loadsave.cpp b/Source/loadsave.cpp index 10d6f0cce..e7a975d11 100644 --- a/Source/loadsave.cpp +++ b/Source/loadsave.cpp @@ -860,8 +860,8 @@ void LoadLighting(LoadHelper *file, Light *pLight) pLight->position.old.x = file->NextLE(); pLight->position.old.y = file->NextLE(); pLight->oldRadius = file->NextLE(); - pLight->position.offset.x = file->NextLE(); - pLight->position.offset.y = file->NextLE(); + pLight->position.offset.deltaX = file->NextLE(); + pLight->position.offset.deltaY = file->NextLE(); pLight->_lflags = file->NextBool32(); } @@ -1571,8 +1571,8 @@ void SaveLighting(SaveHelper *file, Light *pLight) file->WriteLE(pLight->position.old.x); file->WriteLE(pLight->position.old.y); file->WriteLE(pLight->oldRadius); - file->WriteLE(pLight->position.offset.x); - file->WriteLE(pLight->position.offset.y); + file->WriteLE(pLight->position.offset.deltaX); + file->WriteLE(pLight->position.offset.deltaY); file->WriteLE(pLight->_lflags ? 1 : 0); } diff --git a/Source/missiles.cpp b/Source/missiles.cpp index 9affd563f..6f18db9d2 100644 --- a/Source/missiles.cpp +++ b/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(); } } diff --git a/Source/player.cpp b/Source/player.cpp index bfc53ace8..1ee27fe7b 100644 --- a/Source/player.cpp +++ b/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; diff --git a/Source/scrollrt.cpp b/Source/scrollrt.cpp index b9ad2d2c7..09dcf3b49 100644 --- a/Source/scrollrt.cpp +++ b/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; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 749e6d6e8..2405adc39 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -24,6 +24,7 @@ set(tests file_util_test inv_test lighting_test + math_test missiles_test pack_test path_test diff --git a/test/math_test.cpp b/test/math_test.cpp new file mode 100644 index 000000000..8c914de72 --- /dev/null +++ b/test/math_test.cpp @@ -0,0 +1,31 @@ +#include + +#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