What is wrong with this code?

What’s wrong with this code? I used tutorial but it did not show any help with bugs.


public Image healthBarImage;
public PlayerController player;
Tween tween;
bool moving = false;
public CanvasGroup parentCanvasGroup;
void Update() {
  transform.rotation = startRotation;
  if ( moving && player.characterController.velocity.magnitude < 0.1f ) {
    if ( tween != null ) {
      tween.Kill();
    }
    moving = false;
    tween = parentCanvasGroup.DOFade( 1f, 0.75f );
  } else if ( !moving && player.characterController.velocity.magnitude > 0.15f ) {
    if ( tween != null ) {
      tween.Kill();
    }
    moving = true;
    tween = parentCanvasGroup.DOFade( 0f, 0.75f );
  }
}

First of all, for an experienced poster, this is a horrible layout! I think you know better how to post code, given how active you are… Anyway, on to the code.

There’s no class definition and, hence no Monobehaviour so transform is undefined

There’s no Class called Image

startRotation is not defined

characterController appears to be a component but you haven’t got a GetComponent to find it

Fix those then come back again and please use the Code Sample button :o)

There is a lot wrong with it

  1. Its a code fragment i.e. its not all the relevant code
  2. It doesnt run
  3. The formatting is horrific
  4. Its a bit of a mess (see below for specifics)

The issue with it running has already been answered so I will try to address the formatting issues.


  1. Method parameters should not be padded with white space
  2. Scoping brackets i.e. { } should be placed on a new line in C#
  3. Public members should be in Pascal Case
  4. Private members should be in camel case (optionally preceded with an underscore)
  5. I suggest being explicit and consistent with member access modifiers
  6. I suggest being consistent with use of spacing/ new lines
  7. You dont need to repeat code i.e. the tween null check is written twice unnecessarily.
  8. I suggest not binding the logic for determining if something is moving with the logic for what you want to happen as a result of that.
  9. This Monobehaviour doesnt seem to be a well organised entity it seems to just be a hodgepodge of responsibilities.

Example


using UnityEngine;
using System.Collections;

public class MySomethingClass : MonoBehaviour
{   
    public CanvasGroup ParentCanvasGroup;
    public Image HealthBarImage;
    public PlayerController Player;
    private Tween tween;

    // Note this logic is a little odd in your code there is a dead zone ... I have removed it for the sake of argument.
    private bool isMoving => Player.characterController.velocity.magnitude > 0.15f;

    private void Update() 
    {
        transform.rotation = startRotation;

        if (tween == null) 
            return; // Or throw exception if this is not supposed to happen ... or you know just set it in here so you are sure it cant be null.
        
        tween.Kill();

        if(isMoving)
        {
            tween = ParentCanvasGroup.DOFade(1f, 0.75f);
        }
        else 
        {
            tween = ParentCanvasGroup.DOFade(0f, 0.75f);
        }
    }
}

This line tween = parentCanvasGroup.DOFade( 0f, 0.75f ); looks fishy to me. In the documentation doesn’t appear a DOFade method for CanvasGroup but also you’re trying to assign its result to a variable of type tween and that doesn’t look ok.

EDIT: also please try to use a correct formating when inserting code.