- Home /
Optimizing C# Code and physics
I'm trying to optimize the code for my simple jumping game and need some help identifying problem areas. My scene currently has 5 "simple" scripts. A player controller, a spawner, a remover, a control for the spawned objects and a camera follow script. Some or all of these scripts need to be optimized.
Here is a simple description of the game: Rotating balls spawn on the top of the screen and constantly move down and spin. The player jump up from ball to ball, rotating around the ball to aim for the next jump.
Here's the first script. PlayerControl: With this script I am checking in Update if the player has given an input, if so set jump to true, which is handled by FixedUpdate. I was wondering, would it be more efficient to just rename FixedUpdate to something like Jump(), and just call that function on input? Next I noticed some slow down whenever the player would collide with the falling objects, is there anything particularly inefficient or wrong with my code?
using UnityEngine;
using System.Collections;
public class PlayerControl : MonoBehaviour
{
public Sprite standCat;
public Sprite jumpCat;
public Sprite sitCat;
public AudioClip jumpSound;
private bool jump = false;
private bool grounded;
private bool isCollided = false;
private bool waited = false;
private int i = 0;
private SpriteRenderer myRenderer;
private Vector3 localDown = new Vector3(0f, 0f);
// Use this for initialization
void Awake(){
myRenderer = gameObject.GetComponent<SpriteRenderer>();
grounded = true;
}
// Update is called once per frame
void Update()
{
//By default player has gravity set to 0. When player's velocity slows enough, set gravity to 3.
if (this.rigidbody2D.velocity.magnitude <= 1.5f && !isCollided && this.transform.position.y > 1 && waited)
this.rigidbody2D.gravityScale = 3f;
i = 0;
while (i < Input.touchCount)
{
if (Input.GetTouch(i).phase == TouchPhase.Began && (grounded || isCollided))
{
this.collider2D.isTrigger = false;
waited = false;
this.transform.parent = null;
jump = true;
}
i++;
}
}
void FixedUpdate()
{
localDown = -(this.transform.up);
if (jump)
{
if (grounded)
{
myRenderer.sprite = jumpCat;
audio.PlayOneShot(jumpSound);
rigidbody2D.velocity = this.transform.up * 12;
jump = false;
grounded = false;
}
if (isCollided)
{
this.transform.Rotate(0, 0, 180);
myRenderer.sprite = jumpCat;
audio.PlayOneShot(jumpSound);
rigidbody2D.velocity = localDown * 12;
jump = false;
isCollided = false;
StartCoroutine("GravityWait");
}
}
}
IEnumerator GravityWait()
{
yield return new WaitForSeconds(0.75f);
collider2D.enabled = true;
waited = true;
}
//When player touches a ball, set ball to parent, change sprite, change up orientation,
// set gravity and velocity to 0 and turn Collider2D into a trigger to avoid being moved when touched.
void OnCollisionEnter2D(UnityEngine.Collision2D col)
{
if (this.transform.parent == null)
{
isCollided = true;
this.transform.parent = col.transform;
myRenderer.sprite = sitCat;
this.rigidbody2D.gravityScale = 0;
rigidbody2D.velocity = new Vector3(0, 0, 0);
isCollided = true;
var direction = (col.gameObject.transform.position - this.transform.position).normalized;
this.transform.up = direction;
this.collider2D.isTrigger = true;
}
}
//If already parented, knock away balls that come in contact with player.
void OnTriggerEnter2D(UnityEngine.Collider2D col)
{
var direction2 = (this.gameObject.transform.position - col.transform.position).normalized;
col.transform.rigidbody2D.velocity = direction2 * -10f;
}
}
I figured I'd post the other scripts after this one gets looked at. My question might get very long if I post them all. Thanks for the help!
There is absolutely no reason to avoid while loops. They're just loops; basically a different way of doing for loops that can be more convenient in some cases. (But not in this case; a for loop would make more sense here.)
Also no reason whatsoever to avoid nested ifs or having more than two conditional checks, and in fact that's logically impossible depending on what you're doing.
Coroutines only halt the main thread as long as the function is actively running, the same as any other function, so unless the code is extremely CPU-intensive, it won't have any visible effect.
Really bad advice overall, sorry.
Thanks for the replies! Unfortunately I don't know what direction to take! If I am to avoid while loops what is a better way to check for touch input?
As far as avoiding two conditional checks, I may not be able to do that in some cases.
Is any of the rigidbody stuff particularly expensive? Is checking the magnitude every frame a ridiculous thing? I was having a problem where when the player jumped he would immediately fall down randomly. I used the coroutine to make sure it waited a little bit before checking the speed and changing the gravity. Should I just remove the magnitude check and do a set time before turning up the gravity?
Answer by Okari-Draconis · Jun 21, 2014 at 09:32 PM
Like the Previous post said, Remove the " (grounded || isCollided))" check to outside the loop, so your not checking it so many times. The loop isn't too big a deal to be honest, as long as this is the only thing receiving input. Something that could help is Caching the Collider2D, the RigidBody2D and the transform. Everytime you do this.rigidbody2d or this.transform. its the same as running this.GetComponent()
I recommend storing those into variables inside the Awake function/method Also avoid using "var"
Also instead of doing magnitude <= 1.5f you should do sqrMag < 1.5f*1.5f additionally, you can optimize that if statement.
if (waited && !isCollided && this.transform.position.y > 1 && this.rigidbody2D.velocity.sqrMag < 1.5f*1.5f)
basically when the code compiles down and is executing, it checks the first statement, then the next etc, etc... So if waited returns false, it will not check the rest. IE: when the code is running on the cpu it kinda looks like so:
if(waited){
if(!isCollided){
if(this.transform.position.y > 1){
if(this.rigidbody2D.velocity.sqrmag < 1.5*1.5){
//The Code executes
}
}
}
}
anyways, a lot of this are micro improvements that may have dramatic results depending on how badly you are strapped for cpu
Thank you so much! I did have fairly dramatic results by making these changes! I feel pretty foolish having that large if statement and not breaking it up the way you did!
Please don't ask new questions in the comments. Feel free to post this as a new question.
Answer by Kiwasi · Jun 22, 2014 at 03:28 AM
General optimisation hints
Use sqrMagnitue instead of Magnitude for straight comparisons. Much cheaper.
The && is a special operator that will evaluate the left condition first, then stop if the left condition is false. The right condition is only evaluated if the left condition is true. So you always want to put the cheaper checks on the left hand side. Example:
// Slower
if (myVector3.sqrMagnitude < 2 && myBool)
// Faster
if (myBool && myVector3.sqrMagnitude < 2)
Use a for loop instead of a while loop when you are doing i++. Probably no major efficiency gains, but its convention.
Unless you use a variable each frame don't calculate it each frame. Move the calculation for localdown into the if statement where it used.
Your answer
Follow this Question
Related Questions
Location of RigidBody2D on collision 3 Answers
Issue with adding force and lerping position? Possible unity bug? 0 Answers
Public, private and getting stuff efficiency question. 2 Answers
Add 2d rigidbodies to moving objects to increase performance or not? 1 Answer
How to check place on empty for placement polygon Rigidbody2d? 0 Answers