Skip to content

Commit 3839ffd

Browse files
adazem009adazem009
committed
fix #305: Break the frame at the end of loops
1 parent 484aac8 commit 3839ffd

File tree

5 files changed

+65
-29
lines changed

5 files changed

+65
-29
lines changed

‎src/engine/compiler_p.cpp‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,10 @@ void CompilerPrivate::substackEnd()
3636
auto parent = substackTree.back();
3737
switch (parent.second) {
3838
case Compiler::SubstackType::Loop:
39-
if (!atomic) {
40-
if (!warp)
41-
addInstruction(OP_BREAK_ATOMIC);
42-
atomic = true;
43-
}
39+
// Break the frame at the end of the loop so that other scripts can run within the frame
40+
// This won't happen in "warp" scripts
41+
if (!warp)
42+
addInstruction(OP_BREAK_ATOMIC);
4443
addInstruction(OP_LOOP_END);
4544
break;
4645
case Compiler::SubstackType::IfStatement:

‎test/blocks/control_blocks_test.cpp‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -204,21 +204,21 @@ TEST_F(ControlBlocksTest, Forever)
204204

205205
compiler.compile(block1);
206206

207-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_FOREVER_LOOP, vm::OP_NULL, vm::OP_PRINT, vm::OP_LOOP_END, vm::OP_HALT }));
207+
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_FOREVER_LOOP, vm::OP_NULL, vm::OP_PRINT, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
208208
ASSERT_TRUE(compiler.constValues().empty());
209209
ASSERT_TRUE(compiler.variables().empty());
210210
ASSERT_TRUE(compiler.lists().empty());
211211

212212
compiler.compile(block2);
213213

214-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_FOREVER_LOOP, vm::OP_LOOP_END, vm::OP_HALT }));
214+
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_FOREVER_LOOP, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
215215
ASSERT_TRUE(compiler.constValues().empty());
216216
ASSERT_TRUE(compiler.variables().empty());
217217
ASSERT_TRUE(compiler.lists().empty());
218218

219219
compiler.compile(block3);
220220

221-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_FOREVER_LOOP, vm::OP_LOOP_END, vm::OP_HALT }));
221+
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_FOREVER_LOOP, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
222222
ASSERT_TRUE(compiler.constValues().empty());
223223
ASSERT_TRUE(compiler.variables().empty());
224224
ASSERT_TRUE(compiler.lists().empty());
@@ -248,7 +248,7 @@ TEST_F(ControlBlocksTest, Repeat)
248248

249249
compiler.compile(block1);
250250

251-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_CONST, 0, vm::OP_REPEAT_LOOP, vm::OP_NULL, vm::OP_PRINT, vm::OP_LOOP_END, vm::OP_HALT }));
251+
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_CONST, 0, vm::OP_REPEAT_LOOP, vm::OP_NULL, vm::OP_PRINT, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
252252
ASSERT_EQ(compiler.constValues().size(), 1);
253253
ASSERT_EQ(compiler.constValues()[0].toDouble(), 5);
254254
ASSERT_TRUE(compiler.variables().empty());
@@ -304,36 +304,38 @@ TEST_F(ControlBlocksTest, RepeatUntil)
304304

305305
compiler.compile(block1);
306306

307-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 0, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_NULL, vm::OP_PRINT, vm::OP_LOOP_END, vm::OP_HALT }));
307+
ASSERT_EQ(
308+
compiler.bytecode(),
309+
std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 0, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_NULL, vm::OP_PRINT, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
308310
ASSERT_EQ(compiler.constValues().size(), 1);
309311
ASSERT_EQ(compiler.constValues()[0].toBool(), false);
310312
ASSERT_TRUE(compiler.variables().empty());
311313
ASSERT_TRUE(compiler.lists().empty());
312314

313315
compiler.compile(block2);
314316

315-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 1, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_LOOP_END, vm::OP_HALT }));
317+
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 1, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
316318
ASSERT_EQ(compiler.constValues(), std::vector<Value>({ false, false }));
317319
ASSERT_TRUE(compiler.variables().empty());
318320
ASSERT_TRUE(compiler.lists().empty());
319321

320322
compiler.compile(block3);
321323

322-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 2, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_LOOP_END, vm::OP_HALT }));
324+
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 2, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
323325
ASSERT_EQ(compiler.constValues(), std::vector<Value>({ false, false, false }));
324326
ASSERT_TRUE(compiler.variables().empty());
325327
ASSERT_TRUE(compiler.lists().empty());
326328

327329
compiler.compile(block4);
328330

329-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 3, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_LOOP_END, vm::OP_HALT }));
331+
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 3, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
330332
ASSERT_EQ(compiler.constValues(), std::vector<Value>({ false, false, false, Value() }));
331333
ASSERT_TRUE(compiler.variables().empty());
332334
ASSERT_TRUE(compiler.lists().empty());
333335

334336
compiler.compile(block5);
335337

336-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_NULL, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_LOOP_END, vm::OP_HALT }));
338+
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_NULL, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
337339
ASSERT_EQ(compiler.constValues(), std::vector<Value>({ false, false, false, Value() }));
338340
ASSERT_TRUE(compiler.variables().empty());
339341
ASSERT_TRUE(compiler.lists().empty());
@@ -374,36 +376,43 @@ TEST_F(ControlBlocksTest, While)
374376

375377
ASSERT_EQ(
376378
compiler.bytecode(),
377-
std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 0, vm::OP_NOT, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_NULL, vm::OP_PRINT, vm::OP_LOOP_END, vm::OP_HALT }));
379+
std::vector<unsigned int>(
380+
{ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 0, vm::OP_NOT, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_NULL, vm::OP_PRINT, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
378381
ASSERT_EQ(compiler.constValues().size(), 1);
379382
ASSERT_EQ(compiler.constValues()[0].toBool(), false);
380383
ASSERT_TRUE(compiler.variables().empty());
381384
ASSERT_TRUE(compiler.lists().empty());
382385

383386
compiler.compile(block2);
384387

385-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 1, vm::OP_NOT, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_LOOP_END, vm::OP_HALT }));
388+
ASSERT_EQ(
389+
compiler.bytecode(),
390+
std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 1, vm::OP_NOT, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
386391
ASSERT_EQ(compiler.constValues(), std::vector<Value>({ false, false }));
387392
ASSERT_TRUE(compiler.variables().empty());
388393
ASSERT_TRUE(compiler.lists().empty());
389394

390395
compiler.compile(block3);
391396

392-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 2, vm::OP_NOT, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_LOOP_END, vm::OP_HALT }));
397+
ASSERT_EQ(
398+
compiler.bytecode(),
399+
std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 2, vm::OP_NOT, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
393400
ASSERT_EQ(compiler.constValues(), std::vector<Value>({ false, false, false }));
394401
ASSERT_TRUE(compiler.variables().empty());
395402
ASSERT_TRUE(compiler.lists().empty());
396403

397404
compiler.compile(block4);
398405

399-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 3, vm::OP_NOT, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_LOOP_END, vm::OP_HALT }));
406+
ASSERT_EQ(
407+
compiler.bytecode(),
408+
std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_CONST, 3, vm::OP_NOT, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
400409
ASSERT_EQ(compiler.constValues(), std::vector<Value>({ false, false, false, Value() }));
401410
ASSERT_TRUE(compiler.variables().empty());
402411
ASSERT_TRUE(compiler.lists().empty());
403412

404413
compiler.compile(block5);
405414

406-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_NULL, vm::OP_NOT, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_LOOP_END, vm::OP_HALT }));
415+
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_UNTIL_LOOP, vm::OP_NULL, vm::OP_NOT, vm::OP_BEGIN_UNTIL_LOOP, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
407416
ASSERT_EQ(compiler.constValues(), std::vector<Value>({ false, false, false, Value() }));
408417
ASSERT_TRUE(compiler.variables().empty());
409418
ASSERT_TRUE(compiler.lists().empty());
@@ -439,7 +448,8 @@ TEST_F(ControlBlocksTest, ForEach)
439448

440449
ASSERT_EQ(
441450
compiler.bytecode(),
442-
std::vector<unsigned int>({ vm::OP_START, vm::OP_CONST, 0, vm::OP_REPEAT_LOOP, vm::OP_REPEAT_LOOP_INDEX1, vm::OP_SET_VAR, 0, vm::OP_NULL, vm::OP_PRINT, vm::OP_LOOP_END, vm::OP_HALT }));
451+
std::vector<unsigned int>(
452+
{ vm::OP_START, vm::OP_CONST, 0, vm::OP_REPEAT_LOOP, vm::OP_REPEAT_LOOP_INDEX1, vm::OP_SET_VAR, 0, vm::OP_NULL, vm::OP_PRINT, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
443453
ASSERT_EQ(compiler.constValues().size(), 1);
444454
ASSERT_EQ(compiler.constValues()[0].toDouble(), 5);
445455
ASSERT_EQ(compiler.variables().size(), 1);

‎test/compiler/compiler_test.cpp‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,8 @@ TEST_F(CompilerTest, RepeatLoop)
508508
compiler.compile(engine.targetAt(0)->greenFlagBlocks().at(0));
509509
ASSERT_EQ(
510510
compiler.bytecode(),
511-
std::vector<unsigned int>({ vm::OP_START, vm::OP_CONST, 0, vm::OP_SET_VAR, 0, vm::OP_CONST, 1, vm::OP_REPEAT_LOOP, vm::OP_CONST, 2, vm::OP_CHANGE_VAR, 0, vm::OP_LOOP_END, vm::OP_HALT }));
511+
std::vector<unsigned int>(
512+
{ vm::OP_START, vm::OP_CONST, 0, vm::OP_SET_VAR, 0, vm::OP_CONST, 1, vm::OP_REPEAT_LOOP, vm::OP_CONST, 2, vm::OP_CHANGE_VAR, 0, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
512513
ASSERT_EQ(compiler.variablePtrs().size(), 1);
513514
ASSERT_EQ(compiler.variablePtrs()[0]->toString(), "test");
514515
ASSERT_EQ(compiler.lists().size(), 0);
@@ -537,7 +538,7 @@ TEST_F(CompilerTest, ForeverLoop)
537538
compiler.compile(engine.targetAt(0)->greenFlagBlocks().at(0));
538539
ASSERT_EQ(
539540
compiler.bytecode(),
540-
std::vector<unsigned int>({ vm::OP_START, vm::OP_CONST, 0, vm::OP_SET_VAR, 0, vm::OP_FOREVER_LOOP, vm::OP_CONST, 1, vm::OP_CHANGE_VAR, 0, vm::OP_LOOP_END, vm::OP_HALT }));
541+
std::vector<unsigned int>({ vm::OP_START, vm::OP_CONST, 0, vm::OP_SET_VAR, 0, vm::OP_FOREVER_LOOP, vm::OP_CONST, 1, vm::OP_CHANGE_VAR, 0, vm::OP_BREAK_ATOMIC, vm::OP_LOOP_END, vm::OP_HALT }));
541542
ASSERT_EQ(compiler.constValues(), std::vector<Value>({ 0, 1 }));
542543
}
543544

@@ -563,6 +564,7 @@ TEST_F(CompilerTest, RepeatUntilLoop)
563564
1,
564565
vm::OP_CHANGE_VAR,
565566
0,
567+
vm::OP_BREAK_ATOMIC,
566568
vm::OP_LOOP_END,
567569
vm::OP_HALT }));
568570
ASSERT_EQ(compiler.constValues(), std::vector<Value>({ 0, 1 }));
@@ -591,6 +593,7 @@ TEST_F(CompilerTest, RepeatWhileLoop)
591593
1,
592594
vm::OP_CHANGE_VAR,
593595
0,
596+
vm::OP_BREAK_ATOMIC,
594597
vm::OP_LOOP_END,
595598
vm::OP_HALT }));
596599
ASSERT_EQ(compiler.constValues(), std::vector<Value>({ 0, 1 }));
@@ -620,6 +623,7 @@ TEST_F(CompilerTest, RepeatForEachLoop)
620623
2,
621624
vm::OP_CHANGE_VAR,
622625
0,
626+
vm::OP_BREAK_ATOMIC,
623627
vm::OP_LOOP_END,
624628
vm::OP_HALT }));
625629
ASSERT_EQ(compiler.constValues(), std::vector<Value>({ 0, 10, 1 }));
@@ -731,6 +735,7 @@ TEST_F(CompilerTest, NestedStatements)
731735
vm::OP_CHANGE_VAR,
732736
0,
733737
vm::OP_ENDIF,
738+
vm::OP_BREAK_ATOMIC,
734739
vm::OP_LOOP_END,
735740
vm::OP_ELSE,
736741
vm::OP_CONST,
@@ -748,6 +753,7 @@ TEST_F(CompilerTest, NestedStatements)
748753
vm::OP_CHANGE_VAR,
749754
0,
750755
vm::OP_ENDIF,
756+
vm::OP_BREAK_ATOMIC,
751757
vm::OP_LOOP_END,
752758
vm::OP_ENDIF,
753759
vm::OP_HALT }));
@@ -788,7 +794,10 @@ TEST_F(CompilerTest, CustomBlocks)
788794
}
789795
ASSERT_TRUE(definition);
790796
compiler.compile(definition);
791-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_WARP, vm::OP_READ_ARG, 0, vm::OP_SET_VAR, 0, vm::OP_READ_ARG, 1, vm::OP_SET_VAR, 1, vm::OP_HALT }));
797+
ASSERT_EQ(
798+
compiler.bytecode(),
799+
std::vector<unsigned int>(
800+
{ vm::OP_START, vm::OP_WARP, vm::OP_CONST, 1, vm::OP_REPEAT_LOOP, vm::OP_READ_ARG, 0, vm::OP_SET_VAR, 0, vm::OP_READ_ARG, 1, vm::OP_SET_VAR, 1, vm::OP_LOOP_END, vm::OP_HALT }));
792801

793802
definition = nullptr;
794803
for (auto block : stage->blocks()) {
@@ -797,7 +806,7 @@ TEST_F(CompilerTest, CustomBlocks)
797806
}
798807
ASSERT_TRUE(definition);
799808
compiler.compile(definition);
800-
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_CONST, 1, vm::OP_SET_VAR, 2, vm::OP_HALT }));
809+
ASSERT_EQ(compiler.bytecode(), std::vector<unsigned int>({ vm::OP_START, vm::OP_CONST, 2, vm::OP_SET_VAR, 2, vm::OP_HALT }));
801810
}
802811

803812
TEST_F(CompilerTest, MultipleTargets)
@@ -808,7 +817,7 @@ TEST_F(CompilerTest, MultipleTargets)
808817

809818
auto sprite1 = engine.targetAt(engine.findTarget("Sprite1"));
810819
auto script = scripts.at(sprite1->greenFlagBlocks().at(0));
811-
ASSERT_EQ(script->bytecodeVector().size(), 32);
820+
ASSERT_EQ(script->bytecodeVector().size(), 33);
812821
auto vm = script->start();
813822
ASSERT_EQ(vm->target(), sprite1);
814823
ASSERT_EQ(vm->engine(), &engine);

‎test/custom_blocks.sb3‎

-379 Bytes
Binary file not shown.

‎test/engine/engine_test.cpp‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,17 +94,35 @@ TEST(EngineTest, FpsProject)
9494
std::chrono::steady_clock::time_point time2(std::chrono::milliseconds(75));
9595
std::chrono::steady_clock::time_point time3(std::chrono::milliseconds(83));
9696
std::chrono::steady_clock::time_point time4(std::chrono::milliseconds(116));
97-
EXPECT_CALL(clock, currentSteadyTime()).WillOnce(Return(time1)).WillOnce(Return(time2)).WillOnce(Return(time3)).WillOnce(Return(time4)).WillOnce(Return(time4));
98-
EXPECT_CALL(clock, sleep(std::chrono::milliseconds(8)));
97+
EXPECT_CALL(clock, currentSteadyTime())
98+
.WillOnce(Return(time1))
99+
.WillOnce(Return(time1))
100+
.WillOnce(Return(time2))
101+
.WillOnce(Return(time2))
102+
.WillOnce(Return(time3))
103+
.WillOnce(Return(time3))
104+
.WillOnce(Return(time4))
105+
.WillOnce(Return(time4));
106+
EXPECT_CALL(clock, sleep(std::chrono::milliseconds(33)));
107+
EXPECT_CALL(clock, sleep(std::chrono::milliseconds(25)));
99108
p.run();
100109

101110
engine->setFps(10);
102111
std::chrono::steady_clock::time_point time5(std::chrono::milliseconds(100));
103112
std::chrono::steady_clock::time_point time6(std::chrono::milliseconds(115));
104113
std::chrono::steady_clock::time_point time7(std::chrono::milliseconds(200));
105114
std::chrono::steady_clock::time_point time8(std::chrono::milliseconds(300));
106-
EXPECT_CALL(clock, currentSteadyTime()).WillOnce(Return(time5)).WillOnce(Return(time6)).WillOnce(Return(time7)).WillOnce(Return(time8)).WillOnce(Return(time8));
107-
EXPECT_CALL(clock, sleep(std::chrono::milliseconds(85)));
115+
EXPECT_CALL(clock, currentSteadyTime())
116+
.WillOnce(Return(time5))
117+
.WillOnce(Return(time5))
118+
.WillOnce(Return(time6))
119+
.WillOnce(Return(time6))
120+
.WillOnce(Return(time7))
121+
.WillOnce(Return(time7))
122+
.WillOnce(Return(time8))
123+
.WillOnce(Return(time8));
124+
EXPECT_CALL(clock, sleep(std::chrono::milliseconds(100)));
125+
EXPECT_CALL(clock, sleep(std::chrono::milliseconds(15)));
108126
p.run();
109127
}
110128

0 commit comments

Comments
 (0)