Feedback On My Coding - Best Practices

Scripting
1

Hey guys, to be honest, this is my first real attempt at coding and Game Development and I just wanted some feedback on my coding practices and if you guys think there is a more human readable or efficient way to do this script

On a side note this is literally my first week of coding so I won’t be hurt if it’s absolutely horrific

#pragma strict
// Other Scripts
var Inventory : Inventory;
// Soil State
var IsTilled : boolean = false;
var IsHarvested : boolean = false;
var IsWatered : boolean = false;
var IsPlanted : boolean = false;
// Interaction
var HasCollided : boolean = false;

// Stats
var WaterLevel : int = 0;
var WaterCapacity : int = 100;
// Plants And Seeds
// Objects
//Wheat
var RealWheat : GameObject;
var FakeWheat : GameObject;

// Pumpkin
var RealPumpkin : GameObject;
var FakePumpkin : GameObject;

// Flower
var RealFlower : GameObject;
var FakeFlower : GameObject;

// Cucumber
var RealCucumber : GameObject;
var FakeCucumber : GameObject;
// Seeds
// Wheat
var WheatIsPlanted : boolean = false;
// Pumpkin
var PumpkinIsPlanted : boolean = false;
// Flower
var FlowerIsPlanted : boolean = false;
// Cucumber
var CucumberIsPlanted : boolean = false;

function Start () {

}

function Update () {

}

function OnCollisionEnter ( col : Collision ) {
if ( col.gameObject.name == “Player” ) {
HasCollided = true;
Debug.Log(“Entering Soil”);
}
}

function OnTriggerExit ( col : Collider ) {
if (col.gameObject.name == “Player” ) {
HasCollided = false;
Debug.Log(“Leaving Soil”);
}
}

function CreateCrop() {
if (IsPlanted == false && IsTilled == true && IsWatered == true) {
if ( WheatIsPlanted == true ) {
Instantiate(RealWheat,transform.position, Quaternion.identity);
Inventory.Seed[2].Amount -= 1;
IsPlanted = true;
WheatIsPlanted = false;
} else if ( PumpkinIsPlanted == true) {
Instantiate(RealPumpkin, transform.position, Quaternion.identity);
Inventory.Seed[1].Amount -= 1;
IsPlanted = true;
WheatIsPlanted = false;
} else if ( FlowerIsPlanted == true ) {
Inventory.Seed[0].Amount -= 1;
Instantiate(RealFlower, transform.position, Quaternion.identity);
IsPlanted = true;
FlowerIsPlanted = false;
} else if ( CucumberIsPlanted == true ) {
Inventory.Seed[3].Amount =- 1;
Instantiate(RealCucumber, transform.position, Quaternion.identity);
IsPlanted = true;
FlowerIsPlanted = false;
}
} else if (IsPlanted == true) {
PlayerInfo.HelpInfo = “Something Is already Planted here, let it grow.”;
} else if (IsTilled == false && IsWatered == false) {
PlayerInfo.HelpInfo = “You need to till and water this tile first”;
} else if (IsTilled == true && IsWatered == false) {
PlayerInfo.HelpInfo = “You need to water whis tile first.”;
} else if (IsTilled == false && IsWatered == true) {
PlayerInfo.HelpInfo = “You need to till the land first.”;
}
}

2

First feedback, use code tags: Using code tags properly - Unity Engine - Unity Discussions
Second feedback, if you’re new to coding, learn something useful by using C#. UnityScript is losing support and really isn’t used anywhere else.

1 Like
3

Random thoughts

  • Code tags
  • Use C#
  • Remove empty Start and Update methods
  • Consider wrapping wheat, pumpkin, ect into a single Crop class

I can’t actually read the CreateCrop method without code tags and indentation. But I think it could be simplified with a Crop class and a simple loop.

1 Like
4

Not sure if anyone else is as concerned about this as me, but having many “if/else” statements increases this kind of complexity which I feel in general is better to avoid wherever possible. Although with that said, having a ton of “if” statements is pretty much everyone’s first time delving into scripting of any kind, until they learn to make use of for-loops and functions to replace some stuff that they previously depended on “if” statements for.

With so many bools, it would be better to see if you can reduce them all down to “enums” if you know what that is, and control the actions taken in the CreateCrop method via enum states.

Also, none of us can comment of what you made if we don’t know what you are trying to do. From the script alone I can infer it’s a farming game of sorts, but not knowing the rules of the game makes the actual evaluation slightly difficult IMO. Not having code tags hurts it even more. (A tip on putting your script in code tags, if you are trying to type it in yourself, use 4 spaces as a replacement for tab)

And yes, C# would be preferable compared to UnityScript.

1 Like