Help me with this inprofessional code!

[LIam]

I have to make a small program for school. It has to look like, that you are removing a tumor. I chose Batch to program it. Here is my code that I already have:

Code: Select all

@echo off
title Surgery Robot Software
MODE CON COLS=999 LINES=999
color F0
goto start

:start
echo Booting software...
TIMEOUT /T 5
cls
echo SURGERY ROBOT SOFTWARE v1.0
TIMEOUT /T 2
cls
goto B

:B
echo.
echo Would you like to do a tutorial? Answer with yes or no.
set /p answer=
if %answer%==yes goto tutorial
if %answer%==no goto startsurgery
cls
goto B

:tutorial
echo.
echo Controls:
echo Forwards:   [W]
echo Backwards:   [S]
echo Left:      [A]
echo Right:      [D]

PAUSE

echo [] = You
echo {} = Tumor

PAUSE

cls
goto startsurgery

:startsurgery
echo.
echo We will start with the surgery

PAUSE

echo NOW LOADING...

TIMEOUT /T 5

color 4E

cls

echo.
echo ____________________________________
echo |      {}         |
echo |               |
echo |               |
echo |               |
echo |               |
echo |               |
echo |               |
echo |               |
echo |               |
echo |_______________________[]_________|

set /p answer1=
if %answer1%==w goto 1
if %answer1%==a goto 2
if %answer1%==s goto 3
if %answer1%==d goto 4

:1
cls
echo ____________________________________
echo |      {}         |
echo |               |
echo |               |
echo |               |
echo |               |
echo |               |
echo |               |
echo |               |
echo |              []      |
echo |__________________________________|

Well, the problem is, that it stops running when it gets to the stupid GUI.
Can anyone tell me what the problem is?

Liam.

[dbenham]

The | (pipe) character has special meaning (pipes output of one command as input into another). Such characters are sometimes referred to as "poison" characters.

You cannot use the pipe as a string literal unless it is quoted "|" or escaped ^|, so it is sometimes referred to as a poison character. Quotes are not good because you probably don't want the quotes in the output. You could also store your string in a variable with quotes around the entire SET statement, and then safely print out the value without quotes using delayed expansion.

Code: Select all

@echo off
setlocal enableDelayedExpansion
echo "This will succeed, but has unwanted quotes |"
echo This will succeed ^|

set "string=This will succeed |"
echo(!string!
REM The left parenthesis after ECHO is a hack that gives the correct output even if string is empty or only contains spaces

echo This will fail |

There are additional poison characters, for example: < > &, and possibly )


Dave Benham

[Sounak@9434]

The first problem is obviously the | (pipe), which is maybe the most poisonous character. You have to escape them with ^ (caret), so

Code: Select all

echo |

will become

Code: Select all

echo ^|

dbenhan said about it so I'm not going into further explanation.
The second problem I see is that the way your code is built. It's not wrong, it will run fine but just how long do you want to make it?
The way you have used, for a 10x10 cell(excluding wall) gui you have to make (8x8x4)+(8x4x3)+(4x2)=360 gui entries!!
I don't know if you are using any other way but if not then here is one of the easiest way to do it.

Code: Select all

@echo off
set "x=e"
set "y=5"
::Setting all variables " "(white space)
for %%a in (a b c d e) do for /l %%b in (1,1,5) do set "%%a%%b= "
:main
cls
set "%x%%y%=O"
::Main GUI screen
echo -------
echo ^|%a1%%a2%%a3%%a4%%a5%^|
echo ^|%b1%%b2%%b3%%b4%%b5%^|
echo ^|%c1%%c2%%c3%%c4%%c5%^|
echo ^|%d1%%d2%%d3%%d4%%d5%^|
echo ^|%e1%%e2%%e3%%e4%%e5%^|
echo -------
::Getting input from user
set /p "move=>"
::Validating input
if not "%move%"=="w" if not "%move%"=="s" if not "%move%"=="a" if not "%move%"=="d" goto main
::If input is "a" lowering down "y" by 1
if %move%==a (
 rem if "y" is already at leftmost part then goto main
 if %y% equ 1 goto main
 set "%x%%y%= "
 set /a y-=1
 goto main
)
::Same process as "a" but increasing "y" by 1
if %move%==d (
 if %y% equ 5 goto main
 set "%x%%y%= "
 set /a y+=1
 goto main
)
::Increasing "x" by one here
if %move%==w (
 if "%x%"=="a" goto main
 set "%x%%y%= "
 rem As wecan't directly increase numbers we are using (if...set) statement
 if %x%==e set x=d
 if %x%==d set x=c
 if %x%==c set x=b
 if %x%==b set x=a
 goto main
)
::Decreasing "x" by 1 here
if %move%==s (
 if "%x%"=="e" goto main
 set "%x%%y%= "
 if %x%==a set x=b
 if %x%==b set x=c
 if %x%==c set x=d
 if %x%==d set x=e
 goto main
)
goto main

I wanted to use 'choice' instead of 'set /p' as it gives instant output and there is no need for input validation. But if you have windows XP(or lower) then choice is not available by default.
There are far better ways to do this (like using colous.exe) but this one is easy to understand.

//EDIT:-Fixed a few bugs and edited the code with comments to be easier to understand.

[Sounak@9434]

I fixed the code above, the last code required delayed expansion to work. This one works without delayed expansion and fixed a small bug at the start for a better gui window.