Skip to content

Commit 17a745d

Browse files
adazem009adazem009
committed
Use field pointer instead of field ID in hat methods
Resolves: #513
1 parent 572a2ea commit 17a745d

File tree

6 files changed

+41
-38
lines changed

6 files changed

+41
-38
lines changed

‎include/scratchcpp/iengine.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ namespace libscratchcpp
1717
class IExtension;
1818
class Broadcast;
1919
class Block;
20+
class Field;
2021
class Target;
2122
class Sprite;
2223
class Stage;
@@ -294,16 +295,16 @@ class LIBSCRATCHCPP_EXPORT IEngine
294295
virtual void addGreenFlagScript(std::shared_ptr<Block> hatBlock) = 0;
295296

296297
/*! Registers the broadcast script. */
297-
virtual void addBroadcastScript(std::shared_ptr<Block> whenReceivedBlock, int fieldId, Broadcast *broadcast) = 0;
298+
virtual void addBroadcastScript(std::shared_ptr<Block> whenReceivedBlock, Field *field, Broadcast *broadcast) = 0;
298299

299300
/*! Registers the backdrop change script. */
300-
virtual void addBackdropChangeScript(std::shared_ptr<Block> hatBlock, int fieldId) = 0;
301+
virtual void addBackdropChangeScript(std::shared_ptr<Block> hatBlock, Field *field) = 0;
301302

302303
/* Registers the given "when I start as clone" script. */
303304
virtual void addCloneInitScript(std::shared_ptr<Block> hatBlock) = 0;
304305

305306
/* Registers the given "when key pressed" script. */
306-
virtual void addKeyPressScript(std::shared_ptr<Block> hatBlock, int fieldId) = 0;
307+
virtual void addKeyPressScript(std::shared_ptr<Block> hatBlock, Field *field) = 0;
307308

308309
/* Registers the given "when this sprite/stage clicked" script. */
309310
virtual void addTargetClickScript(std::shared_ptr<Block> hatBlock) = 0;

‎src/blocks/eventblocks.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,14 +113,15 @@ void EventBlocks::compileBroadcastAndWait(Compiler *compiler)
113113

114114
void EventBlocks::compileWhenBroadcastReceived(Compiler *compiler)
115115
{
116-
auto broadcast = std::static_pointer_cast<Broadcast>(compiler->field(BROADCAST_OPTION)->valuePtr());
116+
Field *field = compiler->field(BROADCAST_OPTION);
117+
auto broadcast = std::static_pointer_cast<Broadcast>(field->valuePtr());
117118

118-
compiler->engine()->addBroadcastScript(compiler->block(), BROADCAST_OPTION, broadcast.get());
119+
compiler->engine()->addBroadcastScript(compiler->block(), field, broadcast.get());
119120
}
120121

121122
void EventBlocks::compileWhenBackdropSwitchesTo(Compiler *compiler)
122123
{
123-
compiler->engine()->addBackdropChangeScript(compiler->block(), BACKDROP);
124+
compiler->engine()->addBackdropChangeScript(compiler->block(), compiler->field(BACKDROP));
124125
}
125126

126127
void EventBlocks::compileWhenGreaterThanPredicate(Compiler *compiler)
@@ -158,7 +159,7 @@ void EventBlocks::compileWhenGreaterThan(Compiler *compiler)
158159
void EventBlocks::compileWhenKeyPressed(Compiler *compiler)
159160
{
160161
// NOTE: Field values don't have to be registered because keys are referenced by their names
161-
compiler->engine()->addKeyPressScript(compiler->block(), KEY_OPTION);
162+
compiler->engine()->addKeyPressScript(compiler->block(), compiler->field(KEY_OPTION));
162163
}
163164

164165
unsigned int EventBlocks::whenTouchingObjectPredicate(VirtualMachine *vm)

‎src/engine/internal/engine.cpp‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,7 +1057,7 @@ void Engine::addGreenFlagScript(std::shared_ptr<Block> hatBlock)
10571057
addHatToMap(m_greenFlagHats, m_scripts[hatBlock].get());
10581058
}
10591059

1060-
void Engine::addBroadcastScript(std::shared_ptr<Block> whenReceivedBlock, int fieldId, Broadcast *broadcast)
1060+
void Engine::addBroadcastScript(std::shared_ptr<Block> whenReceivedBlock, Field *field, Broadcast *broadcast)
10611061
{
10621062
assert(!broadcast->isBackdropBroadcast());
10631063
Script *script = m_scripts[whenReceivedBlock].get();
@@ -1073,19 +1073,22 @@ void Engine::addBroadcastScript(std::shared_ptr<Block> whenReceivedBlock, int fi
10731073
m_broadcastMap[broadcast] = { script };
10741074

10751075
addHatToMap(m_broadcastHats, script);
1076-
addHatField(script, HatField::BroadcastOption, fieldId);
1076+
addHatField(script, HatField::BroadcastOption, field);
10771077
}
10781078

1079-
void Engine::addBackdropChangeScript(std::shared_ptr<Block> hatBlock, int fieldId)
1079+
void Engine::addBackdropChangeScript(std::shared_ptr<Block> hatBlock, Field *field)
10801080
{
10811081
Stage *stage = this->stage();
10821082

10831083
if (!stage)
10841084
return;
10851085

1086-
// TODO: This assumes the first field holds the broadcast pointer, maybe this is not the best way (e. g. if an extension uses this method)
1087-
assert(hatBlock->fieldAt(0));
1088-
const std::string &backdropName = hatBlock->fieldAt(0)->value().toString();
1086+
if (!field) {
1087+
assert(false);
1088+
return;
1089+
}
1090+
1091+
const std::string &backdropName = field->value().toString();
10891092
auto backdrop = stage->costumeAt(stage->findCostume(backdropName));
10901093

10911094
if (!backdrop)
@@ -1107,19 +1110,19 @@ void Engine::addBackdropChangeScript(std::shared_ptr<Block> hatBlock, int fieldI
11071110
m_backdropBroadcastMap[broadcast] = { script };
11081111

11091112
addHatToMap(m_backdropChangeHats, script);
1110-
addHatField(script, HatField::Backdrop, fieldId);
1113+
addHatField(script, HatField::Backdrop, field);
11111114
}
11121115

11131116
void Engine::addCloneInitScript(std::shared_ptr<Block> hatBlock)
11141117
{
11151118
addHatToMap(m_cloneInitHats, m_scripts[hatBlock].get());
11161119
}
11171120

1118-
void Engine::addKeyPressScript(std::shared_ptr<Block> hatBlock, int fieldId)
1121+
void Engine::addKeyPressScript(std::shared_ptr<Block> hatBlock, Field *field)
11191122
{
11201123
Script *script = m_scripts[hatBlock].get();
11211124
addHatToMap(m_whenKeyPressedHats, script);
1122-
addHatField(script, HatField::KeyOption, fieldId);
1125+
addHatField(script, HatField::KeyOption, field);
11231126
}
11241127

11251128
void Engine::addTargetClickScript(std::shared_ptr<Block> hatBlock)
@@ -1607,15 +1610,15 @@ void Engine::addHatToMap(std::unordered_map<Target *, std::vector<Script *>> &ma
16071610
map[target] = { script };
16081611
}
16091612

1610-
void Engine::addHatField(Script *script, HatField field, int fieldId)
1613+
void Engine::addHatField(Script *script, HatField hatField, Field *targetField)
16111614
{
16121615
auto it = m_scriptHatFields.find(script);
16131616

16141617
if (it == m_scriptHatFields.cend())
1615-
m_scriptHatFields[script] = { { field, fieldId } };
1618+
m_scriptHatFields[script] = { { hatField, targetField } };
16161619
else {
16171620
auto &fieldMap = it->second;
1618-
fieldMap[field] = fieldId;
1621+
fieldMap[hatField] = targetField;
16191622
}
16201623
}
16211624

@@ -2045,8 +2048,6 @@ std::vector<std::shared_ptr<Thread>> Engine::startHats(HatType hatType, const st
20452048
allScriptsByOpcodeDo(
20462049
hatType,
20472050
[this, hatType, &optMatchFields, &newThreads](Script *script, Target *target) {
2048-
auto topBlock = script->topBlock();
2049-
20502051
if (!optMatchFields.empty()) {
20512052
// Get the field map for this script
20522053
auto fieldMapIt = m_scriptHatFields.find(script);
@@ -2061,20 +2062,20 @@ std::vector<std::shared_ptr<Thread>> Engine::startHats(HatType hatType, const st
20612062
assert(fieldIt != fieldMap.cend());
20622063

20632064
if (fieldIt != fieldMap.cend()) {
2064-
assert(topBlock->findFieldById(fieldIt->second));
2065+
assert(fieldIt->second);
20652066

20662067
if (std::holds_alternative<std::string>(fieldValue)) {
2067-
if (topBlock->findFieldById(fieldIt->second)->value().toString() != std::get<std::string>(fieldValue)) {
2068+
if (fieldIt->second->value().toString() != std::get<std::string>(fieldValue)) {
20682069
// Field mismatch
20692070
return;
20702071
}
20712072
} else if (std::holds_alternative<const char *>(fieldValue)) {
2072-
if (topBlock->findFieldById(fieldIt->second)->value().toString() != std::string(std::get<const char *>(fieldValue))) {
2073+
if (fieldIt->second->value().toString() != std::string(std::get<const char *>(fieldValue))) {
20732074
// Field mismatch
20742075
return;
20752076
}
20762077
} else {
2077-
if (topBlock->findFieldById(fieldIt->second)->valuePtr().get() != std::get<Entity *>(fieldValue)) {
2078+
if (fieldIt->second->valuePtr().get() != std::get<Entity *>(fieldValue)) {
20782079
// Field mismatch
20792080
return;
20802081
}
@@ -2103,7 +2104,7 @@ std::vector<std::shared_ptr<Thread>> Engine::startHats(HatType hatType, const st
21032104
}
21042105

21052106
// Start the thread with this top block
2106-
newThreads.push_back(pushThread(topBlock, target));
2107+
newThreads.push_back(pushThread(script->topBlock(), target));
21072108
},
21082109
optTarget);
21092110

‎src/engine/internal/engine.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,10 @@ class Engine : public IEngine
122122

123123
void addWhenTouchingObjectScript(std::shared_ptr<Block> hatBlock) override;
124124
void addGreenFlagScript(std::shared_ptr<Block> hatBlock) override;
125-
void addBroadcastScript(std::shared_ptr<Block> whenReceivedBlock, int fieldId, Broadcast *broadcast) override;
126-
void addBackdropChangeScript(std::shared_ptr<Block> hatBlock, int fieldId) override;
125+
void addBroadcastScript(std::shared_ptr<Block> whenReceivedBlock, Field *field, Broadcast *broadcast) override;
126+
void addBackdropChangeScript(std::shared_ptr<Block> hatBlock, Field *field) override;
127127
void addCloneInitScript(std::shared_ptr<Block> hatBlock) override;
128-
void addKeyPressScript(std::shared_ptr<Block> hatBlock, int fieldId) override;
128+
void addKeyPressScript(std::shared_ptr<Block> hatBlock, Field *field) override;
129129
void addTargetClickScript(std::shared_ptr<Block> hatBlock) override;
130130
void addWhenGreaterThanScript(std::shared_ptr<Block> hatBlock) override;
131131

@@ -215,7 +215,7 @@ class Engine : public IEngine
215215
std::shared_ptr<Entity> getEntity(const std::string &id, Target *target);
216216

217217
void addHatToMap(std::unordered_map<Target *, std::vector<Script *>> &map, Script *script);
218-
void addHatField(Script *script, HatField field, int fieldId);
218+
void addHatField(Script *script, HatField hatField, Field *targetField);
219219
const std::vector<libscratchcpp::Script *> &getHats(Target *target, HatType type);
220220

221221
void updateSpriteLayerOrder();
@@ -270,7 +270,7 @@ class Engine : public IEngine
270270
std::unordered_map<Target *, std::vector<Script *>> m_whenTargetClickedHats;
271271
std::unordered_map<Target *, std::vector<Script *>> m_whenGreaterThanHats;
272272

273-
std::unordered_map<Script *, std::unordered_map<HatField, int>> m_scriptHatFields; // HatField, field ID from the block implementation
273+
std::unordered_map<Script *, std::unordered_map<HatField, Field *>> m_scriptHatFields; // HatField, field from the block implementation
274274

275275
std::unordered_map<Block *, std::unordered_map<Target *, bool>> m_edgeActivatedHatValues; // (block, target, last value) edge-activated hats only run after the value changes from false to true
276276

‎test/blocks/event_blocks_test.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ TEST_F(EventBlocksTest, WhenBroadcastReceived)
475475
auto block1 = createEventBlock("a", "event_whenbroadcastreceived");
476476
addBroadcastField(block1, "BROADCAST_OPTION", EventBlocks::BROADCAST_OPTION, m_broadcast);
477477

478-
EXPECT_CALL(m_engineMock, addBroadcastScript(block1, EventBlocks::BROADCAST_OPTION, m_broadcast.get())).Times(1);
478+
EXPECT_CALL(m_engineMock, addBroadcastScript(block1, block1->fieldAt(0).get(), m_broadcast.get())).Times(1);
479479

480480
compiler.init();
481481
compiler.setBlock(block1);
@@ -500,7 +500,7 @@ TEST_F(EventBlocksTest, WhenBackdropSwitchesTo)
500500
auto backdrop = std::make_shared<Costume>("backdrop2", "a", "svg");
501501
stage.addCostume(backdrop);
502502

503-
EXPECT_CALL(m_engineMock, addBackdropChangeScript(block1, EventBlocks::BACKDROP));
503+
EXPECT_CALL(m_engineMock, addBackdropChangeScript(block1, block1->fieldAt(0).get()));
504504

505505
compiler.init();
506506
compiler.setBlock(block1);
@@ -655,11 +655,11 @@ TEST_F(EventBlocksTest, WhenKeyPressed)
655655

656656
compiler.init();
657657

658-
EXPECT_CALL(m_engineMock, addKeyPressScript(block1, EventBlocks::KEY_OPTION));
658+
EXPECT_CALL(m_engineMock, addKeyPressScript(block1, block1->fieldAt(0).get()));
659659
compiler.setBlock(block1);
660660
EventBlocks::compileWhenKeyPressed(&compiler);
661661

662-
EXPECT_CALL(m_engineMock, addKeyPressScript(block2, EventBlocks::KEY_OPTION));
662+
EXPECT_CALL(m_engineMock, addKeyPressScript(block2, block2->fieldAt(0).get()));
663663
compiler.setBlock(block2);
664664
EventBlocks::compileWhenKeyPressed(&compiler);
665665

‎test/mocks/enginemock.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,10 @@ class EngineMock : public IEngine
104104

105105
MOCK_METHOD(void, addWhenTouchingObjectScript, (std::shared_ptr<Block>), (override));
106106
MOCK_METHOD(void, addGreenFlagScript, (std::shared_ptr<Block>), (override));
107-
MOCK_METHOD(void, addBroadcastScript, (std::shared_ptr<Block>, int, Broadcast *), (override));
108-
MOCK_METHOD(void, addBackdropChangeScript, (std::shared_ptr<Block>, int), (override));
107+
MOCK_METHOD(void, addBroadcastScript, (std::shared_ptr<Block>, Field *, Broadcast *), (override));
108+
MOCK_METHOD(void, addBackdropChangeScript, (std::shared_ptr<Block>, Field *), (override));
109109
MOCK_METHOD(void, addCloneInitScript, (std::shared_ptr<Block>), (override));
110-
MOCK_METHOD(void, addKeyPressScript, (std::shared_ptr<Block>, int), (override));
110+
MOCK_METHOD(void, addKeyPressScript, (std::shared_ptr<Block>, Field *), (override));
111111
MOCK_METHOD(void, addTargetClickScript, (std::shared_ptr<Block>), (override));
112112
MOCK_METHOD(void, addWhenGreaterThanScript, (std::shared_ptr<Block>), (override));
113113

0 commit comments

Comments
 (0)