Switch problem

Discussion forum for all Windows batch related topics.

Moderator: DosItHelp

Message
Author
Adrianvdh
Posts: 177
Joined: 16 May 2013 13:00

Switch problem

#1 Post by Adrianvdh » 03 Oct 2013 08:09

Hi everyone, I am having difficulty try to see where my syntax error is:

Code: Select all

:checkPingLayersReg "switch"
if "%~1"=="d" set pinglayersnumber=2
if "%~1"=="dm" set pinglayersmax=10& set pinglayersmin=1
if "%~1"=="r" (
reg query "%RegKey%" /v "PING_PLAYERS" >nul 2>nul
if "%errorlevel%"=="0" for /f "tokens=2*" %%i in ('reg query "%RegKey%" /v "PING_PLAYERS"') do set "pinglayersnumber=%%j"
call :checkPingLayersReg "dm"
set /a "numeric=pinglayersnumber" 2>nul && (
  if "!numeric!"=="!pinglayersnumber!" (
    if !numeric! geq %pinglayersmin% (
      if !numeric! leq %pinglayersmax% (
        exit /b
      ) else (
        call :checkPingLayersRegsave
      )
    ) else (
      call :checkPingLayersRegsave
    )
  ) else (
    call :checkPingLayersRegsave
  )
) || (
  call :checkPingLayersRegsave
)
:checkPingLayersRegsave
call :checkPingLayersReg "d"
>nul reg add "%RegKey%" /v "PING_PLAYERS" /t REG_SZ /d "%pinglayersnumber%" /f )
exit /b


I have no idea what I am doing wrong. If you can please help me :)

Thanks

penpen
Expert
Posts: 2009
Joined: 23 Jun 2013 06:15
Location: Germany

Re: Switch problem

#2 Post by penpen » 03 Oct 2013 09:21

I assume a ( is missing in the procedure :checkPingLayersRegsave:
Code wrote:>nul (reg add "%RegKey%" /v "PING_PLAYERS" /t REG_SZ /d "%pinglayersnumber%" /f )


penpen

Edit this is wrong as i've overseen the first opening (.
Last edited by penpen on 03 Oct 2013 10:32, edited 1 time in total.

aGerman
Expert
Posts: 4743
Joined: 22 Jan 2010 18:01
Location: Germany

Re: Switch problem

#3 Post by aGerman » 03 Oct 2013 09:24

No idea what your routine is good for but with a better indention you would imagine what happens.

Code: Select all

:checkPingLayersReg "switch"
if "%~1"=="d" set pinglayersnumber=2
if "%~1"=="dm" set pinglayersmax=10& set pinglayersmin=1
if "%~1"=="r" (
  reg query "%RegKey%" /v "PING_PLAYERS" >nul 2>nul
  if "%errorlevel%"=="0" for /f "tokens=2*" %%i in ('reg query "%RegKey%" /v "PING_PLAYERS"') do set "pinglayersnumber=%%j"
  call :checkPingLayersReg "dm"
  set /a "numeric=pinglayersnumber" 2>nul && (
    if "!numeric!"=="!pinglayersnumber!" (
      if !numeric! geq %pinglayersmin% (
        if !numeric! leq %pinglayersmax% (
          exit /b
        ) else (
          call :checkPingLayersRegsave
        )
      ) else (
        call :checkPingLayersRegsave
      )
    ) else (
      call :checkPingLayersRegsave
    )
  ) || (
    call :checkPingLayersRegsave
  )
  :checkPingLayersRegsave
  call :checkPingLayersReg "d"
  >nul reg add "%RegKey%" /v "PING_PLAYERS" /t REG_SZ /d "%pinglayersnumber%" /f
)
exit /b

A label inside of a block of command lines is always a bad idea ...

Regards
aGerman

Adrianvdh
Posts: 177
Joined: 16 May 2013 13:00

Re: Switch problem

#4 Post by Adrianvdh » 03 Oct 2013 09:36

Sorry, none of your code works. What should I change?

aGerman
Expert
Posts: 4743
Joined: 22 Jan 2010 18:01
Location: Germany

Re: Switch problem

#5 Post by aGerman » 03 Oct 2013 09:41

I was aware that my code doesn't work. I only tried to show you why. The closing parenthesis is always seen as soon as you call :checkPingLayersRegsave.
You have to rebuild your routine without the label :checkPingLayersRegsave inside. Outsource it to another subroutine.

Regards
aGerman

Adrianvdh
Posts: 177
Joined: 16 May 2013 13:00

Re: Switch problem

#6 Post by Adrianvdh » 03 Oct 2013 10:23

How about now?:

Code: Select all

:checkPingLayersReg "switch"
if "%~1"=="d" set pinglayersnumber=2
if "%~1"=="dm" set pinglayersmax=10& set pinglayersmin=1
if "%~1"=="r" goto checkPingLayersRegR
exit /b
:checkPingLayersRegR
reg query "%RegKey%" /v "PING_PLAYERS" >nul 2>nul
if "%errorlevel%"=="0" for /f "tokens=2*" %%i in ('reg query "%RegKey%" /v "PING_PLAYERS"') do set "pinglayersnumber=%%j"
call :checkPingLayersReg "dm"
set /a "numeric=pinglayersnumber" 2>nul && (
  if "!numeric!"=="!pinglayersnumber!" (
    if !numeric! geq %pinglayersmin% (
      if !numeric! leq %pinglayersmax% (
        exit /b
      ) else (
        call :checkPingLayersRegsave
      )
    ) else (
      call :checkPingLayersRegsave
    )
  ) else (
    call :checkPingLayersRegsave
  )
) || (
  call :checkPingLayersRegsave
)
:checkPingLayersRegsave
call :checkPingLayersReg "d"
>nul reg add "%RegKey%" /v "PING_PLAYERS" /t REG_SZ /d "%pinglayersnumber%" /f
exit /b


But it still has issues, I am afraid I can't do full debugging because my CPU is running at full speed now, I guess you could say (I am doing something that is using it)

Any luck, can you see a error?

penpen
Expert
Posts: 2009
Joined: 23 Jun 2013 06:15
Location: Germany

Re: Switch problem

#7 Post by penpen » 03 Oct 2013 10:29

I didn't notice that the procedure was defined within a code block, so to make the ) a lonely one is a good idea in such a case, because this is ignored if no opening ('s have been executed after the last call or goto.
And to avoid defining procedures within a code block is a much better idea.

We cannot see the full code, but the only problematic thing in this code sniplet, that i can see may be the variables pinglayersmin and pinglayersmax, that may be undefined if "%~1" != "dm", "d" which may lead to broken if conditions:
- "if !numeric! geq (", and
- "if !numeric! leq ("

penpen

Edit: I've added an echo in front of every reg command, and it runs without errors, if defining the two variables (pinglayersmin and pinglayersmax).

Adrianvdh
Posts: 177
Joined: 16 May 2013 13:00

Re: Switch problem

#8 Post by Adrianvdh » 03 Oct 2013 11:01

@penpen, I don't understand what your saying :(?

penpen
Expert
Posts: 2009
Joined: 23 Jun 2013 06:15
Location: Germany

Re: Switch problem

#9 Post by penpen » 03 Oct 2013 11:12

The two batch variables pinglayersmin and pinglayersmax, may be undefined.
If they are undefined, and assumed numeric is 3, then these lines:

Code: Select all

    if !numeric! geq %pinglayersmin% (
      if !numeric! leq %pinglayersmax% (
will be expanded to:

Code: Select all

    if 3 geq (
      if 3 leq (
which is a syntax error.

The replacement edit "reg" -> "echo reg" should avoid executing reg.exe, but it is echoing something, so the full code can be tested: And the result is no (further) error(, except the undefined variables).

penpen

Edit: So if the issue isn't caused by the undefined variables, then the error may caused by the reg commands or is an outside (of this code snippet) problem.

Adrianvdh
Posts: 177
Joined: 16 May 2013 13:00

Re: Switch problem

#10 Post by Adrianvdh » 03 Oct 2013 11:34

I don't think it is that. My problem could probably be the

Code: Select all

if "%~1"=="r" (
(...)
)


That whole block use to work before I added switches to it. I added switches to make my code look more need and tidy, and not having so many labels and variables lying around.

BTW That code is for just reading a registry item's data, and if the reg's item data is not 1-10 then it will change it automatically to 2
which is the default, hence

Code: Select all

call :checkPingLayersReg "d"


The switch "d" stands for default and the switch "r" is for reading the reg's item's data, etc

Adrianvdh
Posts: 177
Joined: 16 May 2013 13:00

Re: Switch problem

#11 Post by Adrianvdh » 03 Oct 2013 11:35

oh, damn. I see what you mean :)

I just need to add

Code: Select all

call :checkPingLayersReg "dm"


at the beginning of my batch file.

Adrianvdh
Posts: 177
Joined: 16 May 2013 13:00

Re: Switch problem

#12 Post by Adrianvdh » 04 Oct 2013 08:44

Quick question:

How do I make it only call a function/label once?

How can I include:

Code: Select all

call :checkPingLayersReg "d" "r"


? What do I need to add to it?

penpen
Expert
Posts: 2009
Joined: 23 Jun 2013 06:15
Location: Germany

Re: Switch problem

#13 Post by penpen » 04 Oct 2013 09:00

Adrianvdh wrote:How do I make it only call a function/label once?
I'm not sure what you mean: The command "call" always will call a function/label once.

Adrianvdh wrote:How can I include:

Code: Select all

call :checkPingLayersReg "d" "r"

? What do I need to add to it?
This may help you:

Code: Select all

:checkPingLayersReg
   setlocal
   set "d="
   set "r="
   for %%a in (%*) do (
      if "%%~a" == "d" ( set "d=true"
      ) else if "%%~a" == "r" ( set "r=true"
      ) else rem additional
   )

   if defined d ( rem d
   ) else ( rem !d
   )

   if defined r ( rem r
   ) else ( rem !r
   )

::   func code
   endlocal
   goto :eof
If you only need to do something once, then you don't need to create such variables and can do it within the for loop, within the related if.

penpen

Adrianvdh
Posts: 177
Joined: 16 May 2013 13:00

Re: Switch problem

#14 Post by Adrianvdh » 04 Oct 2013 09:14

Cool, but could you tell me what selocal mean? and enlocal?

penpen
Expert
Posts: 2009
Joined: 23 Jun 2013 06:15
Location: Germany

Re: Switch problem

#15 Post by penpen » 04 Oct 2013 09:35

It creates a new block for variables copying the old existing one.
So if you change variables within an setlocal endlocal "block", they are not changed outer this "block":

Code: Select all

set "Test=Test"
setlocal
set "Test=Oh no..."
echo %Test%
endlocal
echo %Test%
For further information type "help setlocal" and "help endlocal" to the dos shell.

penpen

Post Reply