spotlessmind1975 / ugbasic

An isomorphic BASIC language compiler for retrocomputers
Apache License 2.0
92 stars 16 forks source link

Inconsistent IF behavior #920

Closed mzattera closed 3 months ago

mzattera commented 3 months ago

In the below program (at least when compiled for Dragon target) these two lines of code behaves differently.

360 IF _BIT_CAUGHT THEN GOTO 5000       :REM This ALWAYS goes to 5000
360 IF _BIT_CAUGHT==TRUE THEN GOTO 5000 :REM This NEVER goes to 5000

From what I understood of ugBASIC syntax they should be the same though.

(Apologies for the long code below, but I could not replicate it with shorter code, for some reason)

: REM Ignore this, got to BASIC program at the end line 360

CONST _CODE_START=&H2800
DEFINE PROGRAM START _CODE_START

SCREEN #2
DEFINE SCREEN MODE UNIQUE
DEFINE DOUBLE BUFFER OFF
DEFINE TASK COUNT 1

OPTION EXPLICIT ON
OPTION DEFAULT TYPE BYTE
DEFINE DEFAULT TYPE BYTE

:REM CONST and Variables only _BIT_CAUGHT is used

POSITIVE CONST _KEY_FORWARD=250
POSITIVE CONST _KEY_LEFT=252
POSITIVE CONST _KEY_RIGHT=253
POSITIVE CONST _MAZE_SIZE_X = 16    
POSITIVE CONST _MAZE_SIZE_Y = 15
POSITIVE CONST _START_X = _MAZE_SIZE_X-1
POSITIVE CONST _START_Y = _MAZE_SIZE_Y-1
POSITIVE CONST _MW = 0  
POSITIVE CONST _MP = 128 
POSITIVE CONST _ME = 129 
POSITIVE CONST _MR = 255 
POSITIVE CONST _MT = 254 
DIM playerX AS BYTE
DIM playerY AS BYTE
DIM playerDir AS BYTE: 
GLOBAL playerX,playerY,playerDir
DIM _BIT_CAUGHT AS BIT: _BIT_CAUGHT=FALSE
DIM _BIT_PLYFWD AS BIT: _BIT_PLYFWD=FALSE
DIM _BIT_PLYSTOP AS BIT: _BIT_PLYSTOP=TRUE
DIM _BIT_REXMOVED AS BIT: _BIT_REXMOVED=FALSE
GLOBAL "_BIT_*"
DIM rexX AS BYTE
DIM rexY AS BYTE
GLOBAL rexX, rexY
DIM rexStatus AS BYTE
GLOBAL rexStatus
DIM _L408D AS BYTE
GLOBAL _L408D
DIM maze AS BYTE(_MAZE_SIZE_X,_MAZE_SIZE_Y)
GLOBAL maze
DIM _EXIT_PATTERN AS BYTE(11) = #{22, 98,       47,     23, 24, 2,  9, 22,  67,     35, 7}
DIM nextExitPattern AS BYTE
POSITIVE CONST _BOTTOMRIGHTBLACK = 1 + 128
POSITIVE CONST _BOTTOMRIGHTWHITE = 14 + 128
POSITIVE CONST _BOTTOMLEFTBLACK = 2 + 128
POSITIVE CONST _BOTTOMLEFTWHITE = 13 + 128
POSITIVE CONST _TOPRIGHTBLACK = 4 + 128
POSITIVE CONST _TOPRIGHTWHITE = 11 + 128
POSITIVE CONST _TOPLEFTBLACK = 8 + 128
POSITIVE CONST _TOPLEFTWHITE = 7 + 128
POSITIVE CONST _BOTTOMBLACK = 3 + 128
POSITIVE CONST _BOTTOMWHITE = 12 + 128
POSITIVE CONST _TOPBLACK = 12 + 128
POSITIVE CONST _TOPWHITE = 3 + 128
POSITIVE CONST _LEFTBLACK = 10 + 128
POSITIVE CONST _LEFTWHITE = 5 + 128
POSITIVE CONST _RIGHTBLACK = 5 + 128
POSITIVE CONST _RIGHTWHITE = 10 + 128
POSITIVE CONST _ALLBLACK = 15 + 128
POSITIVE CONST _ALLWHITE = 0 + 128
POSITIVE CONST _SLASHBLACK = 6 + 128
POSITIVE CONST _SLASHWHITE = 9 + 128
POSITIVE CONST _GREEN   = 0*16
POSITIVE CONST _YELLOW  = 1*16
POSITIVE CONST _BLUE    = 2*16
POSITIVE CONST _RED     = 3*16
POSITIVE CONST _WHITE   = 4*16
POSITIVE CONST _DKGREEN = 5*16
POSITIVE CONST _MAGENTA = 6*16
POSITIVE CONST _ORANGE  = 7*16

:REM ======================================================================================================================================================

PROCEDURE gameLoop
    PRINT "LOOP ";: WAIT 500 MILLISECONDS

    _BIT_CAUGHT=FALSE

    _BIT_PLYSTOP=FALSE  
    _BIT_PLYFWD=FALSE
    IF SCANCODE==_KEY_FORWARD THEN
        _BIT_PLYFWD=TRUE
    ELSEIF SCANCODE==_KEY_LEFT THEN
        INC playerDir
        playerDir = playerDir MOD 4
    ELSEIF SCANCODE==_KEY_RIGHT THEN
        IF playerDir==0 THEN
            playerDir=3
        ELSE
            DEC playerDir
        ENDIF
    ELSE 
        _BIT_PLYSTOP=TRUE
    ENDIF

    IF maze(playerX,playerY)==_ME THEN RETURN

    IF (rexX<>playerX) OR (rexY<>playerY) THEN 
        PRINT "MOVE"
    ELSEIF (rexStatus AND 2) THEN 
        _BIT_CAUGHT=TRUE: REM Notice ths is set to true if and only if rexX=playerX, rexY=playerY, bit 1 of rexStatuis is 1. This never happens
    ENDIF

END PROC    

:REM ======================================================================================================================================================

:REM BASIC Program START

DIM t AS WORD
DIM xd,xl AS BYTE
CLS: PRINT: PRINT: PRINT
CENTER "PRESS A KEY": WAIT KEY

95 RANDOMIZE

DIM tpl AS BYTE: tpl = TICKS PER SECOND/3
DIM nextLoop AS WORD: nextLoop = TI + tpl
330 nextExitPattern = RND(128)
350 CALL gameLoop[]

360 IF _BIT_CAUGHT THEN GOTO 5000 : REM This ALWAYS goes to 5000
' 360 IF _BIT_CAUGHT==TRUE THEN GOTO 5000 : REM This NEVER goes to 5000

370 WHILE TI<nextLoop: WEND
nextLoop = nextLoop+tpl

390 IF maze(playerX,playerY)<>_ME THEN GOTO 330
PRINT "GOT EXIT": END

5000 PRINT"CAUGHT": END
spotlessmind1975 commented 3 months ago

Hi @mzattera and thank you for your bug report!

Yes, you are right: your single example found two bugs.

The first one was that expressions related to IF were not converted as signed bytes. This means that even if they returned a value of TRUE (-1), they were implicitly converted to 255 instead of -1. So expressions like ... = TRUE would never work. Those with the omission, however, would (because -1 and 255 are both "non-zero").

The second one had to do with the conversion from BIT to SIGNED BYTE which, evidently, did not work. This also explained why you were getting that erratic behavior.

The bug was fixed in COLDFIX v1.16.3 [rev. 20240805].

Then, I would like to compliment you on the choice of adding a _ prefix to the constants. In fact, the need to adapt the language for 10-liner competitions while still leaving it "non-contextual" had forced me to the rule of variables and constants that began with a lowercase character (because the commands are in uppercase). The choice of using the _ character, which is valid but is not an alphabetic character and above all is not used by any command, seems like a really fantastic idea to me!

That said, I would like to suggest a series of small improvements to the code. I don't know if it's real code, but the suggestions are valid anyway. :)

OPTION DEFAULT TYPE BYTE
DEFINE DEFAULT TYPE BYTE

The first one is an alias for the second one, so one is enough.

DIM _BIT_CAUGHT AS BIT: _BIT_CAUGHT=FALSE

In BASIC, variables are always initialized to zero, unless otherwise initialized. So, actually, the =FALSE assignment is redundant, since FALSE is 0.

IF SCANCODE==_KEY_FORWARD THEN

Even though I left the == operator for comparisons, the standard BASIC operator is =: the compiler knows when a = is used for comparison or assignment, depending on the context.

playerDir = playerDir MOD 4

The MOD operation involves a division operation, which is usually expensive in terms of the amount of code generated and the speed of execution. Since the operation is to ensure that playerDir is in the range [0...3], that is a power of two minus one, it is better to use the bitwise AND operator with a bitmask of 3. This bitmask guarantees that the value is between 0 and 3. In other words, I would write:

playerDir = playerDir AND 3

Thank you again!

mzattera commented 3 months ago

Verified fixed :-)

Also, thanks A LOT for the suggestions. ugBASIC is so feature rich that I am sure I am missing a lot of ways to write better code.

I have a couple questions about variable types stemming from:

OPTION DEFAULT TYPE BYTE
DEFINE DEFAULT TYPE BYTE

but I-'ll check documentation and forum first.