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.

Sign up for GitHub

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

Jump to bottom

CATROID-1424 'Start sound PULLDOWN at XX seconds' brick #4566

Merged
merged 1 commit into from
Jul 20, 2022
Merged

CATROID-1424 'Start sound PULLDOWN at XX seconds' brick #4566

merged 1 commit into from
Jul 20, 2022

Conversation

bigsmallpp
Copy link
Contributor

@bigsmallpp bigsmallpp commented May 22, 2022 •
edited
Loading

Catroid-1424
Created the 'Start sound PULLDOWN at XX seconds' brick, which allows you to select a soundfile and play it with a selected offset (in seconds). For instance, if a soundfile is 2 secoonds long and the offset is 0.5 seconds the soundfile will play from 0.5 seconds to the 2 seconds marker.
Added according JUnit tests for the brick.

Your checklist for this pull request

Please review the contributing guidelines and wiki pages of this repository.

  • Include the name of the Jira ticket in the PR’s title
  • 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 gitflow workflow
  • 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 catroid-stage or catroid-ide Slack channel and ask for a code reviewer

@bigsmallpp bigsmallpp force-pushed the CATROID-1424 branch 3 times, most recently from 43c4286 to cc99719 Compare May 30, 2022 12:18
Rawbat
Rawbat requested changes Jun 7, 2022
View reviewed changes
Copy link
Contributor

@Rawbat Rawbat left a comment

Choose a reason for hiding this comment

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

Looks good, just a few minor remarks: Please have a look at the BricksHelpUrlTest as well as the BrickCategoryTest as they are now failing because of the new brick. Also please change the copyright in all edited files to 2022.

catroid/src/main/java/org/catrobat/catroid/content/actions/PlaySoundAtAction.kt Outdated
lateinit var offset: Formula
var sound: SoundInfo? = null
var scope: Scope? = null
var soundManagerMocked: SoundManager? = null
Copy link
Contributor

Choose a reason for hiding this comment

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

This Variable seems to never be used

catroid/src/main/java/org/catrobat/catroid/content/bricks/PlaySoundAtBrick.java Outdated

@Override
public void addActionToSequence(Sprite sprite, ScriptSequenceAction sequence) {
// Check sound file input
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the comment please

@Rawbat
Copy link
Contributor

Rawbat commented Jun 7, 2022

Also please change the title of the PR to: CATROID-1424 'Start sound PULLDOWN at XX seconds' brick

@bigsmallpp bigsmallpp changed the title CATROBAT-1424 'Start sound PULLDOWN at XX seconds' brick CATROID-1424 'Start sound PULLDOWN at XX seconds' brick Jun 9, 2022
@bigsmallpp bigsmallpp force-pushed the CATROID-1424 branch 2 times, most recently from 4c67ba7 to 38bcef9 Compare June 15, 2022 21:51
@bigsmallpp
CATROID-1424 'Start sound PULLDOWN at XX seconds' brick
e3d32cd 
Created layout for brick.
Added fucntionality to brick.
Added JUnit tests.
Fixed Warnings.
Implemented requested changes from pull request.
@bigsmallpp bigsmallpp requested a review from Rawbat June 16, 2022 22:01
Rawbat
Rawbat approved these changes Jun 21, 2022
View reviewed changes
Copy link
Contributor

@Rawbat Rawbat left a comment

Choose a reason for hiding this comment

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

Thanks!

wslany
wslany approved these changes Jul 20, 2022
View reviewed changes
Copy link
Member

@wslany wslany left a comment

Choose a reason for hiding this comment

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

Thanks!

@wslany wslany merged commit 98e1c4e into Catrobat:develop Jul 20, 2022
@wslany
Copy link
Member

wslany commented Jul 21, 2022 •
edited
Loading

Upps, my mistake! The Catrobat Language Version should have been increased with this PR. Added via #4597.

@wslany wslany mentioned this pull request Jul 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rawbat Rawbat Rawbat approved these changes

@wslany wslany wslany approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bigsmallpp @Rawbat @wslany