Port to Python 3

[JuiP]

Tested; Apart from the two issues I didnot notice differences in python3 branch and master.
Differences between this branch and master-

• After selecting the correct object, master displays a smile emoji but after port to python3 seen as a question mark. Still haven't found the cause
• A Pango-Warning is seen after ff4f92c
  Invalid UTF-8 string passed to pango_layout_set_text()

Observed this before port to python3
In master:

• Collaboration does not include turns so joiner can only join the activity but treated as two separate games. Collaboration makes no sense if there are no turns.

After porting ; In python3 branch:

• yet to test if collaboration is same as in master

@quozl @chimosky Please review changes!

[JuiP]

Tested Collaboration on the branch python3, logs are clean for the sharer and is same as master, but got this traceback for the joiner:

Traceback (most recent call last):
  File "/usr/share/sugar/activities/recall.activity/game.py", line 166, in _dance_step
    self._new_game()
  File "/usr/share/sugar/activities/recall.activity/game.py", line 264, in _new_game
    self._parent.send_new_game()
  File "/usr/share/sugar/activities/recall.activity/RecallActivity.py", line 197, in send_new_game
    payload = json_dump(self._game.save_game())
  File "/usr/share/sugar/activities/recall.activity/utils.py", line 52, in json_dump
    jdump(data, _io)
  File "/usr/lib/python2.7/json/__init__.py", line 190, in dump
    fp.write(chunk)
TypeError: unicode argument expected, got 'str' 

What perplexed me was the line: File "/usr/lib/python2.7/json/__init__.py", line 190, in dump when I had ported the activity to python 3 why is it still using python2.7 ? I checked, both the VMs I used to test, git branch is set to python3

[quozl]

Do remember to restart Sugar. At the moment Sugar does not detect change to exec in activity.info. The info files are loaded when Sugar starts or a new directory appears. Alternatively, rename the directory to some place other than /usr/share/sugar/activities, wait a second, then rename it back. Restarting Sugar is in the Python 3 Porting Guide.

[chimosky]

When you say " Collaboration does not include turns so joiner can only join the activity but treated as two separate games. Collaboration makes no sense if there are no turns." do you mean the game state isn't shared?

[JuiP]

When you say " Collaboration does not include turns so joiner can only join the activity but treated as two separate games. Collaboration makes no sense if there are no turns." do you mean the game state isn't shared?

@chimosky Yes, also I had seen the code, it does not include "having turns" , like paths/ memorize/ sugarchess defines a variable and stores boolean value to decide who plays when the game board is shared

[quozl]

Some activities were written by copying the source code of another activity and hacking on it until it worked well enough. That can cause activities that seem to have a collaboration feature but don't. Do you think this might be that scenario?

[JuiP]

Some activities were written by copying the source code of another activity and hacking on it until it worked well enough. That can cause activities that seem to have a collaboration feature but don't. Do you think this might be that scenario?

Well yes, because all one can do is join the activity.

[quozl]

Thanks. I'm not familiar with the activity user experience or data model. Best I've found is app store narrative. I've not worked on the activity before.

In the repository commit history, a133c0a was a widespread change to how collaboration worked. Is it possible that this change broke collaboration?

After resolving the above, do you think it makes sense for the activity to support collaboration?

[JuiP]

I'm not familiar with the activity user experience or data model. Best I've found is app store narrative. I've not worked on the activity before.

It mentions: "The fourth game prompts the user to identify the image which had not appeared in the grid. This time, the images differ by shape and color. Again, it starts easy, with just three images to remember, but gets very challenging as the number of images increases."
This fourth-game is not implemented, you can see the differences in the icons on toolbar in the screenshots I added in 81dacb3 and the one on the app store

In the repository commit history, a133c0a was a widespread change to how collaboration worked. Is it possible that this change broke collaboration?

This may or may not be the case, I need to test the repository at that point to know for sure. On a quick look, I don't see a variable that acts like a turns variable in the code that was changed.

After resolving the above, do you think it makes sense for the activity to support collaboration?

Honestly, if there is no score board, the collaboration doesn't make sense in this activity (there is no incentive for a child to play more attentively and give the correct answer). It is like playing an individual game but just waiting for the other person to take their turn. The activity is just like a set of questions to be answered and it makes no difference to the child whether he has to answer all the questions or have alternate questions answered by the other player.

[quozl]

Thanks. Tested abf1183.

• the toolbar grows at the same time "Recall which image was repeated" is shown,
• there's a Pango-WARNING in logs, invalid UTF-8 string,

I agree it doesn't make sense to have collaboration.

In looking at history and source code I found "dotlist" was in the metadata, and I remember seeing that in another activity. A search shows where else it can be found.

So I constructed a timeline of the activities that mention it;

• 2011-12-07 13:13:18, in Reflection, the initial commit sugarlabs/reflection@3977534 did not have dotlist, or collaboration,
• 2011-12-08 03:37:35 in Reflection, dotlist was added in sugarlabs/reflection@c9bdaca
• 2011-12-08 05:53:54 in Reflection, sharing was added in sugarlabs/reflection@010be3f
• 2011-12-09 08:40:09 in Flip, the initial commit most likely came from Reflection as a copy and change, see sugarlabs/flip@68b6933
• 2011-12-23 11:34:23 in Cookie Search, the initial commit had dotlist, see sugarlabs/cookie-search-activity@3f8b848
• 2012-02-05 17:13:44 in Yupana, the initial commit had dotlist, see sugarlabs/yupana@0499312
• 2012-05-10 05:28:58 in Story, the initial commit had dotlist, see sugarlabs/story@3cf9ca0
• 2012-05-11 09:33:57 in Recall, the initial commit had dotlist, see 3e9aa44

Also related to sugarlabs/cookie-search-activity#18.

[Saumya-Mishra9129]

@JuiP you can cherry-pick 051dd9c. I have solved

A Pango-Warning is seen after ff4f92c
Invalid UTF-8 string passed to pango_layout_set_text()

And also after solving I didnt get again-

After selecting the correct object, master displays a smile emoji but after port to python3 seen as a question mark. Still haven't found the cause

For reference I looked at GNOME DEVELOPERS page and found out that second argument taken by set_text (length) is maximum length of text , in bytes. And -1 indicates that the string is nul-terminated and the length should be calculated. So I tried using -1 there as it is also used in ruler and it worked.Thanks.

[chimosky]

Honestly, if there is no score board, the collaboration doesn't make sense in this activity (there is no incentive for a child to play more attentively and give the correct answer). It is like playing an individual game but just waiting for the other person to take their turn. The activity is just like a set of questions to be answered and it makes no difference to the child whether he has to answer all the questions or have alternate questions answered by the other player.

I agree.

• the toolbar grows at the same time "Recall which image was repeated" is shown,

Yes noticed this too, removing the line wrap set on the label fixes it.

• there's a Pango-WARNING in logs, invalid UTF-8 string,

@JuiP You should update sprites.py and make necessary fixes.

[JuiP]

Thanks @Saumya-Mishra9129 done in 3e21299

[JuiP]

Honestly, if there is no score board, the collaboration doesn't make sense in this activity (there is no incentive for a child to play more attentively and give the correct answer). It is like playing an individual game but just waiting for the other person to take their turn. The activity is just like a set of questions to be answered and it makes no difference to the child whether he has to answer all the questions or have alternate questions answered by the other player.

I agree it doesn't make sense to have collaboration. (quozl)

I agree. (chimosky)

About collaboration, I think it is better to keep the status of collaboration as it is at the moment. Later, maybe we could add scoreboard feature so that it is more meaningful.
@walterbender @quozl @chimosky what do you suggest? Did you have anything specific in mind when you created this activity?

[walterbender]

I wonder if it might be fun to have the players have to cooperate to decide on an answer... The game is quite challenging as it progresses.

[quozl]

The next release will be the first release to let the activity run on Sugar again after the Python 3 transition, so for the moment I think it is fine for collaboration to be functionally unchanged; it doesn't work in the latest release, so it is fine if it doesn't work in the next release. I don't think the collaboration code should be ported. So I suggest ripping it out. If it makes sense to add collaboration, that can be in another release. I don't think it will save any time to keep broken collaboration code in the source.

See branch inhume-sharing for commits against master that remove collaboration. I was able to rebase your latest commit as 5533c49 after resolving a few conflicts. If you wish to proceed on this basis, you might try rebasing as I did, or at least compare 5533c49 against master.

[JuiP]

Tested. Ready to be merged :) @quozl @chimosky

[srevinsaju]

Great; No errors

---

@JuiP Is collaboration fixed or is it planned for a lter release as @quozl suggested?

[JuiP]

Thanks for testing @srevinsaju! :)

@JuiP Is collaboration fixed or is it planned for a lter release as @quozl suggested?

Planned for a later release for now

[srevinsaju]

@JuiP Thanks for the quick reply.

In that case; I would be more glad if you change this to

self.max_participants = 1

This would disable the collaboration button too; This removes the confusion if collaboration is implemented or not; from the users' side

[JuiP]

@srevinsaju It has been done in 666da4c. You can see if you try using that button, then it gets disabled. If you are referring to your PR in iknowSriLanka then the button implementation was a little different than in this activity.

[srevinsaju]

No, I am not talking about my implementation; setting self.max_participants had been quite a standard way to disable the collaboration button; apparently, its not getting disabled due to some reason as it should. I will check into the code

[srevinsaju]

@JuiP, although setting

self.max_participants = 1

at the __init__, you might have forgotten to remove

self.max_participants = 4

on L78

https://github.com/JuiP/recall/blob/python3/RecallActivity.py#L75-L80

    def _setup_toolbars(self, have_toolbox):
        """ Setup the toolbars. """

        self.max_participants = 4
        toolbox = ToolbarBox()

[JuiP]

@srevinsaju Thanks for noticing! I have done it in 3f7a927

[srevinsaju]

@JuiP, I guess; we could remove this instance of self.max_participants if its possible to. then this PR would be perfect!

[chimosky]

Review, thanks.