[vlmc-devel] [PATCH 2/2] Implement SettingValue::Type::List

[Hugo Beauzée-Luyssen]

On 03/11/2016 07:09 AM, Yikai Lu wrote:
> In order to remove save/load methods in many classes, we need to be able to save QList.
>
> ---
>   src/Settings/SettingValue.h |  1 +
>   src/Settings/Settings.cpp   | 52 ++++++++++++++++++++++++++++++++++++++-------
>   2 files changed, 45 insertions(+), 8 deletions(-)
>
> diff --git a/src/Settings/SettingValue.h b/src/Settings/SettingValue.h
> index ebc225e..cf9932f 100644
> --- a/src/Settings/SettingValue.h
> +++ b/src/Settings/SettingValue.h
> @@ -46,6 +46,7 @@ class   SettingValue : public QObject
>               Language,
>               KeyboardShortcut,
>               Path,
> +            List,
>               ByteArray, // For now this is only for private variables, and is not expected to be used at any time
>               //For effect engine settings:
>               Color,
> diff --git a/src/Settings/Settings.cpp b/src/Settings/Settings.cpp
> index ab48a23..2b5dc26 100644
> --- a/src/Settings/Settings.cpp
> +++ b/src/Settings/Settings.cpp
> @@ -77,10 +77,25 @@ Settings::save( QXmlStreamWriter& project )
>           if ( ( val->flags() & SettingValue::Runtime ) != 0 )
>               continue ;
>           project.writeStartElement( "setting" );
> -        project.writeAttribute( "key", val->key() );
> -        if ( val->get().canConvert<QString>() == false )
> -            vlmcWarning() << "Can't serialize" << val->key();
> -        project.writeAttribute( "value", val->get().toString() );
> +        if ( val->type() == SettingValue::List )
> +        {
> +            project.writeAttribute( "key", val->key() + "/List" );
> +            for ( const QVariant& var: val->get().toList() ) {
> +                project.writeStartElement( "setting" );

This seems to imply that all lists will be stored in a <setting> tag, 
which feels wrong.
The way we differentiate between project & global settings in the 
Setting class is quite wrong too though.
I also don't really understand the "/List" suffix. Theoretically, you 
don't care about the type of the value you're saving, QVariant will 
handle it for you.

> +                project.writeAttribute( "key", val->key() );
> +                if ( var.canConvert<QString>() == false )
> +                    vlmcWarning() << "Can't serialize" << val->key();
> +                project.writeAttribute( "value", var.toString() );
> +                project.writeEndElement();
> +            }
> +        }
> +        else
> +        {
> +            project.writeAttribute( "key", val->key() );
> +            if ( val->get().canConvert<QString>() == false )
> +                vlmcWarning() << "Can't serialize" << val->key();
> +            project.writeAttribute( "value", val->get().toString() );
> +        }
>           project.writeEndElement();
>       }
>       project.writeEndElement();
> @@ -100,15 +115,36 @@ Settings::load( const QDomDocument& document )
>       while ( s.isNull() == false )
>       {
>           QString     key = s.attribute( "key" );
> -        QString     value = s.attribute( "value" );
> +        QString     value;
>
>           if ( key.isEmpty() == true )
>               vlmcWarning() << "Invalid setting node.";
>           else
>           {
> -            vlmcDebug() << "Loading" << key << "=>" << value;
> -            if ( setValue( key, value ) == false )
> -                vlmcWarning() << "Loaded invalid project setting:" << key;
> +            if ( key.endsWith("/List") )
> +            {
> +                key.chop(5);
> +                s = s.firstChildElement( "setting" );
> +                QList<QVariant> list;
> +                while ( s.isNull() == false )
> +                {
> +                    if ( key.isEmpty() == true )
> +                        vlmcWarning() << "Invalid setting node.";
> +                    value = s.attribute( "value" );
> +                    vlmcDebug() << "Loading" << key << "=>" << value;
> +                    list << QVariant::fromValue( value );
> +                    s = s.nextSiblingElement();
> +                }
> +                if ( setValue( key, list ) == false )
> +                    vlmcWarning() << "Loaded invalid project setting:" << key;
> +            }
> +            else
> +            {
> +                value = s.attribute( "value" );
> +                vlmcDebug() << "Loading" << key << "=>" << value;
> +                if ( setValue( key, value ) == false )
> +                    vlmcWarning() << "Loaded invalid project setting:" << key;
> +            }
>           }
>           s = s.nextSiblingElement();
>       }
>

While we could indeed handle a List type, the question would be "a list 
of what". So far, all SettingValue types are "plain" type, whereas QList 
is a template type. We could always handle a QList of QVariant, but this 
sounds not as straight forward as using an integer, a QString, ...
It would also be close to impossible to expose in the GUI, though we 
could keep this type as a private setting.

Anyway, it sounds to me like you're trying to solve the mismatch between 
<project><settings> settings list </settings></project> for a project 
file, and the <vlmc><settings> settings list </settings></vlmc> we 
talked about on IRC. That isn't the right way to do it as far as I'm 
concerned.

Regards,