Fixes for a whole bunch of warnings

These warnings were always enabled, but we didn't see them due to https://gitlab.com/OpenMW/openmw/-/issues/7882.
I do not fully understand the cause of 7822 as I can't repro it in a minimal CMake project.

Some of these fixes are thought through.
Some are sensible best guesses.
Some are kind of a stab in the dark as I don't know whether there was a
possible bug the warning was telling me about that I've done nothing to
help by introducing a static_cast.

Nearly all of these warnings were about some kind of narrowing
conversion, so I'm not sure why they weren't firing with GCC and Clang,
which have -Wall -Wextra -pedantic set, which should imply -Wnarrowing,
and they can't have been affected by 7882.

There were also some warnings being triggered from Boost code.
The vast majority of library headers that do questionable things weren't
firing warnings off, but for some reason, /external:I wasn't putting
these Boost headers into external mode.

We need these warnings dealt with one way or another so we can switch
the default Windows CI from MSBuild (which doesn't do ccache) to Ninja
(which does).
I have the necessary magic for that on a branch, but the branch won't
build because of these warnings.

apps/openmw/mwbase/windowmanager.hpp

@@ -290,7 +290,7 @@ namespace MWBase
 
         virtual void setEnemy(const MWWorld::Ptr& enemy) = 0;
 
-        virtual int getMessagesCount() const = 0;
+        virtual std::size_t getMessagesCount() const = 0;
 
         virtual const Translation::Storage& getTranslationDataStorage() const = 0;
 

apps/openmw/mwgui/keyboardnavigation.cpp

@@ -246,12 +246,12 @@ namespace MWGui
 
         bool forward = (direction == D_Next || direction == D_Right || direction == D_Down);
 
-        int index = found - keyFocusList.begin();
+        auto index = found - keyFocusList.begin();
         index = forward ? (index + 1) : (index - 1);
         if (wrap)
             index = (index + keyFocusList.size()) % keyFocusList.size();
         else
-            index = std::clamp<int>(index, 0, keyFocusList.size() - 1);
+            index = std::clamp<decltype(index)>(index, 0, keyFocusList.size() - 1);
 
         MyGUI::Widget* next = keyFocusList[index];
         int vertdiff = next->getTop() - focus->getTop();

apps/openmw/mwgui/messagebox.cpp

@@ -28,7 +28,7 @@ namespace MWGui
         MessageBoxManager::clear();
     }
 
-    int MessageBoxManager::getMessagesCount()
+    std::size_t MessageBoxManager::getMessagesCount()
     {
         return mMessageBoxes.size();
     }

apps/openmw/mwgui/messagebox.hpp

@@ -29,7 +29,7 @@ namespace MWGui
             bool immediate = false, int defaultFocus = -1);
         bool isInteractiveMessageBox();
 
-        int getMessagesCount();
+        std::size_t getMessagesCount();
 
         const InteractiveMessageBox* getInteractiveMessageBox() const { return mInterMessageBoxe.get(); }
 

apps/openmw/mwgui/screenfader.cpp

@@ -97,8 +97,8 @@ namespace MWGui
             imageBox->setImageTexture(texturePath);
             const MyGUI::IntSize imageSize = imageBox->getImageSize();
             imageBox->setImageCoord(
-                MyGUI::IntCoord(texCoordOverride.left * imageSize.width, texCoordOverride.top * imageSize.height,
-                    texCoordOverride.width * imageSize.width, texCoordOverride.height * imageSize.height));
+                MyGUI::IntCoord(static_cast<int>(texCoordOverride.left * imageSize.width), static_cast<int>(texCoordOverride.top * imageSize.height),
+                    static_cast<int>(texCoordOverride.width * imageSize.width), static_cast<int>(texCoordOverride.height * imageSize.height)));
         }
     }
 

apps/openmw/mwgui/trainingwindow.cpp

@@ -189,8 +189,8 @@ namespace MWGui
         mProgressBar.setProgress(0, 2);
         mTimeAdvancer.run(2);
 
-        MWBase::Environment::get().getWindowManager()->fadeScreenOut(0.2);
-        MWBase::Environment::get().getWindowManager()->fadeScreenIn(0.2, false, 0.2);
+        MWBase::Environment::get().getWindowManager()->fadeScreenOut(0.2f);
+        MWBase::Environment::get().getWindowManager()->fadeScreenIn(0.2f, false, 0.2f);
     }
 
     void TrainingWindow::onTrainingProgressChanged(int cur, int total)

apps/openmw/mwgui/windowmanagerimp.cpp

@@ -1685,9 +1685,9 @@ namespace MWGui
         mHud->setEnemy(enemy);
     }
 
-    int WindowManager::getMessagesCount() const
+    std::size_t WindowManager::getMessagesCount() const
     {
-        int count = 0;
+        std::size_t count = 0;
         if (mMessageBoxManager)
             count = mMessageBoxManager->getMessagesCount();
 

apps/openmw/mwgui/windowmanagerimp.hpp

@@ -313,7 +313,7 @@ namespace MWGui
 
         void setEnemy(const MWWorld::Ptr& enemy) override;
 
-        int getMessagesCount() const override;
+        std::size_t getMessagesCount() const override;
 
         const Translation::Storage& getTranslationDataStorage() const override;
 

apps/openmw/mwinput/bindingsmanager.cpp

@@ -627,12 +627,12 @@ namespace MWInput
         return mInputBinder->detectingBindingState();
     }
 
-    void BindingsManager::mousePressed(const SDL_MouseButtonEvent& arg, int deviceID)
+    void BindingsManager::mousePressed(const SDL_MouseButtonEvent& arg, Uint8 deviceID)
     {
         mInputBinder->mousePressed(arg, deviceID);
     }
 
-    void BindingsManager::mouseReleased(const SDL_MouseButtonEvent& arg, int deviceID)
+    void BindingsManager::mouseReleased(const SDL_MouseButtonEvent& arg, Uint8 deviceID)
     {
         mInputBinder->mouseReleased(arg, deviceID);
     }

apps/openmw/mwinput/bindingsmanager.hpp

@@ -47,8 +47,8 @@ namespace MWInput
 
         SDL_GameController* getControllerOrNull() const;
 
-        void mousePressed(const SDL_MouseButtonEvent& evt, int deviceID);
-        void mouseReleased(const SDL_MouseButtonEvent& arg, int deviceID);
+        void mousePressed(const SDL_MouseButtonEvent& evt, Uint8 deviceID);
+        void mouseReleased(const SDL_MouseButtonEvent& arg, Uint8 deviceID);
         void mouseMoved(const SDLUtil::MouseMotionEvent& arg);
         void mouseWheelMoved(const SDL_MouseWheelEvent& arg);
 

apps/openmw/mwsound/ffmpeg_decoder.cpp

@@ -21,7 +21,9 @@ namespace MWSound
             std::streamsize count = stream.gcount();
             if (count == 0)
                 return AVERROR_EOF;
-            return count;
+            if (count > std::numeric_limits<int>::max())
+                return AVERROR_BUG;
+            return static_cast<int>(count);
         }
         catch (std::exception&)
         {
@@ -72,7 +74,7 @@ namespace MWSound
         if (!mStream)
             return false;
 
-        int stream_idx = mStream - mFormatCtx->streams;
+        std::ptrdiff_t stream_idx = mStream - mFormatCtx->streams;
         while (av_read_frame(mFormatCtx, &mPacket) >= 0)
         {
             /* Check if the packet belongs to this stream */
@@ -427,9 +429,9 @@ namespace MWSound
 
     size_t FFmpeg_Decoder::getSampleOffset()
     {
-        int delay = (mFrameSize - mFramePos) / av_get_channel_layout_nb_channels(mOutputChannelLayout)
+        std::size_t delay = (mFrameSize - mFramePos) / av_get_channel_layout_nb_channels(mOutputChannelLayout)
             / av_get_bytes_per_sample(mOutputSampleFormat);
-        return (int)(mNextPts * mCodecCtx->sample_rate) - delay;
+        return static_cast<std::size_t>(mNextPts * mCodecCtx->sample_rate) - delay;
     }
 
     FFmpeg_Decoder::FFmpeg_Decoder(const VFS::Manager* vfs)

apps/openmw/mwsound/ffmpeg_decoder.hpp

@@ -41,8 +41,8 @@ namespace MWSound
         AVPacket mPacket;
         AVFrame* mFrame;
 
-        int mFrameSize;
-        int mFramePos;
+        std::size_t mFrameSize;
+        std::size_t mFramePos;
 
         double mNextPts;
 

apps/openmw/mwsound/loudness.cpp

@@ -15,8 +15,8 @@ namespace MWSound
             return;
 
         int samplesPerSegment = static_cast<int>(mSampleRate / mSamplesPerSec);
-        int numSamples = bytesToFrames(mQueue.size(), mChannelConfig, mSampleType);
-        int advance = framesToBytes(1, mChannelConfig, mSampleType);
+        std::size_t numSamples = bytesToFrames(mQueue.size(), mChannelConfig, mSampleType);
+        std::size_t advance = framesToBytes(1, mChannelConfig, mSampleType);
 
         int segment = 0;
         int sample = 0;
@@ -61,7 +61,7 @@ namespace MWSound
         if (mSamplesPerSec <= 0.0f || mSamples.empty() || sec < 0.0f)
             return 0.0f;
 
-        size_t index = std::clamp<size_t>(sec * mSamplesPerSec, 0, mSamples.size() - 1);
+        size_t index = std::clamp<size_t>(static_cast<size_t>(sec * mSamplesPerSec), 0, mSamples.size() - 1);
         return mSamples[index];
     }
 

components/bsa/ba2dx10file.cpp

@@ -6,16 +6,21 @@
 
 #include <lz4frame.h>
 
+
+#if defined(_MSC_VER)
+// why is this necessary? These are included with /external:I
+#pragma warning(push)
+#pragma warning(disable : 4706)
+#pragma warning(disable : 4702)
 #include <boost/iostreams/copy.hpp>
 #include <boost/iostreams/filtering_stream.hpp>
 #include <boost/iostreams/filtering_streambuf.hpp>
-
-#if defined(_MSC_VER)
-#pragma warning(push)
-#pragma warning(disable : 4706)
 #include <boost/iostreams/filter/zlib.hpp>
 #pragma warning(pop)
 #else
+#include <boost/iostreams/copy.hpp>
+#include <boost/iostreams/filtering_stream.hpp>
+#include <boost/iostreams/filtering_streambuf.hpp>
 #include <boost/iostreams/filter/zlib.hpp>
 #endif
 

components/bsa/ba2gnrlfile.cpp

@@ -6,15 +6,18 @@
 
 #include <lz4frame.h>
 
-#include <boost/iostreams/copy.hpp>
-#include <boost/iostreams/filtering_streambuf.hpp>
-
 #if defined(_MSC_VER)
+// why is this necessary? These are included with /external:I
 #pragma warning(push)
 #pragma warning(disable : 4706)
+#pragma warning(disable : 4702)
+#include <boost/iostreams/copy.hpp>
+#include <boost/iostreams/filtering_streambuf.hpp>
 #include <boost/iostreams/filter/zlib.hpp>
 #pragma warning(pop)
 #else
+#include <boost/iostreams/copy.hpp>
+#include <boost/iostreams/filtering_streambuf.hpp>
 #include <boost/iostreams/filter/zlib.hpp>
 #endif
 

components/bsa/compressedbsafile.cpp

@@ -30,15 +30,18 @@
 
 #include <lz4frame.h>
 
-#include <boost/iostreams/copy.hpp>
-#include <boost/iostreams/filtering_streambuf.hpp>
 
 #if defined(_MSC_VER)
 #pragma warning(push)
 #pragma warning(disable : 4706)
+#pragma warning(disable : 4702)
+#include <boost/iostreams/copy.hpp>
+#include <boost/iostreams/filtering_streambuf.hpp>
 #include <boost/iostreams/filter/zlib.hpp>
 #pragma warning(pop)
 #else
+#include <boost/iostreams/copy.hpp>
+#include <boost/iostreams/filtering_streambuf.hpp>
 #include <boost/iostreams/filter/zlib.hpp>
 #endif
 
@@ -168,7 +171,7 @@ namespace Bsa
                     name.resize(input.gcount());
                     if (name.back() != '\0')
                         fail("Failed to read a filename: filename is too long");
-                    mHeader.mFileNamesLength -= input.gcount();
+                    mHeader.mFileNamesLength -= static_cast<std::uint32_t>(input.gcount());
                     file.mName.insert(file.mName.begin(), folder.mName.begin(), folder.mName.end());
                     file.mName.insert(file.mName.begin() + folder.mName.size(), '\\');
                 }

components/debug/debugdraw.cpp

@@ -124,7 +124,7 @@ static void generateCylinder(osg::Geometry& geom, float radius, float height, in
     for (int i = 0; i < subdiv; i++)
     {
         float theta = (float(i) / float(subdiv)) * osg::PI * 2.;
-        osg::Vec3 pos = sphereCoordToCartesian(theta, osg::PI_2, 1.);
+        osg::Vec3 pos = sphereCoordToCartesian(theta, osg::PI_2f, 1.);
         pos *= radius;
         pos.z() = height / 2.;
         vertices->push_back(pos);
@@ -150,7 +150,7 @@ static void generateCylinder(osg::Geometry& geom, float radius, float height, in
     for (int i = 0; i < subdiv; i++)
     {
         float theta = float(i) / float(subdiv) * osg::PI * 2.;
-        osg::Vec3 pos = sphereCoordToCartesian(theta, osg::PI_2, 1.);
+        osg::Vec3 pos = sphereCoordToCartesian(theta, osg::PI_2f, 1.);
         pos *= radius;
         pos.z() = -height / 2.;
         vertices->push_back(pos);
@@ -162,7 +162,7 @@ static void generateCylinder(osg::Geometry& geom, float radius, float height, in
     for (int i = 0; i < subdiv; i++)
     {
         float theta = float(i) / float(subdiv) * osg::PI * 2.;
-        osg::Vec3 normal = sphereCoordToCartesian(theta, osg::PI_2, 1.);
+        osg::Vec3 normal = sphereCoordToCartesian(theta, osg::PI_2f, 1.);
         auto posTop = normal;
         posTop *= radius;
         auto posBot = posTop;

components/debug/debugging.cpp

@@ -6,7 +6,15 @@
 #include <map>
 #include <memory>
 
+#ifdef _MSC_VER
+// TODO: why is this necessary? this has /external:I
+#pragma warning(push)
+#pragma warning(disable : 4702)
+#endif
 #include <boost/iostreams/stream.hpp>
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif
 
 #include <components/crashcatcher/crashcatcher.hpp>
 #include <components/files/conversion.hpp>
@@ -111,7 +119,7 @@ namespace Debug
                 msg = msg.substr(1);
 
             char prefix[32];
-            int prefixSize;
+            std::size_t prefixSize;
             {
                 prefix[0] = '[';
                 const auto now = std::chrono::system_clock::now();

components/detournavigator/collisionshapetype.cpp

@@ -15,7 +15,7 @@ namespace DetourNavigator
                 return static_cast<CollisionShapeType>(value);
         }
         std::string error("Invalid CollisionShapeType value: \"");
-        error += value;
+        error += std::to_string(value);
         error += '"';
         throw std::invalid_argument(error);
     }

components/files/windowspath.cpp

@@ -101,7 +101,7 @@ namespace Files
         {
             // Key existed, let's try to read the install dir
             std::array<wchar_t, 512> buf{};
-            DWORD len = buf.size() * sizeof(wchar_t);
+            DWORD len = static_cast<DWORD>(buf.size() * sizeof(wchar_t));
 
             if (RegQueryValueExW(hKey, L"Installed Path", nullptr, nullptr, reinterpret_cast<LPBYTE>(buf.data()), &len)
                 == ERROR_SUCCESS)

components/fx/lexer.hpp

@@ -30,7 +30,7 @@ namespace fx
         public:
             struct Block
             {
-                int line;
+                std::size_t line;
                 std::string_view content;
             };
 

components/l10n/messagebundles.cpp

@@ -77,7 +77,7 @@ namespace l10n
             {
                 const auto key = it.first.as<std::string>();
                 const auto value = it.second.as<std::string>();
-                icu::UnicodeString pattern = icu::UnicodeString::fromUTF8(icu::StringPiece(value.data(), value.size()));
+                icu::UnicodeString pattern = icu::UnicodeString::fromUTF8(icu::StringPiece(value.data(), static_cast<std::int32_t>(value.size())));
                 icu::ErrorCode status;
                 UParseError parseError;
                 icu::MessageFormat message(pattern, langOrEn, parseError, status);
@@ -115,7 +115,7 @@ namespace l10n
         std::vector<icu::Formattable> argValues;
         for (auto& [k, v] : args)
         {
-            argNames.push_back(icu::UnicodeString::fromUTF8(icu::StringPiece(k.data(), k.size())));
+            argNames.push_back(icu::UnicodeString::fromUTF8(icu::StringPiece(k.data(), static_cast<std::int32_t>(k.size()))));
             argValues.push_back(v);
         }
         return formatMessage(key, argNames, argValues);
@@ -160,9 +160,9 @@ namespace l10n
         if (message)
         {
             if (!args.empty() && !argNames.empty())
-                message->format(argNames.data(), args.data(), args.size(), result, success);
+                message->format(argNames.data(), args.data(), static_cast<std::int32_t>(args.size()), result, success);
             else
-                message->format(nullptr, nullptr, args.size(), result, success);
+                message->format(nullptr, nullptr, static_cast<std::int32_t>(args.size()), result, success);
             checkSuccess(success, std::string("Failed to format message ") + key.data());
             result.toUTF8String(resultString);
             return resultString;
@@ -174,15 +174,15 @@ namespace l10n
         }
         UParseError parseError;
         icu::MessageFormat defaultMessage(
-            icu::UnicodeString::fromUTF8(icu::StringPiece(key.data(), key.size())), defaultLocale, parseError, success);
+            icu::UnicodeString::fromUTF8(icu::StringPiece(key.data(), static_cast<std::int32_t>(key.size()))), defaultLocale, parseError, success);
         if (!checkSuccess(success, std::string("Failed to create message ") + key.data(), parseError))
             // If we can't parse the key as a pattern, just return the key
             return std::string(key);
 
         if (!args.empty() && !argNames.empty())
-            defaultMessage.format(argNames.data(), args.data(), args.size(), result, success);
+            defaultMessage.format(argNames.data(), args.data(), static_cast<std::int32_t>(args.size()), result, success);
         else
-            defaultMessage.format(nullptr, nullptr, args.size(), result, success);
+            defaultMessage.format(nullptr, nullptr, static_cast<std::int32_t>(args.size()), result, success);
         checkSuccess(success, std::string("Failed to format message ") + key.data());
         result.toUTF8String(resultString);
         return resultString;

components/lua/scriptscontainer.cpp

@@ -590,7 +590,7 @@ namespace LuaUtil
         updateTimerQueue(mGameTimersQueue, gameTime);
     }
 
-    static constexpr float instructionCountAvgCoef = 1.0 / 30; // averaging over approximately 30 frames
+    static constexpr float instructionCountAvgCoef = 1.0f / 30; // averaging over approximately 30 frames
 
     void ScriptsContainer::statsNextFrame()
     {

components/lua/utf8.cpp

@@ -14,7 +14,7 @@ namespace
         return (arg.get_type() == sol::type::lua_nil || arg.get_type() == sol::type::none);
     }
 
-    inline double getInteger(const sol::stack_proxy arg, const size_t n, std::string_view name)
+    inline std::int64_t getInteger(const sol::stack_proxy arg, const size_t n, std::string_view name)
     {
         double integer;
         if (!arg.is<double>())
@@ -25,7 +25,7 @@ namespace
             throw std::runtime_error(
                 Misc::StringUtils::format("bad argument #%i to '%s' (number has no integer representation)", n, name));
 
-        return integer;
+        return static_cast<std::int64_t>(integer);
     }
 
     // If the input 'pos' is negative, it is treated as counting from the end of the string,
@@ -104,7 +104,8 @@ namespace LuaUtf8
                     throw std::runtime_error(
                         "bad argument #" + std::to_string(i + 1) + " to 'char' (value out of range)");
 
-                result += converter.to_bytes(codepoint);
+                // this feels dodgy if wchar_t is 16-bit as MAXUTF won't fit in sixteen bits
+                result += converter.to_bytes(static_cast<wchar_t>(codepoint));
             }
             return result;
         };

components/misc/utf8stream.hpp

@@ -75,7 +75,7 @@ public:
             return std::make_pair(chr, cur);
         }
 
-        int octets;
+        std::size_t octets;
         UnicodeChar chr;
 
         std::tie(octets, chr) = getOctetCount(*cur++);

components/sdlutil/sdlmappings.cpp

@@ -83,7 +83,7 @@ namespace SDLUtil
 
     Uint8 myGuiMouseButtonToSdl(MyGUI::MouseButton button)
     {
-        Uint8 value = button.getValue() + 1;
+        Uint8 value = static_cast<Uint8>(button.getValue() + 1);
         if (value == SDL_BUTTON_RIGHT)
             value = SDL_BUTTON_MIDDLE;
         else if (value == SDL_BUTTON_MIDDLE)