Skip to content

Commit 8c50bad

Browse files
adazem009adazem009
committed
Refactor clone hierarchy implementation
1 parent d3eef68 commit 8c50bad

File tree

8 files changed

+56
-125
lines changed

8 files changed

+56
-125
lines changed

‎include/scratchcpp/sprite.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,8 @@ class LIBSCRATCHCPP_EXPORT Sprite : public Target
3333

3434
bool isClone() const;
3535

36-
Sprite *cloneRoot() const;
37-
Sprite *cloneParent() const;
38-
const std::vector<std::shared_ptr<Sprite>> &children() const;
39-
std::vector<std::shared_ptr<Sprite>> allChildren() const;
36+
Sprite *cloneSprite() const;
37+
const std::vector<std::shared_ptr<Sprite>> &clones() const;
4038

4139
bool visible() const;
4240
void setVisible(bool newVisible);

‎src/engine/internal/engine.cpp‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,8 @@ void Engine::broadcastByPtr(Broadcast *broadcast, VirtualMachine *sourceScript,
239239
if (!root->isStage()) {
240240
Sprite *sprite = dynamic_cast<Sprite *>(root);
241241
assert(sprite);
242-
auto children = sprite->allChildren();
242+
assert(!sprite->isClone());
243+
const auto &children = sprite->clones();
243244

244245
for (auto child : children)
245246
targets.push_back(child.get());
@@ -283,12 +284,10 @@ void Engine::initClone(Sprite *clone)
283284
if (!clone || ((m_cloneLimit >= 0) && (m_clones.size() >= m_cloneLimit)))
284285
return;
285286

286-
Sprite *source = clone->cloneParent();
287-
Target *root = clone->cloneRoot();
288-
assert(source);
287+
Target *root = clone->cloneSprite();
289288
assert(root);
290289

291-
if (!source || !root)
290+
if (!root)
292291
return;
293292

294293
auto it = m_cloneInitScriptsMap.find(root);
@@ -1204,7 +1203,7 @@ void Engine::deleteClones()
12041203
Sprite *sprite = dynamic_cast<Sprite *>(target.get());
12051204

12061205
if (sprite) {
1207-
std::vector<std::shared_ptr<Sprite>> clones = sprite->children();
1206+
std::vector<std::shared_ptr<Sprite>> clones = sprite->clones();
12081207

12091208
for (auto clone : clones) {
12101209
assert(clone);

‎src/engine/script.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ std::shared_ptr<VirtualMachine> Script::start(Target *target)
6363
sprite = dynamic_cast<Sprite *>(target);
6464

6565
if (impl->target && sprite && sprite->isClone() && impl->engine) {
66-
Target *root = sprite->cloneRoot();
66+
Target *root = sprite->cloneSprite();
6767

6868
if (root != impl->target) {
6969
std::cout << "warning: a clone tried to start a script of another target (this is a bug in libscratchcpp or in your code!)" << std::endl;

‎src/scratch/sprite.cpp‎

Lines changed: 21 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,11 @@ Sprite::~Sprite()
2626
if (isClone()) {
2727
IEngine *eng = engine();
2828

29-
if (eng) {
29+
if (eng)
3030
eng->deinitClone(this);
3131

32-
auto children = allChildren();
33-
for (auto child : children)
34-
eng->deinitClone(child.get());
35-
}
36-
37-
assert(impl->cloneParent);
38-
impl->cloneParent->impl->removeClone(this);
32+
assert(impl->cloneSprite);
33+
impl->cloneSprite->impl->removeClone(this);
3934
}
4035
}
4136

@@ -55,13 +50,13 @@ std::shared_ptr<Sprite> Sprite::clone()
5550
if (eng && (eng->cloneLimit() == -1 || eng->cloneCount() < eng->cloneLimit())) {
5651
std::shared_ptr<Sprite> clone = std::make_shared<Sprite>();
5752

58-
if (impl->cloneRoot == nullptr)
59-
clone->impl->cloneRoot = this;
60-
else
61-
clone->impl->cloneRoot = impl->cloneRoot;
62-
63-
clone->impl->cloneParent = this;
64-
impl->childClones.push_back(clone);
53+
if (impl->cloneSprite == nullptr) {
54+
clone->impl->cloneSprite = this;
55+
impl->clones.push_back(clone);
56+
} else {
57+
clone->impl->cloneSprite = impl->cloneSprite;
58+
impl->cloneSprite->impl->clones.push_back(clone);
59+
}
6560

6661
// Copy data
6762
clone->setName(name());
@@ -111,39 +106,23 @@ std::shared_ptr<Sprite> Sprite::clone()
111106
/*! Returns true if this is a clone. */
112107
bool Sprite::isClone() const
113108
{
114-
return (impl->cloneParent != nullptr);
109+
return (impl->cloneSprite != nullptr);
115110
}
116111

117112
/*! Returns the sprite this clone was created from, or nullptr if this isn't a clone. */
118-
Sprite *Sprite::cloneRoot() const
119-
{
120-
return impl->cloneRoot;
121-
}
122-
123-
/*! Returns the sprite or clone this clone was created from, or nullptr if this isn't a clone. */
124-
Sprite *Sprite::cloneParent() const
113+
Sprite *Sprite::cloneSprite() const
125114
{
126-
return impl->cloneParent;
115+
return impl->cloneSprite;
127116
}
128117

129-
/*! Returns list of child clones. */
130-
const std::vector<std::shared_ptr<Sprite>> &Sprite::children() const
118+
/*! Returns list of clones of the sprite. */
119+
const std::vector<std::shared_ptr<Sprite>> &Sprite::clones() const
131120
{
132-
return impl->childClones;
133-
}
134-
135-
/*! Returns list of child clones and their children (recursive). */
136-
std::vector<std::shared_ptr<Sprite>> Sprite::allChildren() const
137-
{
138-
std::vector<std::shared_ptr<Sprite>> ret;
139-
140-
for (auto clone : impl->childClones) {
141-
ret.push_back(clone);
142-
auto children = clone->allChildren();
143-
ret.insert(ret.end(), children.begin(), children.end());
144-
}
145-
146-
return ret;
121+
if (isClone()) {
122+
assert(impl->cloneSprite);
123+
return impl->cloneSprite->impl->clones;
124+
} else
125+
return impl->clones;
147126
}
148127

149128
/*! Returns true if the sprite is visible. */
@@ -419,7 +398,7 @@ void Sprite::clearGraphicsEffects()
419398

420399
Target *Sprite::dataSource() const
421400
{
422-
return impl->cloneRoot;
401+
return impl->cloneSprite;
423402
}
424403

425404
void Sprite::setXY(double x, double y)

‎src/scratch/sprite_p.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ SpritePrivate::SpritePrivate(Sprite *sprite) :
2121
void SpritePrivate::removeClone(Sprite *clone)
2222
{
2323
int index = 0;
24-
for (const auto &child : childClones) {
24+
for (const auto &child : clones) {
2525
if (child.get() == clone) {
26-
childClones.erase(childClones.begin() + index);
26+
clones.erase(clones.begin() + index);
2727
return;
2828
}
2929

‎src/scratch/sprite_p.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,8 @@ struct SpritePrivate
2121

2222
Sprite *sprite = nullptr;
2323
ISpriteHandler *iface = nullptr;
24-
Sprite *cloneRoot = nullptr;
25-
Sprite *cloneParent = nullptr;
26-
std::vector<std::shared_ptr<Sprite>> childClones;
24+
Sprite *cloneSprite = nullptr;
25+
std::vector<std::shared_ptr<Sprite>> clones;
2726
bool visible = true;
2827
double x = 0;
2928
double y = 0;

‎test/blocks/control_blocks_test.cpp‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -914,7 +914,7 @@ TEST_F(ControlBlocksTest, CreateCloneOfImpl)
914914
vm.run();
915915

916916
ASSERT_EQ(vm.registerCount(), 0);
917-
ASSERT_EQ(sprite.allChildren().size(), 1);
917+
ASSERT_EQ(sprite.clones().size(), 1);
918918

919919
EXPECT_CALL(m_engineMock, initClone).Times(1);
920920
EXPECT_CALL(m_engineMock, moveSpriteBehindOther(_, &sprite));
@@ -924,8 +924,7 @@ TEST_F(ControlBlocksTest, CreateCloneOfImpl)
924924
vm.run();
925925

926926
ASSERT_EQ(vm.registerCount(), 0);
927-
ASSERT_EQ(sprite.allChildren().size(), 2);
928-
ASSERT_EQ(sprite.children(), sprite.allChildren());
927+
ASSERT_EQ(sprite.clones().size(), 2);
929928

930929
Sprite *clone3;
931930
EXPECT_CALL(m_engineMock, findTarget).WillOnce(Return(4));
@@ -938,8 +937,7 @@ TEST_F(ControlBlocksTest, CreateCloneOfImpl)
938937
vm.run();
939938

940939
ASSERT_EQ(vm.registerCount(), 0);
941-
ASSERT_EQ(sprite.allChildren().size(), 3);
942-
ASSERT_EQ(sprite.children(), sprite.allChildren());
940+
ASSERT_EQ(sprite.clones().size(), 3);
943941

944942
EXPECT_CALL(m_engineMock, initClone).Times(1);
945943
EXPECT_CALL(m_engineMock, moveSpriteBehindOther(_, &sprite));
@@ -949,8 +947,7 @@ TEST_F(ControlBlocksTest, CreateCloneOfImpl)
949947
vm.run();
950948

951949
ASSERT_EQ(vm.registerCount(), 0);
952-
ASSERT_EQ(sprite.allChildren().size(), 4);
953-
ASSERT_EQ(sprite.children(), sprite.allChildren());
950+
ASSERT_EQ(sprite.clones().size(), 4);
954951

955952
EXPECT_CALL(m_engineMock, deinitClone(clone1));
956953
EXPECT_CALL(m_engineMock, deinitClone(clone3));
@@ -1013,5 +1010,5 @@ TEST_F(ControlBlocksTest, DeleteThisCloneImpl)
10131010
vm.run();
10141011

10151012
ASSERT_EQ(vm.registerCount(), 0);
1016-
ASSERT_TRUE(sprite.children().empty());
1013+
ASSERT_TRUE(sprite.clones().empty());
10171014
}

‎test/scratch_classes/sprite_test.cpp‎

Lines changed: 18 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ TEST(SpriteTest, Clone)
7373

7474
auto checkCloneData = [](Sprite *clone) {
7575
ASSERT_TRUE(clone);
76-
Sprite *root = clone->cloneRoot();
76+
Sprite *root = clone->cloneSprite();
7777

7878
ASSERT_EQ(clone->name(), "Sprite1");
7979
ASSERT_EQ(clone->variables().size(), 2);
@@ -111,13 +111,11 @@ TEST(SpriteTest, Clone)
111111
};
112112

113113
ASSERT_FALSE(sprite->isClone());
114-
ASSERT_EQ(sprite->cloneRoot(), nullptr);
115-
ASSERT_EQ(sprite->cloneParent(), nullptr);
114+
ASSERT_EQ(sprite->cloneSprite(), nullptr);
116115

117116
ASSERT_EQ(sprite->clone(), nullptr);
118117
ASSERT_FALSE(sprite->isClone());
119-
ASSERT_EQ(sprite->cloneRoot(), nullptr);
120-
ASSERT_EQ(sprite->cloneParent(), nullptr);
118+
ASSERT_EQ(sprite->cloneSprite(), nullptr);
121119

122120
EngineMock engine;
123121
sprite->setEngine(&engine);
@@ -131,12 +129,10 @@ TEST(SpriteTest, Clone)
131129
ASSERT_EQ(sprite->clone().get(), clone1);
132130
ASSERT_EQ(clone1, clone1_2);
133131
ASSERT_FALSE(sprite->isClone());
134-
ASSERT_EQ(sprite->cloneRoot(), nullptr);
135-
ASSERT_EQ(sprite->cloneParent(), nullptr);
132+
ASSERT_EQ(sprite->cloneSprite(), nullptr);
136133

137134
ASSERT_TRUE(clone1->isClone());
138-
ASSERT_EQ(clone1->cloneRoot(), sprite.get());
139-
ASSERT_EQ(clone1->cloneParent(), sprite.get());
135+
ASSERT_EQ(clone1->cloneSprite(), sprite.get());
140136

141137
checkCloneData(clone1);
142138

@@ -150,11 +146,9 @@ TEST(SpriteTest, Clone)
150146
ASSERT_EQ(clone1->clone().get(), clone2);
151147
ASSERT_EQ(clone2, clone2_2);
152148
ASSERT_TRUE(clone1->isClone());
153-
ASSERT_EQ(clone1->cloneRoot(), sprite.get());
154-
ASSERT_EQ(clone1->cloneParent(), sprite.get());
149+
ASSERT_EQ(clone1->cloneSprite(), sprite.get());
155150
ASSERT_TRUE(clone2->isClone());
156-
ASSERT_EQ(clone2->cloneRoot(), sprite.get());
157-
ASSERT_EQ(clone2->cloneParent(), clone1);
151+
ASSERT_EQ(clone2->cloneSprite(), sprite.get());
158152

159153
checkCloneData(clone2);
160154

@@ -182,59 +176,24 @@ TEST(SpriteTest, Clone)
182176
EXPECT_CALL(engine, cloneCount()).WillOnce(Return(150));
183177
ASSERT_EQ(sprite->clone(), nullptr);
184178

185-
// children
186-
const auto &children1 = sprite->children();
187-
ASSERT_EQ(children1.size(), 2);
188-
ASSERT_EQ(children1[0].get(), clone1);
189-
ASSERT_EQ(children1[1].get(), clone4);
179+
// clones
180+
const auto &clones = sprite->clones();
181+
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);
190186

191-
const auto &children2 = clone1->children();
192-
ASSERT_EQ(children2.size(), 2);
193-
ASSERT_EQ(children2[0].get(), clone2);
194-
ASSERT_EQ(children2[1].get(), clone3);
195-
196-
ASSERT_TRUE(clone2->children().empty());
197-
198-
ASSERT_TRUE(clone3->children().empty());
199-
200-
ASSERT_TRUE(clone4->children().empty());
201-
202-
// allChildren
203-
auto allChildren = sprite->allChildren();
204-
ASSERT_EQ(allChildren.size(), 4);
205-
ASSERT_EQ(allChildren[0].get(), clone1);
206-
ASSERT_EQ(allChildren[1].get(), clone2);
207-
ASSERT_EQ(allChildren[2].get(), clone3);
208-
ASSERT_EQ(allChildren[3].get(), clone4);
209-
210-
allChildren = clone1->allChildren();
211-
ASSERT_EQ(allChildren.size(), 2);
212-
ASSERT_EQ(allChildren[0].get(), clone2);
213-
ASSERT_EQ(allChildren[1].get(), clone3);
214-
215-
ASSERT_TRUE(clone2->allChildren().empty());
216-
217-
ASSERT_TRUE(clone3->allChildren().empty());
218-
219-
ASSERT_TRUE(clone4->allChildren().empty());
187+
ASSERT_EQ(clone1->clones(), clones);
188+
ASSERT_EQ(clone2->clones(), clones);
189+
ASSERT_EQ(clone3->clones(), clones);
190+
ASSERT_EQ(clone4->clones(), clones);
220191

221192
ASSERT_EQ(clone2->costumes(), sprite->costumes());
222193
auto c2 = std::make_shared<Costume>("costume2", "", "png");
223194
clone2->addCostume(c2);
224195
ASSERT_EQ(clone2->costumes(), sprite->costumes());
225196

226-
EXPECT_CALL(engine, deinitClone(clone2));
227-
clone2->~Sprite();
228-
229-
EXPECT_CALL(engine, deinitClone(clone3));
230-
clone3->~Sprite();
231-
232-
EXPECT_CALL(engine, deinitClone(clone1)).Times(2);
233-
clone1->~Sprite();
234-
235-
EXPECT_CALL(engine, deinitClone(clone4)).Times(2);
236-
clone4->~Sprite();
237-
238197
sprite.reset();
239198
}
240199

0 commit comments

Comments
 (0)