How to make an external goto not break a for loop in batch

Question

So I have a script that I'm writing as a pet project that seemed simpler than it turns out to be...I have a for loop that is meant to increment a certain amount of times based on user input but in the for loop, I have a destination statement for a goto earlier in the script. The issue is that when that goto is used it breaks the for and causes the for loop to only provide 1 answer.

Here is the code:

for /l %%x in (1, 1, %player%) do (
:defense
call :defenders
:defense_return
call :colorEcho 0e %operator%
echo.
)

The :defense_return is the culprit but it's neccesary because I'm using %RANDOM% so I need it to reference back to when the %RANDOM% is used or else I just get the same output when I really want 2 different outputs.

here is the defenders block:

:defenders
set /a operator=%random%%%18+1
REM 707th SMB
if %operator%== 1 set operator=Vigil
REM G.R.O.M
if %operator%== 2 set operator=Ela
REM S.D.U
if %operator%== 3 set operator=Lesion
REM G.E.O
if %operator%== 4 set operator=Mira
REM S.A.T
if %operator%== 5 set operator=Echo
REM BOPE
if %operator%== 6 set operator=Caviera
REM Navy Seal
if %operator%== 7 set operator=Valkyrie
REM JTF2
if %operator%== 8 set operator=Frost
REM S.A.S
if %operator%== 9 set operator=Mute
if %operator%== 10 set operator=Smoke
REM SWAT
if %operator%== 11 set operator=Castle
if %operator%== 12 set operator=Pulse
REM GIGN
if %operator%== 13 set operator=Doc
if %operator%== 14 set operator=Rook
REM GSG9
if %operator%== 15 set operator=Jager
if %operator%== 16 set operator=Bandit
REM Spetsnaz
if %operator%== 17 set operator=Tachanka
if %operator%== 18 set operator=Kapkan
goto :defense_return

I really want to make this script work out, but this for loop is causing me issues...Any help is much appreciated!

Change `goto :defense_return` to `goto :eof`. You also need to use Delayed Expansion. So this line: `call :colorEcho 0e %operator%` changes to `call :colorEcho 0e !operator!`. Then you need to enable delayed expansion with a `SETLOCAL` command. — Squashman, Mar 15 '18 at 15:30
it caused errors when using the accented "a", same with Capitao. I couldn't find a way to get the batch file to parse it correctly as well because it would always output "Jäger" as "JΣger". — Excallypurr, Mar 15 '18 at 15:30
There are two accepted ways to end a CALL to a label. You either use `GOTO :EOF` or `EXIT`. You CANNOT use a GOTO to get back into your FOR command. — Squashman, Mar 15 '18 at 15:35
do you have any suggestions on how I can accomplish what I'm trying to do if I can't do it this way? — Excallypurr, Mar 15 '18 at 15:38
The code snippet you have chosen to share with us works just fine if you make the two changes I suggested in my first comment. You need to end the call correctly and you need to use delayed expansion. — Squashman, Mar 15 '18 at 15:39
I did not see the change you suggested at `%operator%` I changed that and it is working fine! Thank you — Excallypurr, Mar 15 '18 at 15:44

score 2 · Accepted Answer · answered Mar 15 '18 at 17:29

You have a misconception here, as already explained in the comments...

The :defenders block is a subprogram, that is, a code segment that you want to call (or invoke, or execute, or whichever) and when such a code segment ends you want not to "reference back" (goto) another place, but to return to the point where such a subprogram was called. The way to return to the command that follows the call command is exit /B, although goto :EOF also works for this purpose, but it is more confusing (in most programming languages, the statement to do this is called return).

Also, when you want to use the value of a variabe that change inside a for loop, you must use the !variable! construct instead of %variable% one (and insert setlocal EnableDelayedExpansion command at beginning of the program).

Finally, your code may be improved if you use an array. This is the final version of your code after including all previously described modifications:

@echo off
setlocal EnableDelayedExpansion

rem Initialize "operator" array
set i=0
for %%a in (Vigil Ela Lesion Mira Echo Caviera Valkyrie Frost Mute
            Smoke Castle Pulse Doc Rook Jager Bandit Tachanka Kapkan) do (
   set /A i+=1
   set operator[!i!]=%%a
)

rem Get a Back-Space (ASCII 8) character (for :colorEcho routine)
for /F %%a in ('echo prompt $H ^| cmd') do set "BS=%%a"

set player=4

for /l %%x in (1, 1, %player%) do (
call :defenders
call :colorEcho 0e !operator!
echo/
)
pause
goto :EOF

:defenders
set /a operator=%random%%%18+1
set operator=!operator[%operator%]!
exit /B

:colorEcho color text
set /P "=%BS% " > "%~2" <nul
findstr /A:%1 "^" "%~2" nul
del "%~2"
exit /B

Thank you so much for your answer. To be honest, I had no clue I could use arrays in conjunction with random variables and never thought to try. This not only fixes my issue but simplifies and compacts my code, thank you so much! — Excallypurr, Mar 15 '18 at 19:33
May I ask you to upvote both this answer and the array's one? Thanks — Aacini, Mar 16 '18 at 02:27
I upvoted you Antonio. This is a great answer. I should just delete my answer as you have simplified and improved upon the code. — Squashman, Mar 16 '18 at 03:51

score 1 · Answer 2 · answered Mar 15 '18 at 16:14

As I explained above you need to exit the CALL correctly and use Delayed Expansion.

@echo off
setlocal EnableDelayedExpansion

for /F "tokens=1,2 delims=#" %%a in ('"prompt #$H#$E# & echo on & for %%b in (1) do     rem"') do (
  set "DEL=%%a"
)

for /l %%x in (1,1,2) do (
call :defenders
call :colorEcho 0E !operator!
echo.
)
pause
GOTO :EOF

:defenders
set /a operator=%random%%%18+1
REM 707th SMB
if %operator%== 1 set operator=Vigil
REM G.R.O.M
if %operator%== 2 set operator=Ela
REM S.D.U
if %operator%== 3 set operator=Lesion
REM G.E.O
if %operator%== 4 set operator=Mira
REM S.A.T
if %operator%== 5 set operator=Echo
REM BOPE
if %operator%== 6 set operator=Caviera
REM Navy Seal
if %operator%== 7 set operator=Valkyrie
REM JTF2
if %operator%== 8 set operator=Frost
REM S.A.S
if %operator%== 9 set operator=Mute
if %operator%== 10 set operator=Smoke
REM SWAT
if %operator%== 11 set operator=Castle
if %operator%== 12 set operator=Pulse
REM GIGN
if %operator%== 13 set operator=Doc
if %operator%== 14 set operator=Rook
REM GSG9
if %operator%== 15 set operator=Jager
if %operator%== 16 set operator=Bandit
REM Spetsnaz
if %operator%== 17 set operator=Tachanka
if %operator%== 18 set operator=Kapkan
goto :eof

:colorEcho
<nul set /p ".=%DEL%" > "%~2"
findstr /v /a:%1 /R "^$" "%~2" nul
del "%~2" > nul 2>&1
goto :eof

How to make an external goto not break a for loop in batch

2 Answers2

Linked