[milanosie]

So, I am using the EXACT same way for /enter
But with /refuel it tells me im not near a gas station

pawn Код:

CMD:refuel(playerid, params[])
{
    if (IsPlayerInAnyVehicle(playerid))
    {
        if (isrefuelling[playerid] == 0)
        {
            for(new h = 0; h < sizeof(BizzInfo); h++)
            {
                format(file6, sizeof(file6), "realityrp/bizzes/%d.ini", h);
                BizzInfo[h][benx] = dini_Float(file6, "benx");
                BizzInfo[h][beny] = dini_Float(file6, "beny");
                BizzInfo[h][benz] = dini_Float(file6, "benz");
                if(IsPlayerInRangeOfPoint(playerid, 15, dini_Float(file6, "benx"), dini_Float(file6, "beny"), dini_Float(file6, "benz")))
                {
                    if(BizzInfo[h][bprods] >= 5)
                    {
                        if(GetPlayerMoney(playerid) >= BizzInfo[h][bfee])
                        {
                            if(BizzInfo[h][btype] == 7)
                            {
                            GivePlayerMinusCash(playerid,BizzInfo[h][bfee]); 
                            SetCameraBehindPlayer(playerid); 
                            TogglePlayerControllable(playerid,0);refuel at the same time
                            isrefuelling[playerid] = 1; /refuel
                            TextDrawSetString(td_fuel[playerid],"Refuelling..."); /refuel
                            SetTimerEx("timer_refuel",4500,false,"i",playerid); 
                            BizzInfo[h][bprods] = BizzInfo[h][bprods] - 5;
                            BizzInfo[h][bbank] = BizzInfo[h][bbank] + BizzInfo[h][bfee];
                            BizzInfo[h][customers] = BizzInfo[h][customers] + 1;
                            }
                            else return SendClientMessage(playerid, COLOR_GREY, "You are not at a Gass Station!");
                        }
                        else return SendClientMessage(playerid, COLOR_GREY, "Not enough money!");
                    }
                    else return SendClientMessage(playerid, COLOR_GREY, "Gas station is empty! A petrol trucker is needed to refill!");
                }
                else return SendClientMessage(playerid, COLOR_GREY, "You are not at a gas station!");
            }
            return 1;
        }
        else return SendClientMessage(playerid, COLOR_GREY, "You are already refueling!");
    }
    else return SendClientMessage(playerid, COLOR_GREY, "You must be in a vehicle to do this!");
}

[MP2]

How unclear is that?

pawn Код:

CMD:refuel(playerid, params[])
{
    if(!IsPlayerInAnyVehicle(playerid)) return SendClientMessage(playerid, COLOR_GREY, "You must be in a vehicle to do this!");
    if(isrefuelling[playerid]) return SendClientMessage(playerid, COLOR_GREY, "You are already refueling!");
    for(new h = 0; h < sizeof(BizzInfo); h++)
    {
        format(file6, sizeof(file6), "realityrp/bizzes/%d.ini", h);

        // Not sure what the following is for, so disabled. See the InRangeOfPoint check
        //BizzInfo[h][benx] = dini_Float(file6, "benx");
        //BizzInfo[h][beny] = dini_Float(file6, "beny");
        //BizzInfo[h][benz] = dini_Float(file6, "benz");

        if(!IsPlayerInRangeOfPoint(playerid, 15, BizzInfo[h][benx], BizzInfo[h][beny], BizzInfo[h][benz]) || BizzInfo[h][btype] != 7) continue;
        if(BizzInfo[h][bprods] < 5) return SendClientMessage(playerid, COLOR_GREY, "Gas station is empty! A petrol trucker is needed to refill!");
        if(GetPlayerMoney(playerid) < BizzInfo[h][bfee]) return SendClientMessage(playerid, COLOR_GREY, "Not enough money!");
        GivePlayerMinusCash(playerid, BizzInfo[h][bfee]); 
        SetCameraBehindPlayer(playerid); 
        TogglePlayerControllable(playerid, false);
        isrefuelling[playerid] = 1;
        TextDrawSetString(td_fuel[playerid], "Refuelling...");
        SetTimerEx("timer_refuel", 4500, false, "i", playerid); 
        BizzInfo[h][bprods] -= 5;
        BizzInfo[h][bbank] += BizzInfo[h][bfee];
        BizzInfo[h][customers]++;
        return 1;
    }
    SendClientMessage(playerid, COLOR_GREY, "You are not at a gas station!");
    return 1;
}

Not tested. Let me know if there's a problem.
Look how cleaner it looks, without 50 else's.

[milanosie]

yeah it looks cleaner in a way, But I like to make things symetrical for unknown reasons.. It just looks more managed to me then, anyway. thanks
I will test it asap

[MP2]

I disagree. Look at your initial code. It's hard to tell which else is paired with which if statement. It's LOTS clearer this way.

pawn Код:

if(condition) return stop
// code

rather than

if(condition)
{
    // code
}
else
{
    //stop
}

[milanosie]

tested:

1. It does refuel now
2. It still says I am not near a gas station, but it does execute the refueling

[ricardo178]

It is funny how i don't understand anything with the "clean" look you made it.. I agree with Milanosie.
It is easy to see what else is paired with each if, cos they are symetrict... Or just look at the if, and you will know what else is relater to it.

Btw it looks like you are returning this message:

pawn Код:

SendClientMessage(playerid, COLOR_GREY, "You are not at a gas station!");

To this "for":

pawn Код:

for(new h = 0; h < sizeof(BizzInfo); h++)

[milanosie]

obvious, thanks for pointing that out

[MP2]

It just makes it a lot clearer. I read it like this:

command /refuel
if they aren't in a vehicle, show an error
if already refueling, show error
loop through the gas stations
if they aren't near this gas station or this buisness isn't a gas station, continue to the next buisness ID
didn't continue, this is a gas station, refuel here and return 1;
end of loop
refueling failed, they are not near a gas station, show message

It just seems strange to have the if() on one line and the statement 50 lines away.

[Vince]

Quote:

Originally Posted by MP2 It just seems strange to have the if() on one line and the statement 50 lines away.

I completely agree. I prefer to handle cases that would turn out to logically false first. One hundred nested if statements with 100 lines of code in between don't make the code anymore clear. You might want to read the part about 'no early returns' on the website I linked to in my signature; how to write unmaintainable code.

[ricardo178]

Quote:

Originally Posted by Vince I completely agree. I prefer to handle cases that would turn out to logically false first. One hundred nested if statements with 100 lines of code in between don't make the code anymore clear. You might want to read the part about 'no early returns' on the website I linked to in my signature; how to write unmaintainable code.

As far as the command should most of the times execute, and not return error, it is better to return the true first, and the false after.
Like, you can type 100 times /vr, and only 20 of them should return error, like You are not admin.

Guess you uderstend what i mean. Most of the times, the player fit all the ifs, making them return positiveinstead of error.