Merge pull request #539 from scratchcpp/fix_sound_clone_playback

Fix #538: Fix playback of sound clones

Author: adazem009

include/scratchcpp/sound.h

@@ -9,6 +9,7 @@
 namespace libscratchcpp
 {
 
+class Target;
 class SoundPrivate;
 
 /*! \brief The Sound class represents a Scratch sound. */
@@ -32,12 +33,17 @@ class LIBSCRATCHCPP_EXPORT Sound : public Asset
 
         virtual bool isPlaying();
 
+        Target *target() const;
+        void setTarget(Target *target);
+
         std::shared_ptr<Sound> clone() const;
 
     protected:
         void processData(unsigned int size, void *data) override;
 
     private:
+        void stopCloneSounds();
+
         spimpl::unique_impl_ptr<SoundPrivate> impl;
 };
 

src/engine/internal/engine.cpp

@@ -459,14 +459,11 @@ void Engine::deinitClone(std::shared_ptr<Sprite> clone)
 
 void Engine::stopSounds()
 {
-    // TODO: Loop through clones
     for (auto target : m_targets) {
         const auto &sounds = target->sounds();
 
-        for (auto sound : sounds) {
-            if (sound->isPlaying())
-                sound->stop();
-        }
+        for (auto sound : sounds)
+            sound->stop();
     }
 }
 

src/scratch/sound.cpp

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 #include <scratchcpp/sound.h>
+#include <scratchcpp/sprite.h>
 #include <iostream>
 
 #include "sound_p.h"
@@ -47,12 +48,16 @@ void Sound::setVolume(double volume)
 /*! Starts the playback of the sound. */
 void Sound::start()
 {
+    // Stop sounds in clones (#538)
+    stopCloneSounds();
     impl->player->start();
 }
 
 /*! Stops the playback of the sound. */
 void Sound::stop()
 {
+    // Stop sounds in clones (#538)
+    stopCloneSounds();
     impl->player->stop();
 }
 
@@ -62,6 +67,18 @@ bool Sound::isPlaying()
     return impl->player->isPlaying();
 }
 
+/*! Returns the sprite or stage this variable belongs to. */
+Target *Sound::target() const
+{
+    return impl->target;
+}
+
+/*! Sets the sprite or stage this variable belongs to. */
+void Sound::setTarget(Target *target)
+{
+    impl->target = target;
+}
+
 /*! Returns an independent copy of the sound which is valid for as long as the original sound exists. */
 std::shared_ptr<Sound> Sound::clone() const
 {
@@ -83,3 +100,30 @@ void Sound::processData(unsigned int size, void *data)
     if (!impl->player->load(size, data, impl->rate))
         std::cerr << "Failed to load sound " << name() << std::endl;
 }
+
+void Sound::stopCloneSounds()
+{
+    if (impl->target && !impl->target->isStage()) {
+        Sprite *sprite = static_cast<Sprite *>(impl->target);
+
+        if (sprite->isClone())
+            sprite = sprite->cloneSprite();
+
+        // Stop the sound in the sprite (clone root)
+        auto sound = sprite->soundAt(sprite->findSound(name()));
+
+        if (sound && sound.get() != this)
+            sound->impl->player->stop();
+
+        // Stop the sound in clones
+        const auto &clones = sprite->clones();
+
+        for (auto clone : clones) {
+            auto sound = clone->soundAt(clone->findSound(name()));
+            assert(sound);
+
+            if (sound && sound.get() != this)
+                sound->impl->player->stop();
+        }
+    }
+}

src/scratch/sound_p.h

@@ -10,6 +10,8 @@
 namespace libscratchcpp
 {
 
+class Target;
+
 struct SoundPrivate
 {
         SoundPrivate();
@@ -19,6 +21,7 @@ struct SoundPrivate
         int sampleCount = 0;
         static IAudioOutput *audioOutput;
         std::shared_ptr<IAudioPlayer> player = nullptr;
+        Target *target = nullptr;
 };
 
 } // namespace libscratchcpp

src/scratch/target.cpp

@@ -344,8 +344,10 @@ int Target::addSound(std::shared_ptr<Sound> sound)
 
     assert(sound);
 
-    if (sound)
+    if (sound) {
+        sound->setTarget(this);
         sound->setVolume(impl->volume);
+    }
 
     impl->sounds.push_back(sound);
     return impl->sounds.size() - 1;

test/assets/sound_test.cpp

@@ -1,7 +1,9 @@
 #include <scratchcpp/sound.h>
+#include <scratchcpp/sprite.h>
 #include <scratch/sound_p.h>
 #include <audiooutputmock.h>
 #include <audioplayermock.h>
+#include <enginemock.h>
 
 #include "../common.h"
 
@@ -106,30 +108,78 @@ TEST_F(SoundTest, IsPlaying)
     SoundPrivate::audioOutput = nullptr;
 }
 
-TEST_F(SoundTest, Clone)
+TEST_F(SoundTest, Target)
 {
     Sound sound("sound1", "a", "wav");
-    sound.setRate(44100);
-    sound.setSampleCount(10000);
+    ASSERT_EQ(sound.target(), nullptr);
+
+    Target target;
+    sound.setTarget(&target);
+    ASSERT_EQ(sound.target(), &target);
+}
+
+TEST_F(SoundTest, Clone)
+{
+    auto sound = std::make_shared<Sound>("sound1", "a", "wav");
+    sound->setRate(44100);
+    sound->setSampleCount(10000);
 
     const char *data = "abc";
     void *dataPtr = const_cast<void *>(static_cast<const void *>(data));
 
     EXPECT_CALL(*m_player, isLoaded()).WillOnce(Return(false));
     EXPECT_CALL(*m_player, load(3, dataPtr, 44100)).WillOnce(Return(true));
-    sound.setData(3, dataPtr);
+    sound->setData(3, dataPtr);
 
     auto clonePlayer = std::make_shared<AudioPlayerMock>();
     EXPECT_CALL(m_playerFactory, createAudioPlayer()).WillOnce(Return(clonePlayer));
     EXPECT_CALL(*clonePlayer, loadCopy(m_player.get())).WillOnce(Return(true));
     EXPECT_CALL(*m_player, volume()).WillOnce(Return(0.45));
     EXPECT_CALL(*clonePlayer, setVolume(0.45));
     EXPECT_CALL(*clonePlayer, isLoaded()).WillOnce(Return(true));
-    auto clone = sound.clone();
+    auto clone = sound->clone();
     ASSERT_TRUE(clone);
-    ASSERT_EQ(clone->name(), sound.name());
-    ASSERT_EQ(clone->id(), sound.id());
-    ASSERT_EQ(clone->dataFormat(), sound.dataFormat());
-    ASSERT_EQ(clone->rate(), sound.rate());
-    ASSERT_EQ(clone->sampleCount(), sound.sampleCount());
+    ASSERT_EQ(clone->name(), sound->name());
+    ASSERT_EQ(clone->id(), sound->id());
+    ASSERT_EQ(clone->dataFormat(), sound->dataFormat());
+    ASSERT_EQ(clone->rate(), sound->rate());
+    ASSERT_EQ(clone->sampleCount(), sound->sampleCount());
+
+    // Stopping/starting the sound should stop its clones
+    auto anotherPlayer = std::make_shared<AudioPlayerMock>();
+    EXPECT_CALL(m_playerFactory, createAudioPlayer()).WillOnce(Return(anotherPlayer));
+    auto another = std::make_shared<Sound>("another", "c", "mp3");
+    Sprite sprite;
+    EngineMock engine;
+    sprite.setEngine(&engine);
+
+    EXPECT_CALL(engine, cloneLimit()).WillOnce(Return(-1));
+    EXPECT_CALL(engine, initClone);
+    EXPECT_CALL(engine, requestRedraw);
+    EXPECT_CALL(engine, moveSpriteBehindOther);
+    auto spriteClone = sprite.clone();
+
+    EXPECT_CALL(*anotherPlayer, setVolume).Times(2);
+    EXPECT_CALL(*m_player, setVolume);
+    EXPECT_CALL(*clonePlayer, setVolume);
+    sprite.addSound(another);
+    sprite.addSound(sound);
+    spriteClone->addSound(another);
+    spriteClone->addSound(clone);
+
+    EXPECT_CALL(*m_player, stop());
+    EXPECT_CALL(*clonePlayer, stop());
+    sound->stop();
+
+    EXPECT_CALL(*m_player, stop());
+    EXPECT_CALL(*clonePlayer, stop());
+    clone->stop();
+
+    EXPECT_CALL(*m_player, start());
+    EXPECT_CALL(*clonePlayer, stop());
+    sound->start();
+
+    EXPECT_CALL(*m_player, stop());
+    EXPECT_CALL(*clonePlayer, start());
+    clone->start();
 }

test/blocks/sound_blocks_test.cpp

@@ -2,12 +2,12 @@
 #include <scratchcpp/block.h>
 #include <scratchcpp/input.h>
 #include <scratchcpp/field.h>
-#include <scratchcpp/target.h>
 #include <scratchcpp/sound.h>
 #include <scratch/sound_p.h>
 #include <enginemock.h>
 #include <audiooutputmock.h>
 #include <audioplayermock.h>
+#include <targetmock.h>
 
 #include "../common.h"
 #include "blocks/soundblocks.h"
@@ -212,7 +212,8 @@ TEST_F(SoundBlocksTest, PlayImpl)
     EXPECT_CALL(*player1, setVolume);
     EXPECT_CALL(*player2, setVolume);
     EXPECT_CALL(*player3, setVolume);
-    Target target;
+    TargetMock target;
+    EXPECT_CALL(target, isStage()).WillRepeatedly(Return(true));
     target.addSound(std::make_shared<Sound>("some sound", "", ""));
     target.addSound(std::make_shared<Sound>("test", "", ""));
     target.addSound(std::make_shared<Sound>("another sound", "", ""));
@@ -412,7 +413,8 @@ TEST_F(SoundBlocksTest, PlayUntilDoneImpl)
     EXPECT_CALL(*player1, setVolume);
     EXPECT_CALL(*player2, setVolume);
     EXPECT_CALL(*player3, setVolume);
-    Target target;
+    TargetMock target;
+    EXPECT_CALL(target, isStage()).WillRepeatedly(Return(true));
     target.addSound(std::make_shared<Sound>("some sound", "", ""));
     target.addSound(std::make_shared<Sound>("test", "", ""));
     target.addSound(std::make_shared<Sound>("another sound", "", ""));

test/engine/engine_test.cpp

@@ -275,9 +275,7 @@ TEST(EngineTest, StopSounds)
 
     auto engine = p.engine();
 
-    EXPECT_CALL(*player1, isPlaying()).WillOnce(Return(false));
-    EXPECT_CALL(*player2, isPlaying()).WillOnce(Return(true));
-    EXPECT_CALL(*player3, isPlaying()).WillOnce(Return(true));
+    EXPECT_CALL(*player1, stop());
     EXPECT_CALL(*player2, stop());
     EXPECT_CALL(*player3, stop());
     engine->stopSounds();

test/scratch_classes/target_test.cpp

@@ -442,6 +442,10 @@ TEST(TargetTest, Sounds)
     ASSERT_EQ(target.findSound("sound2"), 1);
     ASSERT_EQ(target.findSound("sound3"), 2);
 
+    ASSERT_EQ(s1->target(), &target);
+    ASSERT_EQ(s2->target(), &target);
+    ASSERT_EQ(s3->target(), &target);
+
     SoundPrivate::audioOutput = nullptr;
 }