gcompris / GCompris-qt

GCompris in Qt Quick - Mirror of https://invent.kde.org/education/gcompris
225 stars 169 forks source link

Bargame port completed #160

Closed iamutkarshtiwari closed 7 years ago

iamutkarshtiwari commented 8 years ago

Here is the ported QtQuick version of the Bargame activity (originally written in GTK+).

Link -https://phabricator.kde.org/T1538

iamutkarshtiwari commented 8 years ago

@petitlapin We can make this game playable in forced-landscape mode in Android phones but I don't know how to add it. :/

iamutkarshtiwari commented 8 years ago

@petitlapin Please have a look at the new commit and review my comments :)

iamutkarshtiwari commented 8 years ago

Hi @petitlapin, I have added the configuration dialog where user can set the difficulty level. I haven't disturbed the data values(variables) inside js file because moving them to qml file introduces some anomalous behaviour which causes the activity to crash. I haved tried to clean up the code a bit and removed some unused bindings.

P.S.- Trying to fix the bindings requires moving the data values to qml file which disturbs the interdependency of variables and bindings which might introduce unnecessary bugs

iamutkarshtiwari commented 8 years ago

Shall I move all these variable -

var numberBalls = [[1, 4], [2, 6], [3, 6]]
var boardSize = [15, 19, 29]
var sampleBallsNumber = [4, 6, 6]
var tuxPositionFactor = [1.05, 1.4, 1.05]
var ballSizeFactor = [0, 4, 4]
var elementSizeFactor = [0, 4, 7]
var moveCount = -1
var level = 1
var maxlevel = 4
var sublevel = 1
var listWin = []
var items
var numberOfBalls = 1

var url= "qrc:/gcompris/src/activities/bargame/resource/";

from JS file to QML file in order to make all the bindings work?

iamutkarshtiwari commented 8 years ago

Fixes- 1) Bindings fixed 2) scores added (top-right corner) 3) Ok svg changed 4) Level doesn't change on completion 5) configuration added to change levels

stefant29 commented 8 years ago

Hi! While testing Bargame, i noticed some points you could improve:

  1. User's "balls" appear under the bar buttons
  2. Help menu: "prerequisite: brain" seems wrong: there should be something like "ability to count" or something more explicit.
  3. Add a description in the "Help Menu" for the button that chooses how many "balls" the user wants to place: present how it works (e.g. "you have to press it to change the number of balls you want to place")
  4. After the user's move, there should be a little delay, showing that the computer "thinks" for a second. Now Tux's answer is immediate and it doesn't give an overall good look .
  5. In portrait mode, or when resizing the width of the window to a minimum, the "count button" goes behind the bar buttons. In this view, the "OK" button is very small and the image loses its aspect ratio.
  6. The "balls" have a gradient that differentiate them from the background. Removing the gradient in inkscape or any other photo editor app would be an easy solution.

Apart from that, you did a good job on the visuals (haven't had time to check the implementation (the code). Keep it up!

Stefan

iamutkarshtiwari commented 8 years ago

@stefant29 @petitlapin Thank you for the reviews :) I'll soon modify the patch based on the above specified requirements and will update you accordingly.

iamutkarshtiwari commented 7 years ago

@petitlapin Please take a look on the newer commits and try the 2 player mode.

iamutkarshtiwari commented 7 years ago

@petitlapin I have fixed most of the issues mentioned above. Major once -

Moving all the array based data into a single array will create a lot of confusion. And that's something we should avoid :)

iamutkarshtiwari commented 7 years ago

@petitlapin Fixed the aspect ratios of the image to look better in portrait mode as well. Re-positioned the game elements for better space utilization.

@stefant29 Could you please help me remove gradients from the images? I am not much familiar with inkscape.

rahulyadav170923 commented 7 years ago

@iamutkarshtiwari

  1. the pointer for increasing no. of balls is faulty for medium and hard mode .
  2. for the pointer there should be increase and decrease arrows (personal opinion)(check with your mentors also) 3.you shouldn't allow more balls to be put when there less space even at the end. this might create confusion.
rahulyadav170923 commented 7 years ago

@iamutkarshtiwari 1 .you have chosen a cross and blue circle to mark the players. I think it should be the the colours of the balls being used ( ask you mentors also)

  1. Put some space btw Ok and the ball counter .
  2. arrange the ball and counter inside the rectangle.
iamutkarshtiwari commented 7 years ago

@petitlapin Please test the latest commit both on Android (apk) and Desktop

petitlapin commented 7 years ago

I created a patch to apply to last version: https://drive.google.com/open?id=0B1Wylu9YXKcKNVJwendTUXJqWUk I started it before your last commits so maybe there are some differences. We can discuss about it, you don't have to accept everything blindly :).

For the images/style, don't bother too much, we'll see with Timothée once merged. I agree with the cross/circle being changed to the good color but they'll probably change (they are too close for now) so I don't think it's necessary to update them now (except if you want to play a little with inkscape of course). So globally, I'm satisfied with it :). There is one last point that I'd like to try: in vertical mode (on phone), it's quite difficult to see the balls on hard mode (29 to fill on a small place). Can you test to have the bar vertically and see if it renders well or if we keep like now?

iamutkarshtiwari commented 7 years ago

@petitlapin Portrait mode is now improved. Please test it on an Android device and let me know your suggestions.

stefant29 commented 7 years ago

hi! did you solve your "gradient" problems?

iamutkarshtiwari commented 7 years ago

@stefant29 I didn't, actually couldn't :/ Could you please do it for me? I can share the two svgs for the blue and green balls.

stefant29 commented 7 years ago

send me a private message on irc (StefanT29_) and we'll see if i can do something about it

iamutkarshtiwari commented 7 years ago

@petitlapin Manual merge done 👍

iamutkarshtiwari commented 7 years ago

@Animtim Please review this patch

petitlapin commented 7 years ago

Hi, I merged it in master. I did other changes on your code, please check them.

Thank you for the activities,

Johnny