diff --git a/TODO.md b/TODO.md index 4d14e78..00b3b9e 100644 --- a/TODO.md +++ b/TODO.md @@ -2352,6 +2352,26 @@ each is here so the reasoning does not have to be reconstructed. function answering 42 at the REPL -- registered in `AKBASIC_KNOWN_FAILING_TESTS` until it does. +42. ~~**A name that merely contains a branch verb was treated as a branch.**~~ **Fixed.** The + target prescan walks a line character by character looking for `GOTO`, `GOSUB`, + `COLLISION` and the rest, and checked only the character *after* a match: + + ```basic + T# = RCOLLISION(1, 1) + ``` + + ```output + ? 2 : PARSE ERROR Line 2: branch to line 1, which the program did not number. Branch by LABEL, or RENUMBER first + + ``` + + `COLLISION` was found at `RCOLLISION`'s second character, the `(1,` after it read as a + handler line number, and the program was refused -- naming a line that contains no branch. + `src/renumber.c` now requires a non-alphanumeric character on *both* sides. Its own + comment was already right that the trailing check protects `GOTOX#`; nothing protected + `XGOTO#`, and `RCOLLISION` is the first name in the language to reach it. + `tests/unnumbered.c` covers all three shapes. + ## 7. Filing gaps against `libakgl` When phase 7 or phase 8 needs something `libakgl` does not have, **stop and file it** in diff --git a/examples/breakout/characters/breakout.bas b/examples/breakout/characters/breakout.bas index 7e7f46a..49aa588 100644 --- a/examples/breakout/characters/breakout.bas +++ b/examples/breakout/characters/breakout.bas @@ -114,6 +114,11 @@ OX# = 0 OY# = 0 K# = 0 HIT# = 0 +SX# = 0 +SY# = 0 +A# = 0 +D% = 0.0 +M# = 0 PVX# = 0 DOFF# = 0 STALL# = 0 @@ -179,6 +184,7 @@ CHAR 1, 0, 1, FRAME$ GOSUB MKSPR GOSUB SNDPROBE +COLLISION 2, BRICKHIT GOTO TITLE REM #################################################################### @@ -333,6 +339,10 @@ FOR R# = 0 TO 5 NEXT R# RETURN +REM One wall row. Each brick is registered with SOLID as well as recorded in +REM BR#(), so the interpreter can answer "did the ball hit a brick" and, more +REM usefully, "which one and which way is out". The id is the array index plus +REM one, so nothing has to be looked up on the way back. LABEL BUILDR FOR C# = 0 TO 9 T$ = MID(S$, C#, 1) @@ -340,6 +350,10 @@ FOR C# = 0 TO 9 BR#(I#) = 0 IF T$ = "1" THEN BR#(I#) = 1 IF T$ = "1" THEN LEFTN# = LEFTN# + 1 + SX# = (BLEFT# + (C# * BRW#)) * CW# + SY# = (BTOP# + R#) * CH# + IF T$ = "1" THEN SOLID I# + 1, SX#, SY#, SX# + (BRW# * CW#), SY# + CH# + IF T$ = "0" THEN SOLID I# + 1 NEXT C# RETURN @@ -498,16 +512,16 @@ REM X first and then Y, each move tested on its own, so a ball arriving REM at the corner of a brick reverses one axis rather than both. REM #################################################################### +REM Bricks are not tested here any more. COLLISION 2 fires when the ball meets +REM a SOLID rectangle and BRICKHIT does the rest, which is why there is no +REM OX#/OY# backup: the handler pushes the ball out along the contact normal +REM rather than restoring where it came from. LABEL MOVEBAL -OX# = BX# BX# = BX# + BVX# IF BX# < 0 THEN GOSUB WALLL IF BX# > MAXX# THEN GOSUB WALLR -GOSUB XBRICK -OY# = BY# BY# = BY# + BVY# IF BY# < TOPY# THEN GOSUB WALLT -GOSUB YBRICK GOSUB PADHIT IF BY# > LOSEY# THEN STATE# = 1 STALL# = STALL# + 1 @@ -532,50 +546,42 @@ BVY# = 0 - BVY# GOSUB BEEPW RETURN -REM The leading edge, not the centre: a ball tested by its centre sinks -REM four pixels into a brick before it turns, which looks like the brick -REM was hit late and the one beside it was not hit at all. -LABEL XBRICK -TX# = BX# -IF BVX# > 0 THEN TX# = BX# + 7 -TY# = BY# + 4 -GOSUB HITTEST -IF HIT# = 0 THEN RETURN -BX# = OX# -BVX# = 0 - BVX# -GOSUB KILLBR -RETURN +REM #################################################################### +REM BRICKS +REM +REM The whole of it. COLLISION 2 fires between source lines when the ball +REM overlaps a rectangle SOLID registered, and RCOLLISION says which one and +REM which way to go. This replaced HITTEST, XBRICK and YBRICK -- about forty +REM lines that divided pixels by cell sizes to recover a grid index, tested +REM the ball's leading edge rather than its box, and could miss a brick +REM clipped at the corner by seven pixels' worth of ball. +REM +REM Field 4 is how deep the overlap is and fields 2 and 3 are the way out, so +REM the two together put the ball exactly clear. The normal of a box against a +REM box is exactly -1, 0 or 1, which is why assigning the product into an +REM integer is safe here -- see chapter 13 on the left operand. +REM #################################################################### -LABEL YBRICK -TX# = BX# + 4 -TY# = BY# -IF BVY# > 0 THEN TY# = BY# + 7 -GOSUB HITTEST -IF HIT# = 0 THEN RETURN -BY# = OY# -BVY# = 0 - BVY# +LABEL BRICKHIT +M# = BUMP(2) +IF (M# AND 1) = 0 THEN RETURN +T# = RCOLLISION(1, 1) +IF T# < 1 THEN RETURN +IF BR#(T# - 1) = 0 THEN RETURN +D% = RCOLLISION(1, 4) +BX# = BX# + (RCOLLISION(1, 2) * D%) +BY# = BY# + (RCOLLISION(1, 3) * D%) +A# = RCOLLISION(1, 7) +IF A# = 1 THEN BVX# = 0 - BVX# +IF A# = 2 THEN BVY# = 0 - BVY# +BI# = T# - 1 +BR2# = BI# / BCOLS# GOSUB KILLBR RETURN -REM Which brick, if any, holds the pixel (TX#, TY#). This is the only -REM place in the game that converts pixels into cells. -LABEL HITTEST -HIT# = 0 -RC# = TY# / CH# -IF RC# < BTOP# THEN RETURN -RB# = RC# - BTOP# -IF RB# > (BROWS# - 1) THEN RETURN -CC2# = TX# / CW# -IF CC2# < BLEFT# THEN RETURN -CB# = (CC2# - BLEFT#) / BRW# -IF CB# > (BCOLS# - 1) THEN RETURN -BI# = (RB# * BCOLS#) + CB# -BR2# = RB# -IF BR#(BI#) = 1 THEN HIT# = 1 -RETURN - LABEL KILLBR BR#(BI#) = 0 +SOLID BI# + 1 LEFTN# = LEFTN# - 1 STALL# = 0 PTS# = (BROWS# - BR2#) * 10 diff --git a/src/renumber.c b/src/renumber.c index bb38f51..aa7d40e 100644 --- a/src/renumber.c +++ b/src/renumber.c @@ -214,8 +214,23 @@ static akerr_ErrorContext *rewrite_line(akbasic_TargetWalk *walk, const char *co continue; } + /* + * **Both ends, not just the trailing one.** The check below used to look + * only at the character *after* a match, so a name ending in a branch + * verb matched inside itself: `RCOLLISION(1, 1)` found `COLLISION` at its + * second character, read the `(1,` that followed as a handler argument, + * and refused the whole program with "branch to line 1, which the program + * did not number" -- naming a line that contains no branch at all. + * + * The comment below is right that the trailing check is what protects + * `GOTOX#`. Nothing protected `XGOTO#`, and `RCOLLISION` is the first + * name in the language to reach it. + */ for ( i = 0; i < BRANCH_VERB_COUNT; i++ ) { verblen = strlen(BRANCH_VERBS[i]); + if ( p > code && isalnum((unsigned char)p[-1]) ) { + break; + } if ( strncasecmp(p, BRANCH_VERBS[i], verblen) == 0 && !isalnum((unsigned char)p[verblen]) ) { matched = true; @@ -245,7 +260,8 @@ static akerr_ErrorContext *rewrite_line(akbasic_TargetWalk *walk, const char *co * cannot be pointed at it directly. Copy the type and the comma, then * rewrite what follows. */ - if ( strncasecmp(p, "COLLISION", 9) == 0 && !isalnum((unsigned char)p[9]) ) { + if ( strncasecmp(p, "COLLISION", 9) == 0 && !isalnum((unsigned char)p[9]) + && !(p > code && isalnum((unsigned char)p[-1])) ) { memcpy(out, p, 9); out += 9; p += 9; diff --git a/tests/unnumbered.c b/tests/unnumbered.c index 13c7161..37e5b9f 100644 --- a/tests/unnumbered.c +++ b/tests/unnumbered.c @@ -300,6 +300,41 @@ static void test_a_number_in_a_string_is_not_a_target(void) * it touches comes out numbered -- so the branch the check refused a moment ago * is now naming a number the program owns. That is the remedy the refusal names. */ +/** + * @brief A name that merely *contains* a branch verb is not a branch. + * + * The target prescan walks a line character by character looking for `GOTO`, + * `GOSUB`, `COLLISION` and the rest, and it checked only the character *after* a + * match. So `RCOLLISION(1, 1)` found `COLLISION` at its second character, read + * the `(1,` that followed as a handler line number, and refused the whole + * program with "branch to line 1, which the program did not number" -- naming a + * line containing no branch at all. + * + * `RCOLLISION` is the first name in the language to reach it. The general case + * is any identifier ending in a branch verb, which is why the second half of + * this test does not mention collision at all. + */ +static void test_verb_inside_a_name_is_not_a_branch(void) +{ + TEST_REQUIRE_OK(run_program("X# = 0\n" + "T# = RCOLLISION(1, 1)\n" + "PRINT \"RAN \" + T#\n")); + TEST_REQUIRE_STR(HARNESS_OUTPUT, "RAN 0\n"); + harness_stop(); + + /* And the general shape: a variable whose name ends in a branch verb. */ + TEST_REQUIRE_OK(run_program("MYGOTO# = 7\n" + "PRINT MYGOTO#\n")); + TEST_REQUIRE_STR(HARNESS_OUTPUT, "7\n"); + harness_stop(); + + /* The trailing check still works: a name *starting* with one is not a branch. */ + TEST_REQUIRE_OK(run_program("GOTOX# = 9\n" + "PRINT GOTOX#\n")); + TEST_REQUIRE_STR(HARNESS_OUTPUT, "9\n"); + harness_stop(); +} + static void test_renumber_makes_numeric_branches_legal(void) { TEST_REQUIRE_OK(harness_start(NULL)); @@ -350,6 +385,7 @@ int main(void) test_numeric_branch_to_an_empty_line_is_allowed(); test_label_targets_are_never_refused(); test_a_number_in_a_string_is_not_a_target(); + test_verb_inside_a_name_is_not_a_branch(); test_renumber_makes_numeric_branches_legal(); test_renumber_marks_every_line_numbered(); return akbasic_test_failures;