Fixes of MS'review as proposed

https://dev.vatsim-germany.org/issues/368#change-2380 (1,2)
https://dev.vatsim-germany.org/issues/364#change-2379 (1-11,13)

src/blackcore/context_ownaircraft_impl.cpp

@@ -144,7 +144,7 @@ namespace BlackCore
             emit this->changedAircraft(aircraft, originator);
 
             // now set value
-            this->m_ownAircraft.update(aircraft);
+            this->m_ownAircraft.setAircraft(aircraft);
             changed = true;
         }
         return changed;
@@ -421,7 +421,7 @@ namespace BlackCore
      */
     const CAircraft &CContextOwnAircraft::getAviationAircraft() const
     {
-        return static_cast<CAircraft const &>(this->m_ownAircraft);
+        return this->m_ownAircraft;
     }
 
 } // namespace

src/blackcore/context_simulator_impl.cpp

@@ -291,7 +291,7 @@ namespace BlackCore
         if (m_simulator)
         {
             // depending on shutdown order, network might already have been deleted
-            //! \todo why do we need to disconnet context when we unload driver?
+            //! \todo link with airspace monitor to be reviewed when airspace monitor becomes context
             if (this->getRuntime()->getIContextNetwork()->isUsingImplementingObject())
             {
                 CContextNetwork *network = this->getRuntime()->getCContextNetwork();

src/blackgui/components/mappingcomponent.cpp

@@ -291,7 +291,7 @@ namespace BlackGui
             this->ui->tvp_AircraftModels->updateContainer(ml);
 
             // model completer
-            this->m_modelCompleter->setModel(new QStringListModel(ml.getModelStrings(), this->m_modelCompleter));
+            this->m_modelCompleter->setModel(new QStringListModel(ml.getSortedModelStrings(), this->m_modelCompleter));
             this->m_modelCompleter->setModelSorting(QCompleter::CaseInsensitivelySortedModel);
             this->m_modelCompleter->setCaseSensitivity(Qt::CaseInsensitive);
             this->m_modelCompleter->setWrapAround(true);

src/blackmisc/avaircrafticao.cpp

@@ -37,7 +37,7 @@ namespace BlackMisc
 
         bool CAircraftIcao::hasKnownAircraftDesignator() const
         {
-            return (this->getAircraftDesignator() != "ZZZZ");
+            return (this->hasAircraftDesignator() && this->getAircraftDesignator() != "ZZZZ");
         }
 
         QString CAircraftIcao::asString() const

src/blackmisc/avcallsign.cpp

@@ -179,8 +179,7 @@ namespace BlackMisc
          */
         bool CCallsign::isValidCallsign(const QString &callsign)
         {
-            //! \todo sometimes callsigns such as 12345, really correct?
+            // We allow all number callsigns
-            // static QRegularExpression regexp("^[A-Z]+[A-Z0-9]*$");
             static QRegularExpression regexp("^[A-Z0-9]*$");
             if (callsign.length() < 2 || callsign.length() > 10) { return false; }
             return (regexp.match(callsign).hasMatch());

src/blackmisc/nwaircraftmapping.cpp

@@ -23,7 +23,7 @@ namespace BlackMisc
          * Constructor
          */
         CAircraftMapping::CAircraftMapping(const QString &source, const QString &packageName, const QString &aircraftDesignator, const QString &airlineDesignator, const QString &model) :
-            m_source(source.trimmed()), m_packageName(packageName.trimmed()), m_icao(CAircraftIcao(aircraftDesignator, airlineDesignator)), m_model(BlackMisc::Simulation::CAircraftModel(model, BlackMisc::Simulation::CAircraftModel::TypeModelMapping))
+            m_source(source.trimmed()), m_packageName(packageName.trimmed()), m_icao(aircraftDesignator, airlineDesignator), m_model(model, BlackMisc::Simulation::CAircraftModel::TypeModelMapping)
         { }
 
         /*

src/blackmisc/nwaircraftmappinglist.cpp

@@ -72,7 +72,7 @@ namespace BlackMisc
             return this->findBy(&CAircraftMapping::getIcao, searchIcao);
         }
 
-        CAircraftMappingList CAircraftMappingList::findByModelString(const QString modelString, Qt::CaseSensitivity sensitivity) const
+        CAircraftMappingList CAircraftMappingList::findByModelString(const QString &modelString, Qt::CaseSensitivity sensitivity) const
         {
             return this->findBy([ = ](const CAircraftMapping & mapping)
             {

src/blackmisc/nwaircraftmappinglist.h

@@ -49,7 +49,7 @@ namespace BlackMisc
             CAircraftMappingList findByIcaoCodeExact(const BlackMisc::Aviation::CAircraftIcao &searchIcao) const;
 
             //! Find by model string
-            CAircraftMappingList findByModelString(const QString modelString, Qt::CaseSensitivity sensitivity = Qt::CaseInsensitive) const;
+            CAircraftMappingList findByModelString(const QString &modelString, Qt::CaseSensitivity sensitivity = Qt::CaseInsensitive) const;
 
             //! \copydoc CValueObject::toQVariant
             virtual QVariant toQVariant() const override { return QVariant::fromValue(*this); }

src/blackmisc/nwuser.cpp

@@ -52,10 +52,8 @@ namespace BlackMisc
 
         void CUser::setRealName(const QString &realname)
         {
-            // try to strip homebase
+
-            // I understand the limitations, but we will have more correct hits as failures I assume
+            QString rn(realname.trimmed().simplified());
-            const QRegularExpression reg("(-\\s*|\\s)([A-Z]{4})$");
-            QString rn = realname.trimmed().simplified();
             if (rn.isEmpty())
             {
                 this->m_realname = "";
@@ -64,8 +62,12 @@ namespace BlackMisc
 
             if (!this->hasValidHomebase())
             {
-                //! only apply stripping if home base is not explicitly given
+                // only apply stripping if home base is not explicitly given
-                QRegularExpressionMatch match = reg.match(rn);
+                // try to strip homebase: I understand the limitations, but we will have more correct hits as failures I assume
+                static QThreadStorage<QRegularExpression> tsRegex;
+                if (! tsRegex.hasLocalData()) { tsRegex.setLocalData(QRegularExpression("(-\\s*|\\s)([A-Z]{4})$")); }
+                const auto &regex = tsRegex.localData();
+                QRegularExpressionMatch match = regex.match(rn);
                 if (match.hasMatch())
                 {
                     int pos = match.capturedStart(0);
@@ -74,7 +76,9 @@ namespace BlackMisc
                     this->setHomebase(CAirportIcao(icao));
                 }
             }
-            this->m_realname = rn;
+
+            // do not beautify before stripping home base
+            this->m_realname = beautifyRealName(rn);
         }
 
         CStatusMessageList CUser::validate() const
@@ -140,6 +144,31 @@ namespace BlackMisc
             return i >= 100000 && i <= 9999999;
         }
 
+        QString CUser::beautifyRealName(const QString &realName)
+        {
+            QString newRealName(realName.simplified().trimmed().toLower());
+            if (newRealName.isEmpty()) { return ""; }
+
+            // simple title case
+            QString::Iterator i = newRealName.begin();
+            bool upperNextChar = true;
+            while (i != newRealName.end())
+            {
+                if (i->isSpace())
+                {
+                    upperNextChar = true;
+                }
+                else if (upperNextChar)
+                {
+                    QChar u(i->toUpper());
+                    *i = u;
+                    upperNextChar = false;
+                }
+                ++i;
+            }
+            return newRealName;
+        }
+
         /*
          * Property by index
          */
@@ -166,9 +195,6 @@ namespace BlackMisc
             }
         }
 
-        /*
-         * Set property as index
-         */
         void CUser::setPropertyByIndex(const CVariant &variant, const BlackMisc::CPropertyIndex &index)
         {
             if (index.isMyself())

src/blackmisc/nwuser.h

@@ -52,7 +52,7 @@ namespace BlackMisc
             CUser(const QString &id, const QString &realname, const BlackMisc::Aviation::CCallsign &callsign);
 
             //! Constructor.
-            CUser(const QString &id, const QString &realname, const QString &email = "", const QString &password = "", const BlackMisc::Aviation::CCallsign &callsign = BlackMisc::Aviation::CCallsign());
+            CUser(const QString &id, const QString &realname, const QString &email = "", const QString &password = "", const BlackMisc::Aviation::CCallsign &callsign = {});
 
             //! Get full name.
             QString getRealName() const { return m_realname; }
@@ -132,6 +132,9 @@ namespace BlackMisc
             //! Valid VATSIM id
             static bool isValidVatsimId(const QString &id);
 
+            //! Beautify real name, e.g. "JOE DoE" -> "Joe Doe";
+            static QString beautifyRealName(const QString &realName);
+
         protected:
             //! \copydoc CValueObject::convertToQString
             virtual QString convertToQString(bool i18n = false) const override;

src/blackmisc/pixmap.cpp

@@ -9,6 +9,7 @@
 
 #include "pixmap.h"
 #include <QBuffer>
+#include <tuple>
 
 namespace BlackMisc
 {
@@ -18,9 +19,22 @@ namespace BlackMisc
         this->fillByteArray();
     }
 
+    CPixmap::CPixmap(const CPixmap &other) : CValueObjectStdTuple(), m_pixmap(other.m_pixmap), m_hasCachedPixmap(other.m_hasCachedPixmap), m_array(other.m_array)
+    {}
+
+    CPixmap &CPixmap::operator =(const CPixmap &other)
+    {
+        std::tie(m_pixmap, m_hasCachedPixmap, m_array) = std::tie(other.m_pixmap, other.m_hasCachedPixmap, other.m_array);
+        return (*this);
+    }
+
     const QPixmap &CPixmap::pixmap() const
     {
+        QWriteLocker(&this->m_lock);
         if (this->m_hasCachedPixmap) { return this->m_pixmap; }
+
+        // this part here becomes relevant when marshalling via DBus is used
+        // in this case only the array is transferred
         this->m_hasCachedPixmap = true;
         if (this->m_array.isEmpty()) { return this->m_pixmap; }
         bool s = this->m_pixmap.loadFromData(this->m_array, "PNG");
@@ -31,13 +45,14 @@ namespace BlackMisc
 
     bool CPixmap::isNull() const
     {
+        QReadLocker(&this->m_lock);
         if (this->m_hasCachedPixmap) { return false; }
         return (this->m_array.isEmpty() || this->m_array.isNull());
     }
 
     CPixmap::operator QPixmap() const
     {
-        return QPixmap(pixmap());
+        return pixmap();
     }
 
     QString CPixmap::convertToQString(bool i18n) const

src/blackmisc/pixmap.h

@@ -14,6 +14,7 @@
 
 #include "valueobject.h"
 #include <QPixmap>
+#include <QReadWriteLock>
 
 namespace BlackMisc
 {
@@ -28,6 +29,12 @@ namespace BlackMisc
         //! Constructor.
         CPixmap(const QPixmap &pixmap);
 
+        //! Copy constructor (because of mutex)
+        CPixmap(const CPixmap &other);
+
+        //! Copy assignment (because of mutex)
+        CPixmap &operator =(const CPixmap &other);
+
         //! Corresponding pixmap
         const QPixmap &pixmap() const;
 
@@ -52,6 +59,8 @@ namespace BlackMisc
 
         mutable QPixmap m_pixmap;               //!< cached pixmap, mutable because of lazy initialization
         mutable bool m_hasCachedPixmap = false; //!< pixmap? Mutable because of lazy initialization
+        mutable QReadWriteLock m_lock;          //!< lock (because of mutable members)
+
         QByteArray m_array;                     //!< data of pixmap
     };
 } // namespace

src/blackmisc/pqstring.cpp

@@ -10,6 +10,7 @@
 #include "pqstring.h"
 #include "tuple.h"
 #include "pqallquantities.h"
+#include <QThreadStorage>
 
 namespace BlackMisc
 {
@@ -28,24 +29,27 @@ namespace BlackMisc
          */
         CVariant CPqString::parseToVariant(const QString &value, SeparatorMode mode)
         {
-            static QRegExp rx("([-+]?[0-9]*[\\.,]?[0-9]+)\\s*(\\D*)$");
             CVariant v;
 
             // fine tuning of the string
             QString vs = value.trimmed().simplified();
 
             // check
-            if (vs.isEmpty()) return v;
+            if (vs.isEmpty()) { return v; }
 
-            if (rx.indexIn(value) < 0) return v; // not a valid number
+            static QThreadStorage<QRegExp> tsRegex;
-            QString unit = rx.cap(2).trimmed();
+            if (! tsRegex.hasLocalData()) { tsRegex.setLocalData(QRegExp("([-+]?[0-9]*[\\.,]?[0-9]+)\\s*(\\D*)$")); }
+            const auto &regex = tsRegex.localData();
+
+            if (regex.indexIn(value) < 0) { return v; } // not a valid number
+            QString unit = regex.cap(2).trimmed();
             QString number = QString(value).replace(unit, "");
             unit = unit.trimmed(); // trim after replace, not before
 
-            if (unit.isEmpty() || number.isEmpty()) return v;
+            if (unit.isEmpty() || number.isEmpty()) { return v; }
             bool success;
             double numberD = parseNumber(number, success, mode);
-            if (!success) return v;
+            if (!success) {return v; }
 
             if (CMeasurementUnit::isValidUnitSymbol<CAccelerationUnit>(unit))
             {

src/blackmisc/simulation/aircraftmodellist.cpp

@@ -44,18 +44,13 @@ namespace BlackMisc
 
         CAircraftModelList CAircraftModelList::findModelsStartingWith(const QString &modelString, Qt::CaseSensitivity sensitivity) const
         {
-            CAircraftModelList ml;
+            return this->findBy([ = ](const CAircraftModel & model)
-            for (const CAircraftModel &model : (*this))
             {
-                if (model.getModelString().startsWith(modelString, sensitivity))
+                return model.getModelString().startsWith(modelString, sensitivity);
-                {
+            });
-                    ml.push_back(model);
-                }
-            }
-            return ml;
         }
 
-        QStringList CAircraftModelList::getModelStrings() const
+        QStringList CAircraftModelList::getSortedModelStrings() const
         {
             QStringList ms;
             for (const CAircraftModel &model : (*this))

src/blackmisc/simulation/aircraftmodellist.h

@@ -48,7 +48,7 @@ namespace BlackMisc
             CAircraftModelList findModelsStartingWith(const QString &modelString, Qt::CaseSensitivity sensitivity = Qt::CaseInsensitive) const;
 
             //! Model strings
-            QStringList getModelStrings() const;
+            QStringList getSortedModelStrings() const;
 
             //! \copydoc CValueObject::convertFromQVariant
             virtual void convertFromQVariant(const QVariant &variant) override { BlackMisc::setFromQVariant(this, variant); }

src/blackmisc/simulation/simulatedaircraft.cpp

@@ -121,10 +121,13 @@ namespace BlackMisc
         }
 
         //! \todo Smarter way to do this?
-        void CSimulatedAircraft::update(const CAircraft &aircraft)
+        void CSimulatedAircraft::setAircraft(const CAircraft &aircraft)
         {
-            // override
+            static_cast<CAircraft &>(*this) = aircraft;
-            (*this) = CSimulatedAircraft(aircraft, this->getModel(), this->getClient());
+            this->m_model.setCallsign(aircraft.getCallsign());
+            this->m_client.setAircraftModel(this->getModel());
+            this->m_client.setUser(aircraft.getPilot());
+            this->m_client.setUserCallsign(aircraft.getCallsign());
         }
 
         QString CSimulatedAircraft::convertToQString(bool i18n) const

src/blackmisc/simulation/simulatedaircraft.h

@@ -38,8 +38,8 @@ namespace BlackMisc
 
             //! Constructor.
             CSimulatedAircraft(const BlackMisc::Aviation::CAircraft &aircraft,
-                               const BlackMisc::Simulation::CAircraftModel &model = BlackMisc::Simulation::CAircraftModel(),
+                               const BlackMisc::Simulation::CAircraftModel &model = {},
-                               const BlackMisc::Network::CClient &client = BlackMisc::Network::CClient());
+                               const BlackMisc::Network::CClient &client = {});
 
             //! \copydoc CValueObject::propertyByIndex
             virtual CVariant propertyByIndex(const BlackMisc::CPropertyIndex &index) const override;
@@ -75,7 +75,7 @@ namespace BlackMisc
             void setEnabled(bool enabled) { m_enabled = enabled; }
 
             //! Update from aviation aircraft
-            void update(const BlackMisc::Aviation::CAircraft &aircraft);
+            void setAircraft(const BlackMisc::Aviation::CAircraft &aircraft);
 
         protected:
             //! \copydoc CValueObject::convertToQString()

src/blackmisc/threadedreader.h

@@ -119,7 +119,6 @@ namespace BlackMisc
 
     private:
         QDateTime m_updateTimestamp;                    //!< when was file / resource read
-
         QFuture<FutureRet> m_pendingFuture;             //!< optional future to be stopped
         QNetworkReply *m_pendingNetworkReply = nullptr; //!< optional network reply to be stopped
     };

src/blacksim/fscommon/aircraftcfgentrieslist.h

@@ -96,13 +96,13 @@ namespace BlackSim
             //! Do not include the following directories for FS
             static const QStringList &excludeDirectories()
             {
-                static const QStringList exclude(
+                static const QStringList exclude
                 {
                     "SimObjects/Animals",
                     "SimObjects/Misc",
                     "SimObjects/GroundVehicles",
                     "SimObjects/Boats"
-                });
+                };
                 return exclude;
             }