Skip to content

Commit 4b9584d

Browse files
adazem009adazem009
committed
Use shared_ptr in IEngine::initClone()
1 parent e3cb5f9 commit 4b9584d

File tree

8 files changed

+56
-52
lines changed

8 files changed

+56
-52
lines changed

‎include/scratchcpp/iengine.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ class LIBSCRATCHCPP_EXPORT IEngine
7373
virtual void stopTarget(Target *target, VirtualMachine *exceptScript) = 0;
7474

7575
/*! Calls the "when I start as a clone" blocks of the given sprite. */
76-
virtual void initClone(Sprite *clone) = 0;
76+
virtual void initClone(std::shared_ptr<Sprite> clone) = 0;
7777

7878
/*! Automatically called from clones that are being deleted. */
7979
virtual void deinitClone(Sprite *clone) = 0;

‎src/engine/internal/engine.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ void Engine::stopTarget(Target *target, VirtualMachine *exceptScript)
279279
stopScript(script);
280280
}
281281

282-
void Engine::initClone(Sprite *clone)
282+
void Engine::initClone(std::shared_ptr<Sprite> clone)
283283
{
284284
if (!clone || ((m_cloneLimit >= 0) && (m_clones.size() >= m_cloneLimit)))
285285
return;
@@ -299,20 +299,20 @@ void Engine::initClone(Sprite *clone)
299299
// Since we're initializing the clone, it shouldn't have any running scripts
300300
for (const auto &[target, targetScripts] : m_runningScripts) {
301301
for (const auto script : targetScripts)
302-
assert((target != clone) || (std::find(m_scriptsToRemove.begin(), m_scriptsToRemove.end(), script.get()) != m_scriptsToRemove.end()));
302+
assert((target != clone.get()) || (std::find(m_scriptsToRemove.begin(), m_scriptsToRemove.end(), script.get()) != m_scriptsToRemove.end()));
303303
}
304304
#endif
305305

306306
for (auto script : scripts) {
307-
auto vm = script->start(clone);
307+
auto vm = script->start(clone.get());
308308
addRunningScript(vm);
309309
}
310310
}
311311

312-
assert(std::find(m_clones.begin(), m_clones.end(), clone) == m_clones.end());
313-
assert(std::find(m_executableTargets.begin(), m_executableTargets.end(), clone) == m_executableTargets.end());
314-
m_clones.push_back(clone);
315-
m_executableTargets.push_back(clone); // execution order needs to be updated after this
312+
assert(std::find(m_clones.begin(), m_clones.end(), clone.get()) == m_clones.end());
313+
assert(std::find(m_executableTargets.begin(), m_executableTargets.end(), clone.get()) == m_executableTargets.end());
314+
m_clones.push_back(clone.get());
315+
m_executableTargets.push_back(clone.get()); // execution order needs to be updated after this
316316
}
317317

318318
void Engine::deinitClone(Sprite *clone)

‎src/engine/internal/engine.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class Engine : public IEngine
3636
void broadcastByPtr(Broadcast *broadcast, VirtualMachine *sourceScript, bool wait = false) override;
3737
void stopScript(VirtualMachine *vm) override;
3838
void stopTarget(Target *target, VirtualMachine *exceptScript) override;
39-
void initClone(libscratchcpp::Sprite *clone) override;
39+
void initClone(std::shared_ptr<Sprite> clone) override;
4040
void deinitClone(libscratchcpp::Sprite *clone) override;
4141

4242
void run() override;

‎src/scratch/sprite.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ std::shared_ptr<Sprite> Sprite::clone()
7373
clone->setEngine(engine());
7474

7575
// Call "when I start as clone" scripts
76-
eng->initClone(clone.get());
76+
eng->initClone(clone);
7777

7878
if (impl->visible)
7979
eng->requestRedraw();

‎test/blocks/control_blocks_test.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -902,7 +902,7 @@ TEST_F(ControlBlocksTest, CreateCloneOfImpl)
902902
vm.setFunctions(functions);
903903
vm.setConstValues(constValues);
904904

905-
Sprite *clone1;
905+
std::shared_ptr<Sprite> clone1;
906906
EXPECT_CALL(m_engineMock, targetAt(4)).WillOnce(Return(&sprite));
907907
EXPECT_CALL(m_engineMock, cloneLimit()).Times(8).WillRepeatedly(Return(300));
908908
EXPECT_CALL(m_engineMock, cloneCount()).Times(4).WillRepeatedly(Return(0));
@@ -926,7 +926,7 @@ TEST_F(ControlBlocksTest, CreateCloneOfImpl)
926926
ASSERT_EQ(vm.registerCount(), 0);
927927
ASSERT_EQ(sprite.clones().size(), 2);
928928

929-
Sprite *clone3;
929+
std::shared_ptr<Sprite> clone3;
930930
EXPECT_CALL(m_engineMock, findTarget).WillOnce(Return(4));
931931
EXPECT_CALL(m_engineMock, targetAt(4)).WillOnce(Return(&sprite));
932932
EXPECT_CALL(m_engineMock, initClone).WillOnce(SaveArg<0>(&clone3));
@@ -949,10 +949,10 @@ TEST_F(ControlBlocksTest, CreateCloneOfImpl)
949949
ASSERT_EQ(vm.registerCount(), 0);
950950
ASSERT_EQ(sprite.clones().size(), 4);
951951

952-
EXPECT_CALL(m_engineMock, deinitClone(clone1));
952+
EXPECT_CALL(m_engineMock, deinitClone(clone1.get()));
953953
clone1->deleteClone();
954954

955-
EXPECT_CALL(m_engineMock, deinitClone(clone3));
955+
EXPECT_CALL(m_engineMock, deinitClone(clone3.get()));
956956
clone3->deleteClone();
957957
}
958958

@@ -994,7 +994,7 @@ TEST_F(ControlBlocksTest, DeleteThisCloneImpl)
994994
Sprite sprite;
995995
sprite.setEngine(&m_engineMock);
996996

997-
Sprite *clone;
997+
std::shared_ptr<Sprite> clone;
998998
EXPECT_CALL(m_engineMock, cloneLimit()).Times(2).WillRepeatedly(Return(300));
999999
EXPECT_CALL(m_engineMock, cloneCount()).WillOnce(Return(0));
10001000
EXPECT_CALL(m_engineMock, initClone(_)).WillOnce(SaveArg<0>(&clone));
@@ -1003,11 +1003,11 @@ TEST_F(ControlBlocksTest, DeleteThisCloneImpl)
10031003
sprite.clone();
10041004
ASSERT_TRUE(clone);
10051005

1006-
VirtualMachine vm(clone, &m_engineMock, nullptr);
1006+
VirtualMachine vm(clone.get(), &m_engineMock, nullptr);
10071007
vm.setFunctions(functions);
10081008

1009-
EXPECT_CALL(m_engineMock, stopTarget(clone, nullptr)).Times(1);
1010-
EXPECT_CALL(m_engineMock, deinitClone(clone));
1009+
EXPECT_CALL(m_engineMock, stopTarget(clone.get(), nullptr)).Times(1);
1010+
EXPECT_CALL(m_engineMock, deinitClone(clone.get()));
10111011

10121012
vm.setBytecode(bytecode);
10131013
vm.run();

‎test/mocks/enginemock.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class EngineMock : public IEngine
2121
MOCK_METHOD(void, broadcastByPtr, (Broadcast *, VirtualMachine *, bool), (override));
2222
MOCK_METHOD(void, stopScript, (VirtualMachine *), (override));
2323
MOCK_METHOD(void, stopTarget, (Target *, VirtualMachine *), (override));
24-
MOCK_METHOD(void, initClone, (Sprite *), (override));
24+
MOCK_METHOD(void, initClone, (std::shared_ptr<Sprite>), (override));
2525
MOCK_METHOD(void, deinitClone, (Sprite *), (override));
2626

2727
MOCK_METHOD(void, run, (), (override));

‎test/scratch_classes/sprite_test.cpp‎

Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -122,51 +122,55 @@ TEST(SpriteTest, Clone)
122122
EXPECT_CALL(engine, requestRedraw()).Times(2);
123123
EXPECT_CALL(engine, cloneLimit()).Times(6).WillRepeatedly(Return(300)); // clone count limit is tested later
124124

125-
Sprite *clone1, *clone1_2;
125+
std::shared_ptr<Sprite> clone1;
126+
Sprite *clone1_2;
126127
EXPECT_CALL(engine, cloneCount()).WillOnce(Return(0));
127128
EXPECT_CALL(engine, initClone(_)).WillOnce(SaveArg<0>(&clone1));
128129
EXPECT_CALL(engine, moveSpriteBehindOther(_, &sprite)).WillOnce(SaveArg<0>(&clone1_2));
129-
ASSERT_EQ(sprite.clone().get(), clone1);
130-
ASSERT_EQ(clone1, clone1_2);
130+
ASSERT_EQ(sprite.clone(), clone1);
131+
ASSERT_EQ(clone1.get(), clone1_2);
131132
ASSERT_FALSE(sprite.isClone());
132133
ASSERT_EQ(sprite.cloneSprite(), nullptr);
133134

134135
ASSERT_TRUE(clone1->isClone());
135136
ASSERT_EQ(clone1->cloneSprite(), &sprite);
136137

137-
checkCloneData(clone1);
138+
checkCloneData(clone1.get());
138139

139140
// Modify root sprite data to make sure parent is used
140141
sprite.setLayerOrder(3);
141142

142-
Sprite *clone2, *clone2_2;
143+
std::shared_ptr<Sprite> clone2;
144+
Sprite *clone2_2;
143145
EXPECT_CALL(engine, cloneCount()).WillOnce(Return(1));
144146
EXPECT_CALL(engine, initClone(_)).WillOnce(SaveArg<0>(&clone2));
145-
EXPECT_CALL(engine, moveSpriteBehindOther(_, clone1)).WillOnce(SaveArg<0>(&clone2_2));
146-
ASSERT_EQ(clone1->clone().get(), clone2);
147-
ASSERT_EQ(clone2, clone2_2);
147+
EXPECT_CALL(engine, moveSpriteBehindOther(_, clone1.get())).WillOnce(SaveArg<0>(&clone2_2));
148+
ASSERT_EQ(clone1->clone(), clone2);
149+
ASSERT_EQ(clone2.get(), clone2_2);
148150
ASSERT_TRUE(clone1->isClone());
149151
ASSERT_EQ(clone1->cloneSprite(), &sprite);
150152
ASSERT_TRUE(clone2->isClone());
151153
ASSERT_EQ(clone2->cloneSprite(), &sprite);
152154

153-
checkCloneData(clone2);
155+
checkCloneData(clone2.get());
154156

155157
sprite.setVisible(true);
156158

157-
Sprite *clone3, *clone3_2;
159+
std::shared_ptr<Sprite> clone3;
160+
Sprite *clone3_2;
158161
EXPECT_CALL(engine, cloneCount()).WillOnce(Return(2));
159162
EXPECT_CALL(engine, initClone(_)).WillOnce(SaveArg<0>(&clone3));
160-
EXPECT_CALL(engine, moveSpriteBehindOther(_, clone1)).WillOnce(SaveArg<0>(&clone3_2));
161-
ASSERT_EQ(clone1->clone().get(), clone3);
162-
ASSERT_EQ(clone3, clone3_2);
163+
EXPECT_CALL(engine, moveSpriteBehindOther(_, clone1.get())).WillOnce(SaveArg<0>(&clone3_2));
164+
ASSERT_EQ(clone1->clone(), clone3);
165+
ASSERT_EQ(clone3.get(), clone3_2);
163166

164-
Sprite *clone4, *clone4_2;
167+
std::shared_ptr<Sprite> clone4;
168+
Sprite *clone4_2;
165169
EXPECT_CALL(engine, cloneLimit()).WillOnce(Return(-1));
166170
EXPECT_CALL(engine, initClone(_)).WillOnce(SaveArg<0>(&clone4));
167171
EXPECT_CALL(engine, moveSpriteBehindOther(_, &sprite)).WillOnce(SaveArg<0>(&clone4_2));
168-
ASSERT_EQ(sprite.clone().get(), clone4);
169-
ASSERT_EQ(clone4, clone4_2);
172+
ASSERT_EQ(sprite.clone(), clone4);
173+
ASSERT_EQ(clone4.get(), clone4_2);
170174

171175
EXPECT_CALL(engine, cloneLimit()).Times(2).WillRepeatedly(Return(0));
172176
EXPECT_CALL(engine, cloneCount()).WillOnce(Return(0));
@@ -179,10 +183,10 @@ TEST(SpriteTest, Clone)
179183
// clones
180184
const auto &clones = sprite.clones();
181185
ASSERT_EQ(clones.size(), 4);
182-
ASSERT_EQ(clones[0].get(), clone1);
183-
ASSERT_EQ(clones[1].get(), clone2);
184-
ASSERT_EQ(clones[2].get(), clone3);
185-
ASSERT_EQ(clones[3].get(), clone4);
186+
ASSERT_EQ(clones[0], clone1);
187+
ASSERT_EQ(clones[1], clone2);
188+
ASSERT_EQ(clones[2], clone3);
189+
ASSERT_EQ(clones[3], clone4);
186190

187191
ASSERT_EQ(clone1->clones(), clones);
188192
ASSERT_EQ(clone2->clones(), clones);
@@ -195,28 +199,28 @@ TEST(SpriteTest, Clone)
195199
ASSERT_EQ(clone2->costumes(), sprite.costumes());
196200

197201
// Delete
198-
EXPECT_CALL(engine, deinitClone(clone1));
202+
EXPECT_CALL(engine, deinitClone(clone1.get()));
199203
clone1->deleteClone();
200204

201205
ASSERT_EQ(clones.size(), 3);
202-
ASSERT_EQ(clones[0].get(), clone2);
203-
ASSERT_EQ(clones[1].get(), clone3);
204-
ASSERT_EQ(clones[2].get(), clone4);
206+
ASSERT_EQ(clones[0], clone2);
207+
ASSERT_EQ(clones[1], clone3);
208+
ASSERT_EQ(clones[2], clone4);
205209

206-
EXPECT_CALL(engine, deinitClone(clone3));
210+
EXPECT_CALL(engine, deinitClone(clone3.get()));
207211
clone3->deleteClone();
208212

209213
ASSERT_EQ(clones.size(), 2);
210-
ASSERT_EQ(clones[0].get(), clone2);
211-
ASSERT_EQ(clones[1].get(), clone4);
214+
ASSERT_EQ(clones[0], clone2);
215+
ASSERT_EQ(clones[1], clone4);
212216

213-
EXPECT_CALL(engine, deinitClone(clone2));
217+
EXPECT_CALL(engine, deinitClone(clone2.get()));
214218
clone2->deleteClone();
215219

216220
ASSERT_EQ(clones.size(), 1);
217-
ASSERT_EQ(clones[0].get(), clone4);
221+
ASSERT_EQ(clones[0], clone4);
218222

219-
EXPECT_CALL(engine, deinitClone(clone4));
223+
EXPECT_CALL(engine, deinitClone(clone4.get()));
220224
clone4->deleteClone();
221225
ASSERT_TRUE(clones.empty());
222226
}

‎test/target_interfaces/ispritehandler_test.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class ISpriteHandlerTest : public testing::Test
2929

3030
TEST_F(ISpriteHandlerTest, Clone)
3131
{
32-
Sprite *clone;
32+
std::shared_ptr<Sprite> clone;
3333
Sprite *cloneParam1, *cloneParam2;
3434
EXPECT_CALL(m_engine, cloneLimit()).Times(2).WillRepeatedly(Return(300));
3535
EXPECT_CALL(m_engine, cloneCount()).WillOnce(Return(0));
@@ -40,8 +40,8 @@ TEST_F(ISpriteHandlerTest, Clone)
4040

4141
m_sprite.clone();
4242
ASSERT_TRUE(clone);
43-
ASSERT_EQ(cloneParam1, clone);
44-
ASSERT_EQ(cloneParam2, clone);
43+
ASSERT_EQ(cloneParam1, clone.get());
44+
ASSERT_EQ(cloneParam2, clone.get());
4545

4646
// Engine is used during deletion, so let's just set it to nullptr (this is tested elsewhere)
4747
m_sprite.setEngine(nullptr);

0 commit comments

Comments
 (0)