[ThorAndOdin]

Hey, could anyone help me optimize this code i have written or make it more effective in less code if possible?

Code

@echo off
title Login
:login
cls
echo======
echo  Login
echo======
set /p username="Username: "
set /p password="Password: "
IF %username% EQU thorandodin  IF %password% EQU thorandodin goto :menu
IF %username% NEQ thorandodin  IF %password% NEQ thorandodin goto :error
cls

Obviously i can write an else command to take the user to :error if the password is wrong but any other ideas? thanks in advance. 👍

[Nakamoto.]

Password scripts in batch are rather insecure unless the password is hidden in another file, try hiding the password in a png file and check it with a for loop. I would post some code to help out but I am on my phone right now.

[ThorAndOdin]

Password scripts in batch are rather insecure unless the password is hidden in another file, try hiding the password in a png file and check it with a for loop. I would post some code to help out but I am on my phone right now.

Thanks, i was think about doing something like this... will try and figure out how to code this i guess...

['Palladium]

For god sakes please post your code between code tags. Here it is, and follow Ren's advice.

Code

@ECHO OFF
TITLE LOGIN
:login
cls
echo ======
echo Login
echo ======
SET /P username=Username?:
CLS
SET /P password=Password?:
IF %username% == thorandodin (
GOTO MENU
) ELSE (
GOTO ERROR
)

:ERROR
CLS
ECHO Incorrect Username.
PAUSE >NUL & GOTO login

:MENU
CLS
ECHO Menu
PAUSE >NUL

[ThorAndOdin]

For god sakes please post your code between code tags. Here it is, and follow Ren's advice.

Code

@ECHO OFF
TITLE LOGIN
:login
cls
echo ======
echo Login
echo ======
SET /P username=Username?:
CLS
SET /P password=Password?:
IF %username% == thorandodin (
GOTO MENU
) ELSE (
GOTO ERROR
)

:ERROR
CLS
ECHO Incorrect Username.
PAUSE >NUL & GOTO login

:MENU
CLS
ECHO Menu
PAUSE >NUL

fixed, and thanks for the code 🤝

[ThorAndOdin]

Here is a cat version:

Code

@echo off
title Login
echo======
echo  Login
echo======
set /p name="Username: "
set /p pass="Password: "
if "%name%" == "thorandodin" if "%password%" == "thorandodin" goto :menu
goto :error

A few notes below.

Spoiler (Click to View)1.
I didn't see any mention of "goto :login" anywhere, so I removed the ":login" label and the "cls" just beneath it. If these are required by some other part of the code, feel free to put them back in.

2.
Next... I changed the variable names to %name% and %pass%. It's not a great idea to use %username% as a variable name, because %username% is actually an environment variable. An environment variable is a special type of variable that is defined by Windows. For example, if you open a command prompt and type echo %username% you'll see that the variable has already been set by Windows. If you create a new variable in batch called %username%, then the batch file will ignore the environment variable from then on. That's not always good, because later on in your script you might find it useful to know the name of the logged on user.. So as a best practice, I normally avoid overwriting environment variables. Also, I changed %password% to %pass% to make it 4 characters as well, because short/consistent-length variable names make for neater code. :>

3.
The "if" statements, I've added speech marks around both sides. If you do not add speech marks, then if the user does not enter anything for either %name% or %pass% then the script will have an error. That's because it's going to collapse this code:

Code

if %name% == thorandodin

to this:

Code

if  == thorandodin

That creates an error, because batch things you didn't specify anything on the left side of the comparison.

Using speech marks fixes it:

Code

if "%name%" == "thorandodin"[

In the event of a blank entry, it becomes this:

Code

if "" == "thorandodin"

and that is perfectly valid and won't cause an error. :>

4.
Finally, I removed the 2nd line of ifs because if the first line doesn't trigger it, it should just be an error.
If we examine the logic of your original statement you can see where this could lead to erroneous behaviour:

Code

IF %username% EQU thorandodin  IF %password% EQU thorandodin goto :menu
IF %username% NEQ thorandodin  IF %password% NEQ thorandodin goto :error

Okay, a step at a time.

Code

IF %username% EQU thorandodin

Suppose I enter thorandodin for the username

Code

IF %password% EQU thorandodin goto :menu

Suppose I enter cats for the password.
I don't goto menu because I got the password wrong, so instead we just proceed to next line:

Code

IF %username% NEQ thorandodin

Uh oh, username IS thorandodin, so this won't trigger!

Therefore, this bit:

Code

IF %password% NEQ thorandodin goto :error

will never run.
Instead, we're just going to go straight to the next line, instead of going to the error.

In your version, we only go to the error if the username and password are BOTH wrong.
In the version at the top of my post, if the username and password are not correct, it always goes to :error.

Hope this helps. :>

::purr::

Thanks man! this will help a lot can i contact you about something i could use your help with?

[ThorAndOdin]

Here is a cat version:

Code

@echo off
title Login
echo======
echo  Login
echo======
set /p name="Username: "
set /p pass="Password: "
if "%name%" == "thorandodin" if "%password%" == "thorandodin" goto :menu
goto :error

A few notes below.

Spoiler (Click to View)1.
I didn't see any mention of "goto :login" anywhere, so I removed the ":login" label and the "cls" just beneath it. If these are required by some other part of the code, feel free to put them back in.

2.
Next... I changed the variable names to %name% and %pass%. It's not a great idea to use %username% as a variable name, because %username% is actually an environment variable. An environment variable is a special type of variable that is defined by Windows. For example, if you open a command prompt and type echo %username% you'll see that the variable has already been set by Windows. If you create a new variable in batch called %username%, then the batch file will ignore the environment variable from then on. That's not always good, because later on in your script you might find it useful to know the name of the logged on user.. So as a best practice, I normally avoid overwriting environment variables. Also, I changed %password% to %pass% to make it 4 characters as well, because short/consistent-length variable names make for neater code. :>

3.
The "if" statements, I've added speech marks around both sides. If you do not add speech marks, then if the user does not enter anything for either %name% or %pass% then the script will have an error. That's because it's going to collapse this code:

Code

if %name% == thorandodin

to this:

Code

if  == thorandodin

That creates an error, because batch things you didn't specify anything on the left side of the comparison.

Using speech marks fixes it:

Code

if "%name%" == "thorandodin"[

In the event of a blank entry, it becomes this:

Code

if "" == "thorandodin"

and that is perfectly valid and won't cause an error. :>

4.
Finally, I removed the 2nd line of ifs because if the first line doesn't trigger it, it should just be an error.
If we examine the logic of your original statement you can see where this could lead to erroneous behaviour:

Code

IF %username% EQU thorandodin  IF %password% EQU thorandodin goto :menu
IF %username% NEQ thorandodin  IF %password% NEQ thorandodin goto :error

Okay, a step at a time.

Code

IF %username% EQU thorandodin

Suppose I enter thorandodin for the username

Code

IF %password% EQU thorandodin goto :menu

Suppose I enter cats for the password.
I don't goto menu because I got the password wrong, so instead we just proceed to next line:

Code

IF %username% NEQ thorandodin

Uh oh, username IS thorandodin, so this won't trigger!

Therefore, this bit:

Code

IF %password% NEQ thorandodin goto :error

will never run.
Instead, we're just going to go straight to the next line, instead of going to the error.

In your version, we only go to the error if the username and password are BOTH wrong.
In the version at the top of my post, if the username and password are not correct, it always goes to :error.

Hope this helps. :>

::purr::

Thanks man! this will help a lot can i contact you about something i could use your help with?

[annikk.exe]

Only if it's something deeply personal between us two. :> Otherwise, I'd prefer if we talk about it on the open forum. That way everyone can benefit from our discussions. :>

[ThorAndOdin]

Only if it's something deeply personal between us two. :> Otherwise, I'd prefer if we talk about it on the open forum. That way everyone can benefit from our discussions. :>

its to do with a personal project that im not sure if it will be publicly released yet, ❓

[White Light]

its to do with a personal project that im not sure if it will be publicly released yet, ❓

Even still, the best way to get support is through open channels where you're not restricting it to one person helping. In cases where a project has to be protected, post as much as you can without revealing the whole picture, and then explain exactly what you feel is wrong or you need help with. There's quite a few members here who are more than smart enough to handle just about any problem that can arise, and they'll probably be able to figure out what the project is with some educated guessing, but we're all good enough to keep secrets secret.

['Palladium]

Thanks man! this will help a lot can i contact you about something i could use your help with?

Yeah sure, no problem :) Just helping out noobs like you (no offence).

[WOLGA]

Yeah sure, no problem :) Just helping out noobs like you (no offence).

I'm not sure he was talking to you (no offense).

[White Light]

I'm not sure he was talking to you (no offense).

And I'm not sure he is qualified to call anyone else a noob ( ).

['Palladium]

I'm not sure he was talking to you (no offense).

He was before because I helped him optimize his code.

fixed, and thanks for the code 🤝

[WOLGA]

He was before because I helped him optimize his code.

Oh, is that why he was quoting annikke?