As was being discussed, tabs don't have a mouse. So please check this diff.
Details
Diff Detail
- Repository
- R2 GCompris
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Hi,
I just applied a patch and got some errors:
qrc:/gcompris/src/activities/penalty/Penalty.qml:51:27: Unable to assign QQuickAnchorLine to QQuickItem*
qrc:/gcompris/src/activities/penalty/Penalty.qml:158:27: Unable to assign QQuickAnchorLine to QQuickItem*
If you click on the ball, it does not work anymore. Also, instructions (ActivityInfo.qml) needs to be updated.
Can you fix them?
Please check the current patch I uploaded. Also the ball will not work when about to be kicked. You will have to double tap or double click on the goal to score.
Sorry, I had uploaded the wrong patch. I should have noticed earlier. Let me know if anything is out of order.
src/activities/penalty/Penalty.qml | ||
---|---|---|
5 | add a space between the type and { (coding style rules) | |
6 | what does this mean? The variable should have understandable names | |
8 | hardcoded values (width, height, x, y), will not work in all devices | |
71 | the code should be factorised, it's the same as above and below. Copy/paste are bad. | |
194 | is it still useful (I haven't read the full code but it seems to be the previous MouseArea for the whole screen)? | |
447 | I guess we can keep the previous final positions, no need to add random |
why are there patch files?
Also the patch does not apply anymore (seems to not be based on master branch code):
git apply penalty.diff
penalty.diff:47: trailing whitespace.
penalty.diff:78: trailing whitespace.
penalty.diff:255: trailing whitespace.
penalty.diff:315: trailing whitespace.
penalty.diff:375: trailing whitespace.
error: patch failed: src/activities/penalty/Penalty.qml:39
error: src/activities/penalty/Penalty.qml: patch does not apply
src/activities/penalty/Penalty.qml | ||
---|---|---|
42 | use real names, not abbreviated ones (and if the name is correct, you don't need a comment to explain it) | |
49 | coding convention, space between () and { (needs to be updated not only there) |
I can't test for now but I have a doubt on the behaviour on this case:
you click first on right then on left side, where does the ball go?
(the potential same issue on previous code was if you do a left click followed by a right one)
Can you test and see if it is ok? The aim of the activity is to teach double click (on same button), so for me it should not work and count as a win.
src/activities/penalty/Penalty.qml | ||
---|---|---|
62 | coding rule. | |
102 | where does this value comes from? Is it the same on every screen resolutions or should it be relative to it? | |
111 | is it necessary to check this and the button list below? | |
113 | do you need to store it in local or can't you directly pass it to the function and store it in ball.state? |
The case you mentioned won't work here as the direction of the ball doesn't depend on the mouse button anymore. So long as you double click or double tap on the corresponding side of the goal, the ball will go. Anything else will fail. Let me know your feedback.
penalty.diff:10: trailing whitespace.
signal stop
penalty.diff:33: trailing whitespace.
}
penalty.diff:52: trailing whitespace.
id: rectLeft
penalty.diff:86: trailing whitespace.
id: rectRight
error: patch failed: src/activities/penalty/Penalty.qml:38
error: src/activities/penalty/Penalty.qml: patch does not apply
I actually had the white spaces removed in a later commit. It seemed it had been included in the patch. I'll have to look more into creating patches. Sorry for the inconvenience. I'll send you the diff instead.
still not working on master:
git apply penalty.patch
error: patch failed: src/activities/penalty/Penalty.qml:39
error: src/activities/penalty/Penalty.qml: patch does not apply
src/activities/penalty/Penalty.qml | ||
---|---|---|
97 | harcoded values? |
Created Diff 7684 as requested. Please review. Sorry for the delays. Busy prepping for placements.