Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CATTY-552 Clone object or actor #1686

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

amelak9
Copy link
Contributor

@amelak9 amelak9 commented Aug 6, 2021

To introduce the "clone" metaphor as available in Catroid and Scratch, a new Brick should be developed to clone (a) the current object; or, (b) any other object except the background.
The Brick should be called "Create clone of" and it should be available at the Event category.

In the UI the Brick should contain a dropdown where the user can select between the following values:
yourself
the name of any other object, except the background
When executed, the object for which the Brick is being executed should get cloned. A clone should have the exact same position, scripts, bricks and all other effects which have already been applied to the object which is being cloned. Also, the local variables should be copied so that they (a) evaluate to the same value; but are (b) again local variables available for the clone only.
Later a new script should be triggered whenever a clone is created ("When you start as a clone"), but this is not part of that ticket. However, make sure to think about that and add the cloned object to the scheduler, so that any scripts are being executed (e.g., the "When tapped" script).

  • Include the name of the Jira ticket in the PR’s title
  • Verify that the Jira ticket is in the status Ready for Development
  • Include a summary of the changes plus the relevant context
  • Choose the proper base branch (develop)
  • Confirm that the changes follow the project’s coding guidelines
  • Verify that the changes generate no compiler or linter warnings
  • Perform a self-review of the changes
  • Verify to commit no other files than the intentionally changed ones
  • Include reasonable and readable tests verifying the added or changed behavior
  • Confirm that new and existing unit tests pass locally
  • Check that the commits’ message style matches the project’s guideline
  • Stick to the project’s git workflow (rebase and squash your commits)
  • Verify that your changes do not have any conflicts with the base branch
  • After the PR, verify that all CI checks have passed
  • Post a message in the #catty Slack channel and ask for a code reviewer

@amelak9 amelak9 requested a review from srinner August 6, 2021 13:14
@amelak9 amelak9 force-pushed the CATTY-552-PPPP-Clone-object-or-actor branch 4 times, most recently from 874c252 to 734157d Compare August 7, 2021 12:54
@amelak9 amelak9 force-pushed the CATTY-552-PPPP-Clone-object-or-actor branch from 9d9c4bc to db63258 Compare August 14, 2021 14:35
Copy link
Member

@m-herold m-herold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please compare the behavior of this simple program with Catroid: https://share.catrob.at/app/project/1271b6d4-4d3e-11ec-86de-005056a32daa

It seems like there is no clone on Catty.

@srinner srinner force-pushed the CATTY-552-PPPP-Clone-object-or-actor branch from db63258 to c460c7f Compare November 26, 2021 13:42
@srinner srinner force-pushed the CATTY-552-PPPP-Clone-object-or-actor branch from c460c7f to eceedf7 Compare January 25, 2022 17:05
@srinner srinner force-pushed the CATTY-552-PPPP-Clone-object-or-actor branch 3 times, most recently from 808bd2e to 8f713a7 Compare February 22, 2022 15:53
Copy link
Contributor

@wallisch wallisch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, please don't use conditional statements without brackets when they only have one line, because errors can easily happen if in the future somebody wants to add a line.

And don't forget to update the license header year to 2022 for new files (PR for existing files will be merged soon)


for brick in oldScript.brickList {

switch brick {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should really make the Bricks, Scripts and Variables conform to NSCopying and then just call .copy() on generic objects instead of making gigantic switch cases. It's complicated to understand and this file would have to be edited for every new brick in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a "clone" function instead of using the copy functions. Because I pass the script as parameter and so I can set the member variables for the bricks. Some bricks need uservariable, userlist or other bricks and with the script I can set all these member var.

@wallisch
Copy link
Contributor

Jenkins please

@srinner srinner force-pushed the CATTY-552-PPPP-Clone-object-or-actor branch from 8f713a7 to bb9ddfb Compare March 3, 2022 13:42
@wallisch
Copy link
Contributor

wallisch commented Mar 3, 2022

Jenkins, retest this please

@wallisch
Copy link
Contributor

wallisch commented Mar 3, 2022

One more try, retest this please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants