Provide global config to set number to expose children to(ref: T3173)
Needs ReviewPublic

Authored by rohitdas on Mar 15 2018, 4:05 PM.

Details

Summary

Setting a number limit so that numbers in games like dominoes, dice, etc. can be limited from global config.

Diff Detail

Repository
R2 GCompris
Lint
Lint Skipped
Unit
Unit Tests Skipped
rohitdas created this revision.Mar 15 2018, 4:05 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptMar 15 2018, 4:05 PM
Restricted Application added a subscriber: KDE Edu. · View Herald Transcript
rohitdas requested review of this revision.Mar 15 2018, 4:05 PM

These changes are throwing a segmentation fault on clicking on config. Please review.

It's a really quick review, I just read it once.
Have you found the cause of the segfault?

src/activities/menu/ConfigurationItem.qml
591 ↗(On Diff #29607)

the diff shows us the change, no need to add a comment for it

src/core/ApplicationSettings.h
340 ↗(On Diff #29607)

qBound?

rohitdas added inline comments.Apr 1 2018, 10:56 AM
src/activities/menu/ConfigurationItem.qml
591 ↗(On Diff #29607)

I added the comments so that I could find the lines while editing. I will remove them in the diffs when I submit them.

rohitdas updated this revision to Diff 31089.Apr 1 2018, 11:14 AM

Changed numberLimitMax to 20, since setting it to INT_MAX was resulting in segmentation fault at run time. Will check if its working with other activities. Please review.

I looked at the code but it does not implement what I had in mind when proposing this change.
The number limit is for me to be assigned for a single activity, not globally.
For example we should be able to tell that for falling dice we limit the numbers to 5 or 6. And for Multiplication that we limit the multiplication table to the table of 4.
It should not be done in the global configuration.

Ok. But in the link here: https://phabricator.kde.org/T3173, the topic was changed to providing a global configuration. I guess this needs further discussion.

jjazeix requested changes to this revision.Apr 7 2018, 7:38 AM

For me, I agree having a configuration per activity would be more useful than a global one. Not sure about the number of impacted activities but if there are only a few ones (do the math tables activities are to be updated with this change?). All activities won't have the same bounds so it may be difficult to have one value for all (if addition and multiplication, we can expect the children know the additions up to 10 but the multiplication only up to 5).
For now, smallnumbers2 (the dominoes activity) is an extension of gletters which already have a configuration (for letters).
The diff should focus on adding a specific configuration for both smallnumbers and smallnumbers2.

This revision now requires changes to proceed.Apr 7 2018, 7:38 AM

We could do something like this.
Having an option in the global setting configuration menu to turn on or turn off "Detailed (or customised) target set for certain activity".
Then in individual activity we would see an additional config icon allowing to set the limitation number or limit the range of questions that would be asked.
Activities that could have a custom setting:
number with dices: (number limited)
learning clocks (limiting to full hours - half - quarters) - (asking hours > than 12 eg: 23:43) etc...
Division (in this case the config dialog needs to be presented in a table way to choose the values presented to the user, this work as been started by nitish)
Guess a number
Number with dominos
Even and Odd Numbers
Numbers in order
Reverscount
ClickOnUppercase (here again with a table which allows to select the letters to be worked with)
ClickOnLowerCase

We could do something like this.
Having an option in the global setting configuration menu to turn on or turn off "Detailed (or customised) target set for certain activity".
Then in individual activity we would see an additional config icon allowing to set the limitation number or limit the range of questions that would be asked.
Activities that could have a custom setting:
number with dices: (number limited)
learning clocks (limiting to full hours - half - quarters) - (asking hours > than 12 eg: 23:43) etc...
Division (in this case the config dialog needs to be presented in a table way to choose the values presented to the user, this work as been started by nitish)
Guess a number
Number with dominos
Even and Odd Numbers
Numbers in order
Reverscount
ClickOnUppercase (here again with a table which allows to select the letters to be worked with)
ClickOnLowerCase

I think it's too complicated for the children to do this and will be duplicated when we'll do the server. It will be mostly useful for classrooms when a teacher will be present to select the ranges but not at home.

True, this was in my mind a temporary solution until we get the server. This is for the classroom indeed.
Maybe we should not loose time then on this and keep our energy for the server instead.

So should I change this instead to be a boolean variable, like "Use Custom Config" or such?

So should I change this instead to be a boolean variable, like "Use Custom Config" or such?

Not a global config, you can do a specific variable for smallnumbers and smallnumbers2 activities as it was intended at the beginning and use it

I observed that level 2 in both smallnumbers and smallnumbers2 contained values between 5-9 only. So, should those values change too, or are those kept to introduce the numbers, and only level 3 should be affected?

rohitdas updated this revision to Diff 31885.Apr 11 2018, 1:59 PM

Changes made as suggested. Please review.

With your change, even gletters will have this setting displayed and smallnumbers will have the locale and uppercase mode displayed.
We don't really want it as these settings are not relevant for these activities.
Also, the limits for the smallnumbers and smallnumbers2 should be dependant of the max possible for each activity, not a value that can't be reached.

rohitdas updated this revision to Diff 32056.Apr 13 2018, 11:55 AM

Hidden numberLimit in gletters. Please review.

When using GSlider in smallnumber I can not see the max number I have set.

I can also see the upercase within smallnumbers which does not make sens.

jjazeix added a comment.EditedApr 14 2018, 9:47 AM

When adding new code in an activity please try to refactor and remove what is no more useful

src/activities/gletters/Gletters.qml
171

why 10?

178

maximum number limit

rohitdas added inline comments.Apr 15 2018, 5:21 AM
src/activities/gletters/Gletters.qml
171

I used 10 because the maximum number of dots on dice or dominoes available is 9.

rohitdas updated this revision to Diff 32272.Apr 16 2018, 11:45 AM

Changes made as requested. Please review.

jjazeix added inline comments.Apr 16 2018, 12:11 PM
src/activities/gletters/Gletters.qml
176

do you really need to update it there? Shouldn't the binding do the job by itself?

181

qsTr().arg

rohitdas updated this revision to Diff 32276.Apr 16 2018, 1:09 PM

Changes made. Please review.

@rohitdas sorry for the long delay but on my side, I think it will be better to implement it via the multiple dataset (basically, it will mean adding a new abstraction level to let the child choose the difficulty he wants to play with. We can have one dataset with number between 1 and 3, another one between 2 and 6...). For now, multiple_dataset is still a WIP (https://cgit.kde.org/gcompris.git/log/?h=multiple_dataset) but once available, it will provide more separated difficulty levels for children.

@echarruau I'm really late to reply to this diff but is it still useful to limit knowing that we will change to multiple dataset soon or should we drop this?

Restricted Application edited subscribers, added: kde-edu; removed: KDE Edu. · View Herald TranscriptJan 2 2019, 10:38 AM

@jjazeix For me it is important that GC contributor know that we are going in the multidataset direction. You have a working version of Reversecount and I have a working version of algebra_by (I have to port the other algebra before to release it).
As they are in a separate branch, there would be no problem for our community to start to experiment.
What needs to be clear is that we did not reach a final architecture for multidataset.
We are in a on progress mode and the final architecture is not known, they will have to adapt their activities to evoluate with our experimentation.