D26196: Add multiple dataset in guessnumber activity

[Johnny Jazeix]

jjazeix added a comment.


  Please fix the following errors:
  qrc:/gcompris/src/activities/guessnumber/Guessnumber.qml:65: TypeError: Cannot read property 'difficulty' of null
  qrc:/gcompris/src/activities/guessnumber/Guessnumber.qml:64: TypeError: Cannot read property 'data' of null
  qrc:/gcompris/src/activities/guessnumber/Guessnumber.qml:183: Error: Cannot assign to non-existent property "currentLevel"

INLINE COMMENTS

> AnswerArea.qml:73
>          if(userEntry != "")
> +            userEntryText.text = Number(answerBackground.userEntry).toLocaleString(Qt.locale(), 'f', 0)
>              Activity.setUserAnswer(parseInt(userEntry))

you have to use the locale used in GCompris, not the one of the computer

> Guessnumber.qml:65
> +            property var levels: activity.datasetLoader.item.data
> +            property var difficulty: activity.datasetLoader.item.difficulty
>              property int currentMax: 0

it's an integer so it's better to declare it as integer than var

> Guessnumber.qml:129
> +                topMargin: 20 + textArea.contentHeight
> +                }
> +            visible: false

indentation

> Guessnumber.qml:134
> +                    id: repeatNumbers
> +                    model: ["1", "2", "3", "4", "5", "6", "7", "8", "9", "10",
> +                            "11", "12", "13", "14", "15", "16", "17", "18", "19", "20"]

It should be dependent on the dataset max value (and handle well the scale), not hardcoded.
Maybe it would be nice to only have a max number of displayed values (5 or 10) and round, for example for first dataset, and the plane moves accordingly:

| 0 | 5 | 10 | 15 | 20 |

> Guessnumber.qml:137
> +                    Rectangle {
> +                    id: suiteNumber
> +                    width: 80

this does not scale well (if you have a small screen, you don't see the last numbers)

> Guessnumber.qml:145
> +                    gradient: Gradient {
> +                    GradientStop { position: 0.0; color: "#CCFFFFFF" }
> +                    GradientStop { position: 0.5; color: "#80FFFFFF" }

indentation

> Guessnumber.qml:149
> +                    }
> +                        Text {
> +                            text: modelData

indentation + use GCText for texts (to have the good font used)

> guessnumber.js:49
> +        numberToGuess = getRandomInt(1, items.levels[currentLevel].maxNumber)
> +    if(items.difficulty == 1)
> +        if(currentLevel == 0 || currentLevel == 2 || currentLevel == 4)

make the visibility of the item a variable of the dataset, we don't want hardcode depending on the dataset in the common code

REPOSITORY
  R2 GCompris

REVISION DETAIL
  https://phabricator.kde.org/D26196

To: sambhavkaul, #gcompris_improvements
Cc: jjazeix, echarruau, kde-edu, sanjayshetty, parimalprasoon, harrymecwan, ganeshredcobra, asagtani, nityanandkumar, andreask, rahulyadav, narvaez, scagarwal, apol, timotheegiet, bcoudoin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20191227/fd6411ff/attachment-0001.html>