Skip to content

Commit 4c016db

Browse files
adazem009adazem009
authored
Merge pull request #585 from scratchcpp/clamp_effect_values
Fix #584: Clamp effect values in Target
2 parents 17a745d + 6c004ce commit 4c016db

File tree

9 files changed

+81
-36
lines changed

9 files changed

+81
-36
lines changed

‎include/scratchcpp/igraphicseffect.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ class LIBSCRATCHCPP_EXPORT IGraphicsEffect
1717

1818
/*! Returns the name of the graphics effect. */
1919
virtual std::string name() const = 0;
20+
21+
/*! Returns the clamped value of the graphic effect. */
22+
virtual double clamp(double value) const = 0;
2023
};
2124

2225
} // namespace libscratchcpp

‎src/scratch/sound.cpp‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,6 @@
1111

1212
using namespace libscratchcpp;
1313

14-
static std::unordered_map<Sound::Effect, std::pair<double, double>> EFFECT_RANGE = {
15-
{ Sound::Effect::Pitch, { -360, 360 } }, // -3 to 3 octaves
16-
{ Sound::Effect::Pan, { -100, 100 } } // // 100% left to 100% right
17-
};
18-
1914
/*! Constructs Sound. */
2015
Sound::Sound(const std::string &name, const std::string &id, const std::string &format) :
2116
Asset(name, id, format),
@@ -56,13 +51,6 @@ void Sound::setVolume(double volume)
5651
/*! Sets the value of the given sound effect. */
5752
void Sound::setEffect(Effect effect, double value)
5853
{
59-
auto it = EFFECT_RANGE.find(effect);
60-
61-
if (it == EFFECT_RANGE.cend())
62-
return;
63-
64-
value = std::clamp(value, it->second.first, it->second.second);
65-
6654
switch (effect) {
6755
case Effect::Pitch: {
6856
// Convert from linear

‎src/scratch/target.cpp‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <scratchcpp/block.h>
88
#include <scratchcpp/comment.h>
99
#include <scratchcpp/iengine.h>
10+
#include <scratchcpp/igraphicseffect.h>
1011

1112
#include <unordered_set>
1213

@@ -16,6 +17,11 @@ using namespace libscratchcpp;
1617

1718
static const std::unordered_set<std::string> RESERVED_NAMES = { "_mouse_", "_stage_", "_edge_", "_myself_", "_random_" };
1819

20+
static std::unordered_map<Sound::Effect, std::pair<double, double>> SOUND_EFFECT_RANGE = {
21+
{ Sound::Effect::Pitch, { -360, 360 } }, // -3 to 3 octaves
22+
{ Sound::Effect::Pan, { -100, 100 } } // // 100% left to 100% right
23+
};
24+
1925
/*! Constructs target. */
2026
Target::Target() :
2127
impl(spimpl::make_unique_impl<TargetPrivate>())
@@ -436,6 +442,12 @@ double Target::soundEffectValue(Sound::Effect effect) const
436442
/*! Sets the value of the given sound effect. */
437443
void Target::setSoundEffectValue(Sound::Effect effect, double value)
438444
{
445+
auto it = SOUND_EFFECT_RANGE.find(effect);
446+
447+
if (it == SOUND_EFFECT_RANGE.cend())
448+
return;
449+
450+
value = std::clamp(value, it->second.first, it->second.second);
439451
impl->soundEffects[effect] = value;
440452

441453
for (auto sound : impl->sounds) {
@@ -540,7 +552,8 @@ double Target::graphicsEffectValue(IGraphicsEffect *effect) const
540552
/*! Sets the value of the given graphics effect. */
541553
void Target::setGraphicsEffectValue(IGraphicsEffect *effect, double value)
542554
{
543-
impl->graphicsEffects[effect] = value;
555+
assert(effect);
556+
impl->graphicsEffects[effect] = effect->clamp(value);
544557
}
545558

546559
/*! Sets the value of all graphics effects to 0 (clears them). */

‎test/assets/sound_test.cpp‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,6 @@ TEST_F(SoundTest, SetEffect)
8484
Sound sound("sound1", "a", "wav");
8585

8686
// Pitch
87-
EXPECT_CALL(*m_player, setPitch(0.125));
88-
sound.setEffect(Sound::Effect::Pitch, -400);
89-
9087
EXPECT_CALL(*m_player, setPitch(0.125));
9188
sound.setEffect(Sound::Effect::Pitch, -360);
9289

@@ -102,13 +99,7 @@ TEST_F(SoundTest, SetEffect)
10299
EXPECT_CALL(*m_player, setPitch(8));
103100
sound.setEffect(Sound::Effect::Pitch, 360);
104101

105-
EXPECT_CALL(*m_player, setPitch(8));
106-
sound.setEffect(Sound::Effect::Pitch, 400);
107-
108102
// Pan
109-
EXPECT_CALL(*m_player, setPan(-1));
110-
sound.setEffect(Sound::Effect::Pan, -150);
111-
112103
EXPECT_CALL(*m_player, setPan(-1));
113104
sound.setEffect(Sound::Effect::Pan, -100);
114105

@@ -123,9 +114,6 @@ TEST_F(SoundTest, SetEffect)
123114

124115
EXPECT_CALL(*m_player, setPan(1));
125116
sound.setEffect(Sound::Effect::Pan, 100);
126-
127-
EXPECT_CALL(*m_player, setPan(1));
128-
sound.setEffect(Sound::Effect::Pan, 150);
129117
}
130118

131119
TEST_F(SoundTest, Start)

‎test/blocks/looks_blocks_test.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
using namespace libscratchcpp;
2323

2424
using ::testing::Return;
25+
using ::testing::ReturnArg;
26+
using ::testing::_;
2527

2628
class LooksBlocksTest : public testing::Test
2729
{
@@ -857,6 +859,14 @@ static void initEffects()
857859
LooksBlocks::m_mosaicEffect = ScratchConfiguration::getGraphicsEffect("mosaic");
858860
LooksBlocks::m_brightnessEffect = ScratchConfiguration::getGraphicsEffect("brightness");
859861
LooksBlocks::m_ghostEffect = ScratchConfiguration::getGraphicsEffect("ghost");
862+
863+
EXPECT_CALL(*colorEffect, clamp(_)).WillRepeatedly(ReturnArg<0>());
864+
EXPECT_CALL(*fisheyeEffect, clamp(_)).WillRepeatedly(ReturnArg<0>());
865+
EXPECT_CALL(*whirlEffect, clamp(_)).WillRepeatedly(ReturnArg<0>());
866+
EXPECT_CALL(*pixelateEffect, clamp(_)).WillRepeatedly(ReturnArg<0>());
867+
EXPECT_CALL(*mosaicEffect, clamp(_)).WillRepeatedly(ReturnArg<0>());
868+
EXPECT_CALL(*brightnessEffect, clamp(_)).WillRepeatedly(ReturnArg<0>());
869+
EXPECT_CALL(*ghostEffect, clamp(_)).WillRepeatedly(ReturnArg<0>());
860870
}
861871

862872
static void removeEffects()
@@ -888,6 +898,8 @@ TEST_F(LooksBlocksTest, ChangeEffectByImpl)
888898
static Value constValues[] = { 0, 55.15, 1, -40.54, 1.2, 2.3, -3.4, -4.5, 0.5, -8.54, 0.01 };
889899

890900
GraphicsEffectMock effect1, effect2;
901+
EXPECT_CALL(effect1, clamp(_)).WillRepeatedly(ReturnArg<0>());
902+
EXPECT_CALL(effect2, clamp(_)).WillRepeatedly(ReturnArg<0>());
891903

892904
Target target;
893905
target.setGraphicsEffectValue(&effect1, 12.5);
@@ -1164,6 +1176,8 @@ TEST_F(LooksBlocksTest, SetEffectToImpl)
11641176
static Value constValues[] = { 0, 55.15, 1, -40.54, 1.2, 2.3, -3.4, -4.5, 0.5, -8.54, 0.01 };
11651177

11661178
GraphicsEffectMock effect1, effect2;
1179+
EXPECT_CALL(effect1, clamp(_)).WillRepeatedly(ReturnArg<0>());
1180+
EXPECT_CALL(effect2, clamp(_)).WillRepeatedly(ReturnArg<0>());
11671181

11681182
Target target;
11691183
target.setGraphicsEffectValue(&effect1, 12.5);
@@ -1283,6 +1297,8 @@ TEST_F(LooksBlocksTest, ClearGraphicEffectsImpl)
12831297

12841298
Target target;
12851299
GraphicsEffectMock effect1, effect2;
1300+
EXPECT_CALL(effect1, clamp(_)).WillRepeatedly(ReturnArg<0>());
1301+
EXPECT_CALL(effect2, clamp(_)).WillRepeatedly(ReturnArg<0>());
12861302
target.setGraphicsEffectValue(&effect1, 48.21);
12871303
target.setGraphicsEffectValue(&effect2, -107.08);
12881304

‎test/mocks/graphicseffectmock.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,6 @@ class GraphicsEffectMock : public IGraphicsEffect
99
{
1010
public:
1111
MOCK_METHOD(std::string, name, (), (const, override));
12+
13+
MOCK_METHOD(double, clamp, (double), (const, override));
1214
};

‎test/scratch_classes/sprite_test.cpp‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -853,24 +853,22 @@ TEST(SpriteTest, GraphicsEffects)
853853

854854
EngineMock engine;
855855
sprite.setEngine(&engine);
856-
EXPECT_CALL(engine, requestRedraw()).Times(3);
856+
EXPECT_CALL(engine, requestRedraw()).Times(4);
857857

858858
GraphicsEffectMock effect1, effect2;
859+
EXPECT_CALL(effect1, clamp(48.21)).WillOnce(Return(48.21));
859860
sprite.setGraphicsEffectValue(&effect1, 48.21);
860861
ASSERT_EQ(sprite.graphicsEffectValue(&effect1), 48.21);
861862
ASSERT_EQ(sprite.graphicsEffectValue(&effect2), 0);
862863

864+
EXPECT_CALL(effect2, clamp(-107.08)).WillOnce(Return(-107.08));
863865
sprite.setGraphicsEffectValue(&effect2, -107.08);
864866
ASSERT_EQ(sprite.graphicsEffectValue(&effect1), 48.21);
865867
ASSERT_EQ(sprite.graphicsEffectValue(&effect2), -107.08);
866868

867-
sprite.clearGraphicsEffects();
868-
ASSERT_EQ(sprite.graphicsEffectValue(&effect1), 0);
869-
ASSERT_EQ(sprite.graphicsEffectValue(&effect2), 0);
870-
871-
sprite.setVisible(false);
872-
sprite.setGraphicsEffectValue(&effect2, -107.08);
873-
ASSERT_EQ(sprite.graphicsEffectValue(&effect1), 0);
869+
EXPECT_CALL(effect1, clamp(300)).WillOnce(Return(101.5));
870+
sprite.setGraphicsEffectValue(&effect1, 300);
871+
ASSERT_EQ(sprite.graphicsEffectValue(&effect1), 101.5);
874872
ASSERT_EQ(sprite.graphicsEffectValue(&effect2), -107.08);
875873

876874
sprite.clearGraphicsEffects();

‎test/scratch_classes/stage_test.cpp‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,17 +248,24 @@ TEST(StageTest, GraphicsEffects)
248248

249249
EngineMock engine;
250250
stage.setEngine(&engine);
251-
EXPECT_CALL(engine, requestRedraw()).Times(3);
251+
EXPECT_CALL(engine, requestRedraw()).Times(4);
252252

253253
GraphicsEffectMock effect1, effect2;
254+
EXPECT_CALL(effect1, clamp(48.21)).WillOnce(Return(48.21));
254255
stage.setGraphicsEffectValue(&effect1, 48.21);
255256
ASSERT_EQ(stage.graphicsEffectValue(&effect1), 48.21);
256257
ASSERT_EQ(stage.graphicsEffectValue(&effect2), 0);
257258

259+
EXPECT_CALL(effect2, clamp(-107.08)).WillOnce(Return(-107.08));
258260
stage.setGraphicsEffectValue(&effect2, -107.08);
259261
ASSERT_EQ(stage.graphicsEffectValue(&effect1), 48.21);
260262
ASSERT_EQ(stage.graphicsEffectValue(&effect2), -107.08);
261263

264+
EXPECT_CALL(effect1, clamp(300)).WillOnce(Return(101.5));
265+
stage.setGraphicsEffectValue(&effect1, 300);
266+
ASSERT_EQ(stage.graphicsEffectValue(&effect1), 101.5);
267+
ASSERT_EQ(stage.graphicsEffectValue(&effect2), -107.08);
268+
262269
stage.clearGraphicsEffects();
263270
ASSERT_EQ(stage.graphicsEffectValue(&effect1), 0);
264271
ASSERT_EQ(stage.graphicsEffectValue(&effect2), 0);

‎test/scratch_classes/target_test.cpp‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -519,13 +519,36 @@ TEST(TargetTest, SoundEffects)
519519
ASSERT_EQ(target.soundEffectValue(Sound::Effect::Pan), 0);
520520

521521
auto s1 = std::make_shared<SoundMock>();
522+
EXPECT_CALL(*s1, setVolume);
523+
target.addSound(s1);
524+
525+
EXPECT_CALL(*s1, setEffect(Sound::Effect::Pitch, -360));
526+
target.setSoundEffectValue(Sound::Effect::Pitch, -400);
527+
ASSERT_EQ(target.soundEffectValue(Sound::Effect::Pitch), -360);
528+
529+
EXPECT_CALL(*s1, setEffect(Sound::Effect::Pitch, 360));
530+
target.setSoundEffectValue(Sound::Effect::Pitch, 400);
531+
ASSERT_EQ(target.soundEffectValue(Sound::Effect::Pitch), 360);
532+
533+
EXPECT_CALL(*s1, setEffect(Sound::Effect::Pan, -100));
534+
target.setSoundEffectValue(Sound::Effect::Pan, -200);
535+
ASSERT_EQ(target.soundEffectValue(Sound::Effect::Pan), -100);
536+
537+
EXPECT_CALL(*s1, setEffect(Sound::Effect::Pan, 100));
538+
target.setSoundEffectValue(Sound::Effect::Pan, 200);
539+
ASSERT_EQ(target.soundEffectValue(Sound::Effect::Pan), 100);
540+
541+
EXPECT_CALL(*s1, setEffect(Sound::Effect::Pitch, 0));
542+
EXPECT_CALL(*s1, setEffect(Sound::Effect::Pan, 0));
543+
target.clearSoundEffects();
544+
ASSERT_EQ(target.soundEffectValue(Sound::Effect::Pitch), 0);
545+
ASSERT_EQ(target.soundEffectValue(Sound::Effect::Pan), 0);
546+
522547
auto s2 = std::make_shared<SoundMock>();
523548
auto s3 = std::make_shared<SoundMock>();
524549

525-
EXPECT_CALL(*s1, setVolume);
526550
EXPECT_CALL(*s2, setVolume);
527551
EXPECT_CALL(*s3, setVolume);
528-
target.addSound(s1);
529552
target.addSound(s2);
530553
target.addSound(s3);
531554

@@ -727,14 +750,21 @@ TEST(TargetTest, GraphicsEffects)
727750
target.setEngine(&engine);
728751

729752
GraphicsEffectMock effect1, effect2;
753+
EXPECT_CALL(effect1, clamp(48.21)).WillOnce(Return(48.21));
730754
target.setGraphicsEffectValue(&effect1, 48.21);
731755
ASSERT_EQ(target.graphicsEffectValue(&effect1), 48.21);
732756
ASSERT_EQ(target.graphicsEffectValue(&effect2), 0);
733757

758+
EXPECT_CALL(effect2, clamp(-107.08)).WillOnce(Return(-107.08));
734759
target.setGraphicsEffectValue(&effect2, -107.08);
735760
ASSERT_EQ(target.graphicsEffectValue(&effect1), 48.21);
736761
ASSERT_EQ(target.graphicsEffectValue(&effect2), -107.08);
737762

763+
EXPECT_CALL(effect1, clamp(300)).WillOnce(Return(101.5));
764+
target.setGraphicsEffectValue(&effect1, 300);
765+
ASSERT_EQ(target.graphicsEffectValue(&effect1), 101.5);
766+
ASSERT_EQ(target.graphicsEffectValue(&effect2), -107.08);
767+
738768
target.clearGraphicsEffects();
739769
ASSERT_EQ(target.graphicsEffectValue(&effect1), 0);
740770
ASSERT_EQ(target.graphicsEffectValue(&effect2), 0);

0 commit comments

Comments
 (0)