patterncsharpMinor
Do you want to be a super (voxel) hero?
Viewed 0 times
wantyousuperherovoxel
Problem
I have implemented an upgrades system in my Unity3d game, and I am pretty sure that I am not doing things in an optimal way. Any advice regarding best practices would be much appreciated.
The idea is that when the player collects a powerup, their run speed and jump speed increases. Collect enough powerups, and you can jump around the level like the incredible hulk.
Play the game here: Super Voxel
The powerup is a basic Sphere object with its Sphere Collider
SphereTriggerScript.cs
In the scene I have a basic Player1 prefab with this script attached:
PlayerScript.cs
The powerup references this script with
I have modified the default
Finally, I am doing something a bit funny to position the powerup orbs properly. When creating them, I use a RayCast to position them at the "top" of the
The idea is that when the player collects a powerup, their run speed and jump speed increases. Collect enough powerups, and you can jump around the level like the incredible hulk.
Play the game here: Super Voxel
The powerup is a basic Sphere object with its Sphere Collider
Is Trigger checkbox activated. It has this script attached:SphereTriggerScript.cs
public class SphereTriggerScript : MonoBehaviour {
public GameObject sphereObject;
private GameObject playerObject;
void Start() {
playerObject = GameObject.Find("Player1");
}
void OnTriggerEnter (Collider other) {
Destroy(sphereObject);
PlayerScript script = playerObject.GetComponent();
script.jumpForce += 1;
script.runSpeed += 1;
}
}In the scene I have a basic Player1 prefab with this script attached:
PlayerScript.cs
public class PlayerScript : MonoBehaviour {
public int runSpeed;
public int jumpForce;
void Start () {
runSpeed = 20;
jumpForce = 10;
}
}The powerup references this script with
GameObject.Find. I do understand that this is suboptimal, however the powerups are created programmatically at run time, so it is not possible to simply drag and drop the Player1 game object onto the powerup prefab.I have modified the default
FPSController prefab (specifically the FirstPersonController script) to add a public GameObject field that I populate with the Player1 prefab. Then when I need to get the jump or run speed, I access that script like so:PlayerScript script = playerObject.GetComponent();
m_MoveDir.y = script.jumpForce;Finally, I am doing something a bit funny to position the powerup orbs properly. When creating them, I use a RayCast to position them at the "top" of the
Solution
In your
This isn't necessary. Rather, you can do something like this:
This will also mean that the variables
In addition, in your
In your
This ensures that in case any other
The
Compare:
To:
Other than the above, I just have a few small nitpicks:
-
The general style for storing an instantiated
-
In addition, C# braces should be placed on the next line, not like Java braces, as seen in the below example.
-
Finally, you have a few magic numbers scattered across your code, so here's a small list of places containing magic numbers:
-
-
-
-
Other than that, your code looks pretty nice! I'm excited to see where this game goes!
PlayerScript class, you have two variables, runSpeed, and jumpForce, and you initialize them in Start, like this:void Start () {
runSpeed = 20;
jumpForce = 10;
}This isn't necessary. Rather, you can do something like this:
public class PlayerScript : MonoBehaviour {
public int runSpeed = 20;
public int jumpForce = 10;
}This will also mean that the variables
runSpeed and jumpForce will have the default values of 20 and 10 in the Unity Editor as well.In addition, in your
SphereTriggerScript class, you also don't need to initialize playerObject in Start as well.In your
SphereTriggerScript class, in the method OnTriggerEnter, you never do a check to make sure that the collider is actually the Player GameObject. In order to do this, you'll have to wrap the contents of SphereTriggerScript.OnTriggerEnter in this if statement:if(other.gameObject.name == playerObject.name)
{
...
}This ensures that in case any other
GameObject happens to collide with a power-up, that it won't be destroyed.The
UnityEngine namespace comes with a static class Random, which I prefer over System.Random while using Unity3D. Rather than having to instantiate System.Random, I can just call the methods directly from UnityEngine.Random.Compare:
System.Random random = new System.Random();
int randomNumber = random.Next(0, 100);To:
int randomNumber = (int)Random.Range(0, 100);Other than the above, I just have a few small nitpicks:
-
The general style for storing an instantiated
GameObject is the below example, not containing the Gameobject. prefix, and using as for a cast instead.GameObject gameObject = Instantiate( ... ) as GameObject;-
In addition, C# braces should be placed on the next line, not like Java braces, as seen in the below example.
if( ... )
{
...
}-
Finally, you have a few magic numbers scattered across your code, so here's a small list of places containing magic numbers:
-
script.jumpForce += 1;-
script.runSpeed += 1;-
int randomNumber = random.Next(0, 100);-
int randomHeightOffset = random.Next(2, 12);Other than that, your code looks pretty nice! I'm excited to see where this game goes!
Code Snippets
void Start () {
runSpeed = 20;
jumpForce = 10;
}public class PlayerScript : MonoBehaviour {
public int runSpeed = 20;
public int jumpForce = 10;
}if(other.gameObject.name == playerObject.name)
{
...
}System.Random random = new System.Random();
int randomNumber = random.Next(0, 100);int randomNumber = (int)Random.Range(0, 100);Context
StackExchange Code Review Q#101324, answer score: 6
Revisions (0)
No revisions yet.