Stop.sh with delay time doesn't work (continuous loop)
da99Beast opened this issue · 3 comments
The part of the stop.sh script responsible for delaying the stop doesn't seem to loop properly as it never decreases the CountdownTime parameter. In my tests I hard coded the CountdownTime to be 1 and it ran through the while and the first if, then left the if statement and presented the "Waiting for 1 more minutes..." then looped through the first if again. It never left the while loop.
# Stop the server
while [ $CountdownTime -gt 0 ]; do
if [ $CountdownTime -eq 1 ]; then
screen -Rd minecraft -X stuff "say Stopping server in 60 seconds...$(printf '\r')"
sleep 30;
screen -Rd minecraft -X stuff "say Stopping server in 30 seconds...$(printf '\r')"
sleep 20;
screen -Rd minecraft -X stuff "say Stopping server in 10 seconds...$(printf '\r')"
sleep 10;
else
screen -Rd minecraft -X stuff "say Stopping server in $CountdownTime minutes...$(printf '\r')"
sleep 60;
fi
echo "Waiting for $CountdownTime more minutes ..."
done
echo "Stopping Minecraft server ..."
screen -Rd minecraft -X stuff "say Stopping server (stop.sh called)...$(printf '\r')"
screen -Rd minecraft -X stuff "stop$(printf '\r')"
Secondary - what is the proper command line format for the parameter? I couldn't get it to work.
Found the issues and resolved them . Here is what I did to resolve:
To fix the command line options issue:
change line 22 from "while getopts ":t" opt; do
" to "while getopts "t:" opt; do
" because the ":" before the "t" means no argument while the ":" after the "t" means there is an argument
change line 25 from " case $string in
" to " case $OPTARG in
" because the argument being evaluated in the case statement is not in $string
but is in $OPTARG
.
To fix the countdown loop issue (in this order as the line numbers change):
Remove line 54 ( echo "Waiting for $CountdownTime more minutes ..."
) as it is not needed
Insert line below line 52 ( sleep 60;
) that reads " ((--CountdownTime))
" to decrease the variable by 1
Insert line below line 49 ( sleep 10;
) that reads " ((--CountdownTime))
" to decrease the variable by 1
This will make the following commands work:
stop.sh -t 1
(waits for 60 seconds and stops)
stop.sh -t 2
(waits for 120 seconds and stops)
etc.
Found the issues and resolved them . Here is what I did to resolve:
To fix the command line options issue:
change line 22 from "
while getopts ":t" opt; do
" to "while getopts "t:" opt; do
" because the ":" before the "t" means no argument while the ":" after the "t" means there is an argument
This is incorrect. Placing the colon after the t makes the argument required and will fail loudly by the shell. Leaving it before allows the option to be optional and fail silently to be manually checked, like it currently is.
To fix the countdown loop issue (in this order as the line numbers change):
Remove line 54 (echo "Waiting for $CountdownTime more minutes ..."
) as it is not needed
This is to help with logging as it'll be captured by journalctl.
Everything else is fine. Suggest you write these changes up and issue a pull request to update accordingly.
Hmm, I actually don't even recognize this countdown code oddly enough. I think this may have snuck through in a pull request with something else or I didn't test it nearly closely enough.
I'm actually getting the same behavior as you where it won't work without the colon switched like you said. It actually looks like you are both right. It needed to look like this:
while getopts ":t:" opt; do
I found this here for an explanation: https://unix.stackexchange.com/questions/426483/what-is-the-purpose-of-the-very-first-character-of-the-option-string-of-getopts
Thank you to both of you. I've committed this fix and da99Beast's other fixes to GitHub!