Looking for input and feedback on my code

[Ursidae]

Hey, I am new to programming and have created few scripts. I don't really want to move forward until I know if what I do is bad form or if there will be any issues in the future if I implement more things as I do.

Any feedback is appreciated, be it positive or negative. 

 

Clientside of the job: https://pastebin.com/ab9Aeycm

Serverside of the job: https://pastebin.com/xAFm3e2L

 

Feel free to ask any question as well.

 

❤️

[introzen]

I've just checked your server-side really fast and what came to my mind was that the startjob-command was done inefficiently. I'd do it like this:

	[Command("startjob")]
        public void CMD_StartJob(Client client)
        {
            if (!client.HasData("ID"))
            {
                client.SendChatMessage("~r~Please login.");
                return;
            }
 
            if (!client.HasData("Civilian"))
                return;
 
            if (client.HasData("InColShape"))
            {
                if(client.IsInVehicle)
                {
                    client.SendChatMessage("~r~Get out of your vehicle.");
                    return;
                }
                client.ResetData("InColShape");
 
                Vehicle veh = NAPI.Vehicle.CreateVehicle(VehicleHash.Sanchez, new Vector3(client.Position.X, client.Position.Y + 10, client.Position.Z), 0, 255, 255);
                client.SetIntoVehicle(veh, -1);
 
                colShape.Delete();
                colShape.ResetData("BikeJob");
                DeleteDefaultInstance(client);
                client.SendChatMessage("Started ~y~bikejob~w~. Drive to the next objective for your reward!");
 
                CreateJobInstance();
                CreateClientJob(client);
            }
        }

Since client can't execute the command unless logged in, that should be the first thing to check. client.HasData("ID")

In most servers, clients aren't given any data at all until they login, so checking for "Civilian" or "InColShape" is inefficient, should it turn out later than the client is not even logged in. Also, no need to check for client.IsInVehicle unless the player is actually inside the ColShape, again - unefficient.

The rest will be up to someone else, I'm far to inexperienced to decide upon the different functions etc.

Edited March 28, 2019 by introzen