Why is my score updating erratically?

My game has 2 controls. Move up/down, and change the player color. If the player (the circle) color matches the color of the tile they are on, their score increases. If the player and tile do not match colors, the player loses life.

For some reason the score and life update smoothly as expected (every 0.1f second) for 29-30 times but then it pauses for a second or two and becomes a bit erratic from then on despite the player continuously colliding with a solid block of tiles of the same color. I was hoping someone could pass their eyes over what I’ve done in case I’ve made a mistake with my general approach.

On Start I set a coroutine going in the PlayerController called StatusUpdateTimer.
In StatusUpdateTimer I wait for updateDelay seconds, call a StatusUpdate() method then set the coroutine going again.

In StatusUpdate(), I use Physics2D.OverlapCircle to create a list of objects colliding with player. I have a layer for each color (R/G/B) so I use a For loop to compare the layer of each colliding object with the player’s layer. If the colliding object layer matches the player’s then I increase the points based off how overlapped the player and the object are. If the colliding object is not the same layer (or the background) then I reduce the player health.

All this is happening in the PlayerController script, except the actual health and points updates which are functions of a GameManager script.

Does this sound like it should work?

It’s a lot more helpful to post your code then try to describe it for us to reconstruct. Don’t forget to use code tags.

1 Like

Yes please post code.

One thing to note is that a coroutine is not going to reliably run every .1 seconds using WaitForSeconds. There’s going to be a lot of error involved in that. Sometimes it will be .15 seconds, sometimes .102 seconds, etc…

A better way would be to use a float point counter, do your overlap check, and then add points based on Time.deltaTime * pointsPerSecond. You can then unload whole numbers of points from the float into an int in order to keep the value of the float low, and thus its precision very high. you could just do this in Update without a coroutine.

public class PlayerController : MonoBehaviour
{
    //references
    public GameManager gameManager;
    Rigidbody2D myRB;

    private Touch touch;
    public float moveSpeed;

    private GameObject player;
    private Renderer pSprite;

    public LayerMask playerLayerIndex; //8 White ,9 Red , 10 Green, 11 Blue
    private Color playerColor;

    public Color White;
    public Color Red;
    public Color Green;
    public Color Blue;
    Color[] ColorArray = new Color[4];



    //Tile interactions


    private List<Collider2D> collisions = new List<Collider2D>();
    public float radius = 0.47f;
    private float dist;

    private float colX;
    private float colY;
    public Vector2 colPos = new Vector2(0f, 0f);


    void Start()
    {
        player = gameObject;

        myRB = GetComponent<Rigidbody2D>();
        pSprite = GetComponent<Renderer>();

        playerLayerIndex = 8;
   
        ColorArray[0] = White;
        ColorArray[1] = Red;
        ColorArray[2] = Green;
        ColorArray[3] = Blue;
        StartCoroutine(StatusUpdateTimer());
 
    }

    void Update()
    {
   
    }

    void FixedUpdate()
    {
        if (Input.touchCount > 0)
        {
            touch = Input.GetTouch(0);


            if (touch.phase == TouchPhase.Moved)
            {
                //Use MovePosition in FixedUpdate as i'm being told its the most reliable for collisions
                Vector3 move = new Vector3(transform.position.x, transform.position.y + touch.deltaPosition.y * moveSpeed, transform.position.z);
                myRB.MovePosition(move);
            }
        }

    }


    public void SetPlayerColor(int layerIndex)
    {
        playerLayerIndex = layerIndex;
        pSprite.material.color = ColorArray[layerIndex - 8];
        player.layer = playerLayerIndex;

    }


    private void StatusUpdate()
    {
        collisions.Add(Physics2D.OverlapCircle(transform.position, radius));

        foreach (Collider2D nearbyObject in collisions)
        {

            //Calculate how much object overlaps player
            colX = nearbyObject.transform.position.x + 0.5f;
            colY = nearbyObject.transform.position.y - 0.5f;
            colPos = new Vector2(colX, colY);
            dist = Vector3.Distance(colPos, transform.position);

            //Decrease life
            if (nearbyObject.gameObject.layer != playerLayerIndex && nearbyObject.gameObject.layer != 8)
            {
                gameManager.UpdateLife(Mathf.Lerp(GameManager.minDmg, GameManager.maxDmg, dist));
                print("Decrease life called");
                //print(nearbyObject.name);
            }

            //Increase score
            if (nearbyObject.gameObject.layer == playerLayerIndex && nearbyObject.gameObject.layer != 8 && nearbyObject.gameObject != player)
            {
                gameManager.UpdateScore(Mathf.Lerp(GameManager.minScore, GameManager.maxScore, dist));
            
            }     
        }

        collisions.Clear();

    }


        public IEnumerator StatusUpdateTimer()
    {
       yield return new WaitForSeconds(GameManager.updateDelay);

        StatusUpdate();
        StartCoroutine(StatusUpdateTimer());
 
    }


 
 
}

I think the first thing to discern: Is StatusUpdate being called reliably, and functioning wrong? Or is it functioning fine but not being called at the right time?

As mentioned, a coroutine is a pretty bad way to do timed events, and a recursive timer like that adds another potential trouble spot. Here’s an example of a simpler, more reliable timer:

private float lastStatusUpdateTime = 0f;

void Update() {
while (Time.time > lastStatusUpdateTime + GameManager.updateDelay) {
lastStatusUpdateTime += GameManager.updateDelay;
StatusUpdate();
}
}

Note: The reason I used that += line is so that it runs a predictable number of times over a given period of time, preventing the issue described by @PraetorBlue . In most scenarios when using a timer like this, I’d just use lastStatusUpdateTime = Time.time; but in this situation running it the correct number of times is important.

Yeah, I think that’s what I’m seeing.

I’m not sure I entirely understand. Are you saying I should I replace the coroutine with a float that I increment in Update to a certain size (analogous to my previous updateDelay) at which point I call my Status update?

Thanks Star, I’ll take a look at this.

Edit:

That’s nice and simple and I can replace my coroutine with it but the effect seems to be similar. After 29-30 calls of the StatusUpdate(), it pauses for a few seconds before carrying on.

Could there be something wrong with my UpdateLife method?

    public void UpdateLife(float damage)
    {
        currentLife = currentLife - damage;
        txtLife.text = "Life: " + Mathf.Round(currentLife);
        txtDebugDmg.text = "dDmg: " + damage;
        healthBar.rectTransform.localScale = new Vector3 (currentLife / maxLife, 1f,1f);
 
    }

Edit2: I’m fairly sure its a problem with the code as I’m trying it with a grid like this now.