Skip to content

Commit 17ba443

Browse files
adazem009adazem009
authored
Merge pull request #568 from scratchcpp/optimize_target_processing
Optimize target processing
2 parents edb788e + 366e11d commit 17ba443

File tree

4 files changed

+117
-33
lines changed

4 files changed

+117
-33
lines changed

‎src/engine/internal/engine.cpp‎

Lines changed: 95 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,8 @@ void Engine::resolveIds()
107107
const auto &blocks = target->blocks();
108108
for (auto block : blocks) {
109109
auto container = blockSectionContainer(block->opcode());
110-
block->setNext(getBlock(block->nextId()));
111-
block->setParent(getBlock(block->parentId()));
110+
block->setNext(getBlock(block->nextId(), target.get()));
111+
block->setParent(getBlock(block->parentId(), target.get()));
112112

113113
if (container) {
114114
block->setCompileFunction(container->resolveBlockCompileFunc(block->opcode()));
@@ -117,16 +117,16 @@ void Engine::resolveIds()
117117

118118
const auto &inputs = block->inputs();
119119
for (const auto &input : inputs) {
120-
input->setValueBlock(getBlock(input->valueBlockId()));
120+
input->setValueBlock(getBlock(input->valueBlockId(), target.get()));
121121

122122
if (container)
123123
input->setInputId(container->resolveInput(input->name()));
124124

125125
InputValue *value = input->primaryValue();
126126
std::string id = value->valueId(); // no reference!
127-
value->setValuePtr(getEntity(id));
127+
value->setValuePtr(getEntity(id, target.get()));
128128
assert(input->secondaryValue()->type() != InputValue::Type::Variable && input->secondaryValue()->type() != InputValue::Type::List); // secondary values never have a variable or list
129-
input->secondaryValue()->setValuePtr(getEntity(input->secondaryValue()->valueId()));
129+
input->secondaryValue()->setValuePtr(getEntity(input->secondaryValue()->valueId(), target.get()));
130130

131131
// Add missing variables and lists
132132
if (!value->valuePtr()) {
@@ -149,7 +149,7 @@ void Engine::resolveIds()
149149
const auto &fields = block->fields();
150150
for (auto field : fields) {
151151
std::string id = field->valueId(); // no reference!
152-
field->setValuePtr(getEntity(id));
152+
field->setValuePtr(getEntity(id, target.get()));
153153

154154
if (container) {
155155
field->setFieldId(container->resolveField(field->name()));
@@ -179,7 +179,7 @@ void Engine::resolveIds()
179179
block->updateInputMap();
180180
block->updateFieldMap();
181181

182-
auto comment = getComment(block->commentId());
182+
auto comment = getComment(block->commentId(), target.get());
183183
block->setComment(comment);
184184

185185
if (comment) {
@@ -209,7 +209,7 @@ void Engine::resolveIds()
209209
assert(target);
210210

211211
for (auto field : fields) {
212-
field->setValuePtr(getEntity(field->valueId()));
212+
field->setValuePtr(getEntity(field->valueId(), target));
213213

214214
if (container) {
215215
field->setFieldId(container->resolveField(field->name()));
@@ -1411,47 +1411,99 @@ const std::unordered_map<std::shared_ptr<Block>, std::shared_ptr<Script>> &Engin
14111411
}
14121412

14131413
// Returns the block with the given ID.
1414-
std::shared_ptr<Block> Engine::getBlock(const std::string &id)
1414+
std::shared_ptr<Block> Engine::getBlock(const std::string &id, Target *target)
14151415
{
14161416
if (id.empty())
14171417
return nullptr;
14181418

1419-
for (auto target : m_targets) {
1420-
int index = target->findBlock(id);
1419+
int index;
1420+
1421+
if (target) {
1422+
index = target->findBlock(id);
1423+
14211424
if (index != -1)
14221425
return target->blockAt(index);
14231426
}
14241427

1428+
for (auto t : m_targets) {
1429+
index = t->findBlock(id);
1430+
1431+
if (index != -1)
1432+
return t->blockAt(index);
1433+
}
1434+
14251435
return nullptr;
14261436
}
14271437

14281438
// Returns the variable with the given ID.
1429-
std::shared_ptr<Variable> Engine::getVariable(const std::string &id)
1439+
std::shared_ptr<Variable> Engine::getVariable(const std::string &id, Target *target)
14301440
{
14311441
if (id.empty())
14321442
return nullptr;
14331443

1434-
for (auto target : m_targets) {
1435-
int index = target->findVariableById(id);
1444+
Stage *stage = this->stage();
1445+
int index;
1446+
1447+
// Check stage
1448+
index = stage->findVariableById(id);
1449+
1450+
if (index != -1)
1451+
return stage->variableAt(index);
1452+
1453+
// Check currently compiled target
1454+
if (target != stage) {
1455+
index = target->findVariableById(id);
1456+
14361457
if (index != -1)
14371458
return target->variableAt(index);
14381459
}
14391460

1461+
// Fall back to checking all the other targets
1462+
for (auto t : m_targets) {
1463+
if (t.get() != stage && t.get() != target) {
1464+
int index = t->findVariableById(id);
1465+
1466+
if (index != -1)
1467+
return t->variableAt(index);
1468+
}
1469+
}
1470+
14401471
return nullptr;
14411472
}
14421473

14431474
// Returns the Scratch list with the given ID.
1444-
std::shared_ptr<List> Engine::getList(const std::string &id)
1475+
std::shared_ptr<List> Engine::getList(const std::string &id, Target *target)
14451476
{
14461477
if (id.empty())
14471478
return nullptr;
14481479

1449-
for (auto target : m_targets) {
1450-
int index = target->findListById(id);
1480+
Stage *stage = this->stage();
1481+
int index;
1482+
1483+
// Check stage
1484+
index = stage->findListById(id);
1485+
1486+
if (index != -1)
1487+
return stage->listAt(index);
1488+
1489+
// Check currently compiled target
1490+
if (target != stage) {
1491+
index = target->findListById(id);
1492+
14511493
if (index != -1)
14521494
return target->listAt(index);
14531495
}
14541496

1497+
// Fall back to checking all the other targets
1498+
for (auto t : m_targets) {
1499+
if (t.get() != stage && t.get() != target) {
1500+
int index = t->findListById(id);
1501+
1502+
if (index != -1)
1503+
return t->listAt(index);
1504+
}
1505+
}
1506+
14551507
return nullptr;
14561508
}
14571509

@@ -1469,35 +1521,40 @@ std::shared_ptr<Broadcast> Engine::getBroadcast(const std::string &id)
14691521
}
14701522

14711523
// Returns the comment with the given ID.
1472-
std::shared_ptr<Comment> Engine::getComment(const std::string &id)
1524+
std::shared_ptr<Comment> Engine::getComment(const std::string &id, Target *target)
14731525
{
14741526
if (id.empty())
14751527
return nullptr;
14761528

1477-
for (auto target : m_targets) {
1478-
int index = target->findComment(id);
1529+
int index;
1530+
1531+
if (target) {
1532+
index = target->findComment(id);
1533+
14791534
if (index != -1)
14801535
return target->commentAt(index);
14811536
}
14821537

1538+
for (auto t : m_targets) {
1539+
index = t->findComment(id);
1540+
1541+
if (index != -1)
1542+
return t->commentAt(index);
1543+
}
1544+
14831545
return nullptr;
14841546
}
14851547

14861548
// Returns the entity with the given ID. \see IEntity
1487-
std::shared_ptr<Entity> Engine::getEntity(const std::string &id)
1549+
std::shared_ptr<Entity> Engine::getEntity(const std::string &id, Target *target)
14881550
{
1489-
// Blocks
1490-
auto block = getBlock(id);
1491-
if (block)
1492-
return std::static_pointer_cast<Entity>(block);
1493-
14941551
// Variables
1495-
auto variable = getVariable(id);
1552+
auto variable = getVariable(id, target);
14961553
if (variable)
14971554
return std::static_pointer_cast<Entity>(variable);
14981555

14991556
// Lists
1500-
auto list = getList(id);
1557+
auto list = getList(id, target);
15011558
if (list)
15021559
return std::static_pointer_cast<Entity>(list);
15031560

@@ -1506,6 +1563,16 @@ std::shared_ptr<Entity> Engine::getEntity(const std::string &id)
15061563
if (broadcast)
15071564
return std::static_pointer_cast<Entity>(broadcast);
15081565

1566+
// Blocks
1567+
auto block = getBlock(id, target);
1568+
if (block)
1569+
return std::static_pointer_cast<Entity>(block);
1570+
1571+
// Comments
1572+
auto comment = getComment(id, target);
1573+
if (comment)
1574+
return std::static_pointer_cast<Entity>(comment);
1575+
15091576
return nullptr;
15101577
}
15111578

‎src/engine/internal/engine.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -202,12 +202,12 @@ class Engine : public IEngine
202202
void removeExecutableClones();
203203
void createMissingMonitors();
204204
void addVarOrListMonitor(std::shared_ptr<Monitor> monitor, Target *target);
205-
std::shared_ptr<Block> getBlock(const std::string &id);
206-
std::shared_ptr<Variable> getVariable(const std::string &id);
207-
std::shared_ptr<List> getList(const std::string &id);
205+
std::shared_ptr<Block> getBlock(const std::string &id, Target *target);
206+
std::shared_ptr<Variable> getVariable(const std::string &id, Target *target);
207+
std::shared_ptr<List> getList(const std::string &id, Target *target);
208208
std::shared_ptr<Broadcast> getBroadcast(const std::string &id);
209-
std::shared_ptr<Comment> getComment(const std::string &id);
210-
std::shared_ptr<Entity> getEntity(const std::string &id);
209+
std::shared_ptr<Comment> getComment(const std::string &id, Target *target);
210+
std::shared_ptr<Entity> getEntity(const std::string &id, Target *target);
211211
std::shared_ptr<IBlockSection> blockSection(const std::string &opcode) const;
212212

213213
void addHatToMap(std::unordered_map<Target *, std::vector<Script *>> &map, Script *script);

‎test/engine/engine_test.cpp‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2333,3 +2333,20 @@ TEST(EngineTest, AlwaysStopCloneThreads)
23332333
ASSERT_VAR(stage, "test");
23342334
ASSERT_EQ(GET_VAR(stage, "test")->value().toInt(), 0);
23352335
}
2336+
2337+
TEST(EngineTest, DuplicateVariableOrListIDs)
2338+
{
2339+
// Regtest for #567
2340+
Project p("regtest_projects/567_duplicate_variable_list_id.sb3");
2341+
ASSERT_TRUE(p.load());
2342+
2343+
auto engine = p.engine();
2344+
2345+
Stage *stage = engine->stage();
2346+
ASSERT_TRUE(stage);
2347+
2348+
engine->run();
2349+
2350+
ASSERT_VAR(stage, "passed");
2351+
ASSERT_TRUE(GET_VAR(stage, "passed")->value().toBool());
2352+
}

‎test/regtest_projects/567_duplicate_variable_list_id.sb3‎

1.65 KB
Binary file not shown.

0 commit comments

Comments
 (0)