2
\$\begingroup\$

I have some code that boots up a minecraft server, more complicated then most. I'm looking for some positive criticism on how to improve my code (and coding practice!).

File:

::Copyright notice
::RandomWords.py
::By Dylan Prins
::
::This work is licensed under CC BY 4.0.
::To view a copy of this license, visit
::https://creativecommons.org/licenses/by/4.0

::bootstrap
:bootstrap
@echo off
title %sver% %sname%
SETLOCAL EnableDelayedExpansion
if exist "runfig.conf" set "fig=True"
if NOT exist "runfig.conf" set "fig=False"
echo MConsole Beta
echo.
echo Config loaded?
if %fig% == True goto confLoad
echo %fig%
ping 0 >nul
goto question

::Load the stuffs
:confLoad
echo %fig%
< runfig.conf (
  set /p fname=
  set /p name=
  set /p mem=
  set /p sname=
  set /p sver=
)
echo.
echo %fname%
echo %name%
echo %mem%
echo %sname%
echo %sver%
echo.
title %sver% %sname%
ping 0 >nul

::main menu
:question
cls
echo %sver% %sname% Main Menu
echo Type 'help' for help and type 'settings' for the settings menu
echo.
set /p "option=>"
if %option% == boot goto boot
if %option% == bootnorm goto quesbootnorm
if %option% == help set "menu=main" && goto help
if %option% == settings goto settings
if %option% == vars goto vars
if %option% == refresh cls && goto bootstrap
if %option% == exit exit
goto question

::boot modded
:boot
cls
echo.
echo Booting...
echo.
java -Xmx%mem%G -Xms%mem%G -jar %fname%.jar nogui
ping 0 >nul
pause
goto question

::Ask if booting the vanilla server is what they want
:quesbootnorm
cls
echo.
echo WARNING: Booting a modded server in vanilla mode can corrupt the world!
echo.
set /p "option=Continue? (y/n)"
if %option% == y goto bootnorm
if %option% == Y goto bootnorm
if %option% == N goto question
if %option% == n goto question
echo.
echo Invalid choice (y/n)
echo.
pause
goto quesbootnorm

:: Boot vanilla server
:bootnorm
cls
echo.
echo You have been warned!
echo Booting...
echo.
java -Xmx%mem%G -Xms500M -jar %name%.jar nogui
ping 0 >nul
pause
goto question

::set the name of the forge file
:fnameset
cls
echo What do you want to set the name of the forge file to (without the .jar)
echo It is currently "%fname%"
echo.
set /p "fname=>"
echo.
echo Name set to "%fname%"
echo Don't forget to run saveconf if you wish to save these changes!
echo.
pause
goto settings

::set the name of the vanilla boot file
:nameset
cls
echo What do you want to set the name of the regular file to (without the .jar)
echo It is currently "%name%"
echo.
set /p "name=>"
echo.
echo Name set to "%name%"
echo Don't forget to run saveconf if you wish to save these changes!
echo.
pause
goto settings

::save current name(s) to file
:saveconf
cls
echo Do you wish to save these files into config file for later?
echo.
echo (Forge server name) - %fname%
echo (Vanilla server name) - %name%
echo (Memory) - %mem%GB
echo (Server name) - %sname%
echo (Server version) - %sver%
echo.
set /p "option=Continue? (y/n)"
if %option% == y goto dosaveconf
if %option% == Y goto dosaveconf
if %option% == N goto settings
if %option% == n goto settings
echo.
echo Invalid choice (y/n)
echo.
pause
goto saveconf

::actually save the config
:dosaveconf
echo.
echo %fname%>runfig.conf && echo Saving "%fname%" to file...
echo %name%>>runfig.conf && echo Saving "%name%" to file...
echo %mem%>>runfig.conf && echo Saving "%mem%" to file...
echo %sname%>>runfig.conf && echo Saving "%sname%" to file...
echo %sver%>>runfig.conf && echo Saving "%sver%" to file...
echo.
echo Save complete!
pause
goto settings

::set the memory cap
:memory
cls
echo The current memory cap is "%mem%GB"
echo What would you like to change it to? (without the 'GB' just the number)
set /p "mem=>"
echo.
echo Memory cap changed to "%mem%GB"
echo Don't forget to run saveconf if you wish to save these changes!
echo.
pause
goto settings

::Variables list
:vars
echo.
echo %mem%GB
echo %fname%.jar
echo %name%.jar
echo %sname%
echo %sver%
echo.
pause
goto question

::self explanitory
:help
cls
echo.
echo Help menu
echo.
echo Main
echo.
echo boot - Reboot modded server
echo bootnorm - Reboot normal server
echo help - Show this list
echo exit - Exit
echo.
echo Settings
echo.
echo memory - Set the max memory cap
echo setforgename - Set the name of the forge server file
echo setname - Set the name of the normal server file
echo saveconf - Save the current name(s) to a conf file
echo sname - Set the server name
echo sver - Set the server version
echo.
echo Debug (main)
echo.
echo vars - List all current loaded variables
echo refresh - Start the console again from bootstrap
echo.
pause
if %menu% == main goto question
if %menu% == set goto settings
echo.
echo Error code 1; value "menu" is "%menu%"
echo.
pause
goto question

::Set server name
:sname
cls 
echo The current server name is "%sname%"
echo What would you like to change it to?
set /p "sname=>"
echo.
echo Server name changed to "%sname%"
echo Don't forget to run saveconf if you wish to save these changes!
echo.
pause
goto settings

::Set server version
:sver
cls 
echo The current server version is "%sver%"
echo What would you like to change it to?
set /p "sver=>"
echo.
echo Server version changed to "%sver%"
echo Don't forget to run saveconf if you wish to save these changes!
echo.
pause
goto settings

:settings
cls
echo %sver% %sname% Settings Menu
echo Type 'help' for help
echo.
set /p "option=>"
if %option% == setforgename goto fnameset
if %option% == setname goto nameset
if %option% == saveconf goto saveconf
if %option% == memory goto memory
if %option% == help set goto help
if %option% == sname goto sname
if %option% == sver goto sver
if %option% == help set "menu=set" && goto help
if %option% == exit goto question
goto settings
```
\$\endgroup\$
5
  • \$\begingroup\$ i am working on the script, can you make a flowchart (just a simple one)? GOTO is a mess... \$\endgroup\$ May 15, 2020 at 22:48
  • \$\begingroup\$ Alright, working on a flowchart now \$\endgroup\$
    – Joyte
    May 16, 2020 at 0:43
  • \$\begingroup\$ in my answer i'm currently working on, i mostly use CALL, but sometimes GOTO is necessary. \$\endgroup\$ May 16, 2020 at 0:47
  • \$\begingroup\$ Hey, while scrolling through my code i just released in the :settings menu 'help' is listed twice. That's a mistake, and you can probably delete the one that's listed which does not change the 'menu' variable. \$\endgroup\$
    – Joyte
    May 16, 2020 at 1:22
  • \$\begingroup\$ app.lucidchart.com/invitations/accept/… This looks like it covers everything, PNG link: i.imgur.com/a/akZOhpD.png incase you dont want to make a lucidchart account \$\endgroup\$
    – Joyte
    May 16, 2020 at 1:38

1 Answer 1

2
\$\begingroup\$

I completely omitted the :HELP section by adding description to labels

:label - Description here

and FINDSTR to search the script for descriptions.

@echo off
SETLOCAL EnableDelayedExpansion EnableExtensions


REM Define variables
=:: User-defined
set "conf=runfig.conf"
set "#MAIN=boot bootnorm settings" MAIN section
set "#SETTINGS=memory setforgename setname saveconf sname sver" SETTINGS section
set "#DEBUG=vars refresh help exit" DEBUG section
=:: Initalized
(set LF=^
%=-----DO NOT REMOVE THIS LINE. Expands to nothing.-----=%
)
set ^"NL=^^^%LF%%LF%^%LF%%LF%^^" Escaped LF gets ignored, next LF always escaped


====:BOOTSTRAP
cls
title %sver% %sname%

if EXIST "%conf%" (set "fig=True") ELSE set "fig=false"
echo MConsole Beta
echo Config loaded? %fig%

if "%fig%" == "True" goto :confLoad
>nul ping 127.1
goto :MENU


====:CONFLOAD - Load the stuffs
echo %fig%
<"%conf%" (FOR %%V in (fname name mem sname sver) do (
    set/p"%%V="
    echo(!%%V!
))
title %sver% %sname%
>nul ping 127.1


====:MENU - main menu
cls
echo %sver% %sname% Main Menu
echo Type 'help' for help and type 'settings' for the settings menu

set "option="
set/p"option=>"

cls
=:: EXIT, a special case all by itself
if "!option!" == "exit" exit /b
FOR %%O in (%#MAIN% %#DEBUG%) do if "!option!" == "%%O" call :%%O

goto :MENU


:::::::::::::::::::::::::::::::::::::::::::::::::
::::MAIN
:boot - boot modded
echo Booting...

java -Xmx%mem%G -Xms%mem%G -jar %fname%.jar nogui
>nul ping 127.1

pause >nul&exit /b


:bootnorm - Boot vanilla server
echo WARNING: Booting a modded server in vanilla mode can corrupt the world!

CHOICE /C yn /M "Continue?"
if ERRORLEVEL 2 goto :MENU

echo You have been warned!
echo Booting...

java -Xmx%mem%G -Xms500M -jar %name%.jar nogui
>nul ping 127.1

pause >nul&exit /b


:settings
cls
echo %sver% %sname% Settings Menu
echo Type 'help' for help

set "option="
set/p"option=>"

cls
if "!option!" == "exit" goto :MENU
FOR %%O in (%#SETTINGS% %#DEBUG%) do if "!option!" == "%%O" call :%%O

goto :settings


:
::::SETTINGS
:memory - set the memory cap
echo The current memory cap is "%mem%GB"
echo What would you like to change it to? (without the 'GB' just the number)
set/p"mem=>"
echo Memory cap changed to "%mem%GB"
echo Don't forget to run saveconf if you wish to save these changes!
pause >nul
goto :settings


:setforgename - set the name of the forge file
echo What do you want to set the name of the forge file to (without the .jar)
echo It is currently "%fname%"

set/p"fname=>"
echo Name set to "%fname%"
echo Don't forget to run saveconf if you wish to save these changes!

pause >nul&exit /b


:setname - set the name of the vanilla boot file
echo What do you want to set the name of the regular file to (without the .jar)
echo It is currently "%name%"

set/p"name=>"
echo Name set to "%name%"
echo Don't forget to run saveconf if you wish to save these changes!

pause >nul&exit /b


:saveconf - save current name(s) to file
=:: Prompt for user
echo Do you wish to save these files into config file for later?%NL%

(Forge server name) - %fname%%NL%
(Vanilla server name) - %name%%NL%
(Memory) - %mem%GB%NL%
(Server name) - %sname%%NL%
(Server version) - %sver%

CHOICE /C yn /M "Continue?"
if %ERRORLEVEL% NEQ 1 goto :settings

=:: Really save the config
>"%conf%" (FOR %%V in (fname name mem sname sver) do (
echo(!%%V!
>&3 echo Saving "%fname%" to file...
))
echo Save complete!

pause >nul&exit /b


:sname - Set server name
:sver - Set server version
set "var=%0"
set "var=!var:~1!"

echo The current server name is "!%var%!"
echo What would you like to change it to?
set/p"%var%=>"
echo Server name changed to "!%var%!"
echo Don't forget to run saveconf if you wish to save these changes!

pause >nul&exit /b



:
::::DEBUG
:vars - Variables list
echo %mem%GB%NL%
  %fname%.jar%NL%
  %name%.jar%NL%
  %sname%%NL%
  %sver%

pause >nul&exit /b


:refresh - Start the console again from bootstrap
goto :BOOTSTRAP


:help - Show this list
echo Help menu
findstr /R "^:" "%~f0"

pause >nul&cls&exit /b


:exit - Exit
exit /b

Rules:

1. NEVER trust user input. Use !DELAYED EXPANSION!

  1. Use CALL instead of GOTO
  2. Use FOR loops to shorten repetitive commands
  3. Use CHOICE instead of SET /P
  4. When you want to ECHO a multi-line string, the "normal" way to do it is:

    echo foo
    echo bar
    echo foobar
    

    but it can be very slow, depending on how many ECHOs there are.
    A faster and more readable way is to create a line continuation character yourself (\n + ^):

    (set LF=^
    %=-----DO NOT REMOVE THIS LINE. Expands to nothing.-----=%
    )
    set ^"NL=^^^%LF%%LF%^%LF%%LF%^^"
    echo foo%NL%
    bar%NL%
    foobar
    
  5. Always use full paths (use variables like %__CD__%/%__APPDIR__%), not relative paths for external commands, but i didn't do it here

\$\endgroup\$
1
  • \$\begingroup\$ I don't really know the proper etiquette on what to do once you've gotten a code review, but tysm! \$\endgroup\$
    – Joyte
    May 16, 2020 at 16:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.