diff --git a/CHANGELOG.md b/CHANGELOG.md index 9eb66475bb..55f2185763 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ Bug #7042: Weapon follow animations that immediately follow the hit animations cause multiple hits Bug #7044: Changing a class' services does not affect autocalculated NPCs Bug #7084: Resurrecting an actor doesn't take into account base record changes + Bug #7121: Crash on TimeStamp construction with invalid hour value Feature #6447: Add LOD support to Object Paging Feature #6933: Support high-resolution cursor textures Feature #6945: Support S3TC-compressed and BGR/BGRA NiPixelData diff --git a/apps/openmw/mwworld/datetimemanager.cpp b/apps/openmw/mwworld/datetimemanager.cpp index 2d137518cf..f2dd55d9dd 100644 --- a/apps/openmw/mwworld/datetimemanager.cpp +++ b/apps/openmw/mwworld/datetimemanager.cpp @@ -3,6 +3,7 @@ #include "../mwbase/environment.hpp" #include "../mwbase/world.hpp" +#include "duration.hpp" #include "esmstore.hpp" #include "globals.hpp" #include "timestamp.hpp" @@ -60,11 +61,11 @@ namespace MWWorld if (hour < 0) hour = 0; - int days = static_cast(hour / 24); - hour = std::fmod(hour, 24); - mGameHour = static_cast(hour); + const Duration duration = Duration::fromHours(hour); - if (days > 0) + mGameHour = duration.getHours(); + + if (const int days = duration.getDays(); days > 0) setDay(days + mDay); } diff --git a/apps/openmw/mwworld/duration.hpp b/apps/openmw/mwworld/duration.hpp new file mode 100644 index 0000000000..78693ca0dd --- /dev/null +++ b/apps/openmw/mwworld/duration.hpp @@ -0,0 +1,39 @@ +#ifndef GAME_MWWORLD_DURATION_H +#define GAME_MWWORLD_DURATION_H + +#include +#include + +namespace MWWorld +{ + inline const double maxFloatHour = static_cast(std::nextafter(24.0f, 0.0f)); + + class Duration + { + public: + static Duration fromHours(double hours) + { + if (hours < 0) + throw std::runtime_error("Negative hours is not supported Duration"); + + return Duration( + static_cast(hours / 24), static_cast(std::min(std::fmod(hours, 24), maxFloatHour))); + } + + int getDays() const { return mDays; } + + float getHours() const { return mHours; } + + private: + int mDays; + float mHours; + + explicit Duration(int days, float hours) + : mDays(days) + , mHours(hours) + { + } + }; +} + +#endif diff --git a/apps/openmw/mwworld/timestamp.cpp b/apps/openmw/mwworld/timestamp.cpp index 22fa504d45..c21d90f285 100644 --- a/apps/openmw/mwworld/timestamp.cpp +++ b/apps/openmw/mwworld/timestamp.cpp @@ -6,6 +6,8 @@ #include +#include "duration.hpp" + namespace MWWorld { TimeStamp::TimeStamp(float hour, int day) @@ -31,11 +33,10 @@ namespace MWWorld if (hours < 0) throw std::runtime_error("can't move time stamp backwards in time"); - hours += mHour; + const Duration duration = Duration::fromHours(mHour + hours); - mHour = static_cast(std::fmod(hours, 24)); - - mDay += static_cast(hours / 24); + mHour = duration.getHours(); + mDay += duration.getDays(); return *this; } diff --git a/apps/openmw_test_suite/CMakeLists.txt b/apps/openmw_test_suite/CMakeLists.txt index 81f6fadabe..cf1be3a18a 100644 --- a/apps/openmw_test_suite/CMakeLists.txt +++ b/apps/openmw_test_suite/CMakeLists.txt @@ -11,7 +11,11 @@ file(GLOB UNITTEST_SRC_FILES ../openmw/mwworld/store.cpp ../openmw/mwworld/esmstore.cpp + ../openmw/mwworld/timestamp.cpp + mwworld/test_store.cpp + mwworld/testduration.cpp + mwworld/testtimestamp.cpp mwdialogue/test_keywordsearch.cpp diff --git a/apps/openmw_test_suite/mwworld/testduration.cpp b/apps/openmw_test_suite/mwworld/testduration.cpp new file mode 100644 index 0000000000..ce48397403 --- /dev/null +++ b/apps/openmw_test_suite/mwworld/testduration.cpp @@ -0,0 +1,63 @@ +#include + +#include + +#include "apps/openmw/mwworld/duration.hpp" + +namespace MWWorld +{ + namespace + { + TEST(MWWorldDurationTest, fromHoursShouldProduceZeroDaysAndHoursFor0) + { + const Duration duration = Duration::fromHours(0); + EXPECT_EQ(duration.getDays(), 0); + EXPECT_EQ(duration.getHours(), 0); + } + + TEST(MWWorldDurationTest, fromHoursShouldProduceOneDayAndZeroHoursFor24) + { + const Duration duration = Duration::fromHours(24); + EXPECT_EQ(duration.getDays(), 1); + EXPECT_EQ(duration.getHours(), 0); + } + + TEST(MWWorldDurationTest, fromHoursShouldProduceOneDayAndRemainderHoursFor42) + { + const Duration duration = Duration::fromHours(42); + EXPECT_EQ(duration.getDays(), 1); + EXPECT_EQ(duration.getHours(), 18); + } + + TEST(MWWorldDurationTest, fromHoursShouldProduceZeroDaysAndZeroHoursForMinDouble) + { + const Duration duration = Duration::fromHours(std::numeric_limits::min()); + EXPECT_EQ(duration.getDays(), 0); + EXPECT_EQ(duration.getHours(), 0); + } + + TEST(MWWorldDurationTest, fromHoursShouldProduceZeroDaysAndSomeHoursForMinFloat) + { + const Duration duration = Duration::fromHours(std::numeric_limits::min()); + EXPECT_EQ(duration.getDays(), 0); + EXPECT_GT(duration.getHours(), 0); + EXPECT_FLOAT_EQ(duration.getHours(), std::numeric_limits::min()); + } + + TEST(MWWorldDurationTest, fromHoursShouldProduceZeroDaysAndRemainderHoursForValueJustBelow24InDoublePrecision) + { + const Duration duration = Duration::fromHours(std::nextafter(24.0, 0.0)); + EXPECT_EQ(duration.getDays(), 0); + EXPECT_LT(duration.getHours(), 24); + EXPECT_FLOAT_EQ(duration.getHours(), 24); + } + + TEST(MWWorldDurationTest, fromHoursShouldProduceZeroDaysAndRemainderHoursForValueJustBelow24InFloatPrecision) + { + const Duration duration = Duration::fromHours(std::nextafter(24.0f, 0.0f)); + EXPECT_EQ(duration.getDays(), 0); + EXPECT_LT(duration.getHours(), 24); + EXPECT_FLOAT_EQ(duration.getHours(), 24); + } + } +} diff --git a/apps/openmw_test_suite/mwworld/testtimestamp.cpp b/apps/openmw_test_suite/mwworld/testtimestamp.cpp new file mode 100644 index 0000000000..8a1a4c2aa8 --- /dev/null +++ b/apps/openmw_test_suite/mwworld/testtimestamp.cpp @@ -0,0 +1,67 @@ +#include + +#include + +#include "apps/openmw/mwworld/timestamp.hpp" + +namespace MWWorld +{ + namespace + { + TEST(MWWorldTimeStampTest, operatorPlusShouldNotChangeTimeStampForZero) + { + TimeStamp timeStamp; + timeStamp += 0; + EXPECT_EQ(timeStamp.getDay(), 0); + EXPECT_EQ(timeStamp.getHour(), 0); + } + + TEST(MWWorldTimeStampTest, operatorPlusShouldProperlyHandleDoubleValuesCloseTo24) + { + TimeStamp timeStamp; + timeStamp += std::nextafter(24.0, 0.0); + EXPECT_EQ(timeStamp.getDay(), 0); + EXPECT_LT(timeStamp.getHour(), 24); + EXPECT_FLOAT_EQ(timeStamp.getHour(), 24); + } + + TEST(MWWorldTimeStampTest, operatorPlusShouldProperlyHandleFloatValuesCloseTo24) + { + TimeStamp timeStamp; + timeStamp += std::nextafter(24.0f, 0.0f); + EXPECT_EQ(timeStamp.getDay(), 0); + EXPECT_LT(timeStamp.getHour(), 24); + EXPECT_FLOAT_EQ(timeStamp.getHour(), 24); + } + + TEST(MWWorldTimeStampTest, operatorPlusShouldAddDaysForEach24Hours) + { + TimeStamp timeStamp; + timeStamp += 24.0 * 42; + EXPECT_EQ(timeStamp.getDay(), 42); + EXPECT_EQ(timeStamp.getHour(), 0); + } + + TEST(MWWorldTimeStampTest, operatorPlusShouldAddDaysForEach24HoursAndSetRemainderToHours) + { + TimeStamp timeStamp; + timeStamp += 24.0 * 42 + 13.0; + EXPECT_EQ(timeStamp.getDay(), 42); + EXPECT_EQ(timeStamp.getHour(), 13); + } + + TEST(MWWorldTimeStampTest, operatorPlusShouldAccumulateExistingValue) + { + TimeStamp timeStamp(13, 42); + timeStamp += 24.0 * 2 + 17.0; + EXPECT_EQ(timeStamp.getDay(), 45); + EXPECT_EQ(timeStamp.getHour(), 6); + } + + TEST(MWWorldTimeStampTest, operatorPlusShouldThrowExceptionForNegativeValue) + { + TimeStamp timeStamp(13, 42); + EXPECT_THROW(timeStamp += -1, std::runtime_error); + } + } +}