From 999f44cf0745d7dad05f53043fb80aaefe5c4c69 Mon Sep 17 00:00:00 2001 From: Gleb Mazovetskiy Date: Fri, 11 Aug 2023 11:23:13 +0900 Subject: [PATCH] Point: Safer `operator-` 1. Require both operands to have the same signedness to avoid surprises. 2. Only allow unary negation on signed points. Fixes #6409 Also fixes a potential bug in the sound position calculation. --- Source/engine/point.hpp | 2 ++ Source/engine/sound_position.cpp | 9 ++++----- Source/missiles.cpp | 6 +++--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/Source/engine/point.hpp b/Source/engine/point.hpp index ff809bbfa..e336edeaf 100644 --- a/Source/engine/point.hpp +++ b/Source/engine/point.hpp @@ -90,6 +90,7 @@ struct PointOf { constexpr PointOf operator-() const { + static_assert(std::is_signed::value, "CoordT must be signed"); return { -x, -y }; } @@ -193,6 +194,7 @@ constexpr PointOf operator+(PointOf a, Direction direc template constexpr DisplacementOf operator-(PointOf a, PointOf b) { + static_assert(std::is_signed::value == std::is_signed::value, "points must have the same signedness"); return { static_cast(a.x - b.x), static_cast(a.y - b.y) }; } diff --git a/Source/engine/sound_position.cpp b/Source/engine/sound_position.cpp index e2ab755ec..088890a13 100644 --- a/Source/engine/sound_position.cpp +++ b/Source/engine/sound_position.cpp @@ -7,14 +7,13 @@ namespace devilution { bool CalculateSoundPosition(Point soundPosition, int *plVolume, int *plPan) { - const auto &playerPosition = MyPlayer->position.tile; - const auto delta = soundPosition - playerPosition; + const Point playerPosition { MyPlayer->position.tile }; + const Displacement delta = soundPosition - playerPosition; - int pan = (delta.deltaX - delta.deltaY) * 256; + const int pan = (delta.deltaX - delta.deltaY) * 256; *plPan = clamp(pan, PAN_MIN, PAN_MAX); - int volume = playerPosition.ApproxDistance(soundPosition); - volume *= -64; + const int volume = playerPosition.ApproxDistance(soundPosition) * -64; if (volume <= ATTENUATION_MIN) return false; diff --git a/Source/missiles.cpp b/Source/missiles.cpp index 837344c31..a5dfe4a95 100644 --- a/Source/missiles.cpp +++ b/Source/missiles.cpp @@ -1474,10 +1474,10 @@ void AddWarp(Missile &missile, AddMissileParameter ¶meter) } app_fatal(StrCat("invalid leveltype", static_cast(leveltype))); }; - Displacement triggerOffset = getTriggerOffset(trg); + const Displacement triggerOffset = getTriggerOffset(trg); candidate += triggerOffset; - Displacement off = player.position.tile - candidate; - int distanceSq = off.deltaY * off.deltaY + off.deltaX * off.deltaX; + const Displacement off = Point { player.position.tile } - candidate; + const int distanceSq = off.deltaY * off.deltaY + off.deltaX * off.deltaX; if (distanceSq < minDistanceSq) { minDistanceSq = distanceSq; tile = candidate;