2016-05-29 12 views
1

Zusammenfassung: Ich frage mich, ob mein Code unnötigerweise überflüssig ist. Ich bemerkte, als ich über meinen Code sah, dass ich eine unnötige Variable und if-Anweisung hatte. Beide Fire() - Methoden erreichen das gewünschte Ergebnis ohne Fehler (von dem, was ich gesehen habe). Für mich ist die "Redundant Fire" -Methode viel besser und meine bevorzugte Methode, aber ich frage mich nur, ob die Alternate-Fire-Methode aufgrund der Effizienz immer noch den Auswertbarkeitsgrad wert ist. Ich frage mich auch, ob es irgendwelche zugrundeliegenden Fehler geben könnte, die in meinen Methoden auftreten können, von denen ich nichts weiß.Redundanter Aufnahmecode

EDIT: Unity Version 4.6.3

Feuer Methode Ergebnis:, wenn die Leertaste gedrückt wird - erzeugt einen Laser, der nach oben in Richtung des Gegners mit der vorgegebenen Geschwindigkeit ausgelöst und durch das Nachladen verzögert Zeit.

Gewünschtes Ergebnis: Wenn die Leertaste gedrückt wird - spawnen Sie einen Laser und lassen Sie ihn mit der voreingestellten Geschwindigkeit nach oben auf die Feinde schießen und haben dann eine Verzögerung entsprechend der voreingestellten Nachladezeit.

Variablen:

public float reloadTime = 0.2f; 
public GameObject playerProjectilePrefab; 
public float playerProjectileSpeed = 10f; 

private bool canShoot = true; 
private float reloadCountDown; 

Redundant Feuer Methode:

void Fire(){ 
    Vector3 startPosition = transform.position + new Vector3(0 ,(renderer.bounds.size.y * 0.5f), 0); 

    if (canShoot && Input.GetKey(KeyCode.Space)){ 
     canShoot = false; 
     GameObject playerProjectile = Instantiate(playerProjectilePrefab, startPosition, Quaternion.identity) as GameObject; 
     playerProjectile.rigidbody2D.velocity = new Vector3 (0, playerProjectileSpeed, 0); 
     reloadCountDown = reloadTime; 
    } else if (!canShoot){ 
     reloadCountDown -= Time.deltaTime; 
    } 
    if (reloadCountDown <= 0f){ 
     canShoot = true; 
    } 
} 

Alternate Feuer Methode:

void Fire(){ 
    Vector3 startPosition = transform.position + new Vector3(0 ,(renderer.bounds.size.y * 0.5f), 0); 

    if (reloadCountDown <= 0f && Input.GetKey(KeyCode.Space)){ 
     GameObject playerProjectile = Instantiate(playerProjectilePrefab, startPosition, Quaternion.identity) as GameObject; 
     playerProjectile.rigidbody2D.velocity = new Vector3 (0, playerProjectileSpeed, 0); 
     reloadCountDown = reloadTime; 
    } else if (reloadCountDown > 0f){ 
     reloadCountDown -= Time.deltaTime; 
    } 
} 

Th anks im voraus!

+2

Der Umfang dieser Frage scheint besser geeignet für den [Code-Review] (http://codereview.stackexchange.com/) Seite? ˅. –

+0

@MrD Okay, danke, ich werde es dort erneut posten, wenn ich eine andere Frage stellen darf (40 Minuten). – jozza710

+2

Welche Unity-Version verwenden Sie und geben Sie ** Edit ** in Ihre Frage ein und beschreiben Sie, was in der Fire-Funktion passiert. Das wird helfen, festzustellen, ob es mehr Verbesserung braucht – Programmer

Antwort

1

Basierend auf was Sie den Code in der Funktion tun, ist der erste Code ein wenig redundant, aber die zweite behoben.

Da Sie Effizienz erwähnt haben, können Sie immer noch den zweiten Code optimieren.

Ihre nächste Problem ist, auf dieser Codezeile:

GameObject playerProjectile = Instantiate(playerProjectilePrefab, startPosition, Quaternion.identity) as GameObject; 

, das zu tun, werden Sie das Zuweisen von Speicher jedes Mal, wenn Sie schießen. Wenn Sie Objekt mehr als einmal instanziieren möchten, dann sollte Object Pooling implementiert werden. Alles was es tut, um das bereits instantiierte GameObject wiederzuverwenden.

Es ist sehr wahrscheinlich, dass Sie Destroy(gameObject) in Ihrem Code haben, der auch teuer ist. Wenn nicht, dann ist das noch schlimmer, weil Sie neue Objekte erstellen, ohne die alten zu zerstören. Sobald das Object Pooling implementiert ist, müssen Sie und Destroy nicht erneut verwenden. Sie verwenden nur Ihr Pooling-Skript, um das verfügbare GameObject zurückgeben zu können.

Here können Sie lernen, wie Sie Ihr eigenes Objektpooling in Unity implementieren.

Weitere Hinweise:

Wenn Sie Unity aktualisieren 5,

playerProjectile.rigidbody2D.velocity wird playerProjectile.GetComponent<Rigidbody2D>().velocity.

renderer.bounds.size.y wird gameObject.GetComponent<Renderer>().bounds.size.y

Da renderer.bounds ist die-gleiche wie gameObject.renderer.bounds und Sie effizienten Code wollen, sollten Sie die renderer in der Start() Funktion zwischenzuspeichern.

Renderer cachedRender = null; 
void Start() 
{ 
    cachedRender = gameObject.GetComponent<Renderer>(); 
} 

Dann in Ihrer Fire() Funktion, können Sie es gerne verwenden:

cachedRender.bounds.size.y * 0.5f 
Verwandte Themen