Hi everyone,
I’m trying to set up zoom in and out UI buttons and believe my issue comes from my coroutine below. The problem is that the zoom in and out FOV doesn’t stop on coroutine end. I’ve tried using Mathf.Approximately with the same result as below.
I’m calling the function by the zoomGo bools in the update and then the rest of the values are set else where in the script – all of which seem to be right; at least I think so. I suspect that the problem is in my return statements but both debug logs are firing. Any advice would be much appreciated!
IEnumerator CameraZoomIn(float _zoomValue)
{
_zoomInGo = false;
_zoomOutGo = false;
while (_zoomValue <= _currentFOV)
{
mainCamera.fieldOfView = _currentFOV += _zoomValue * Time.deltaTime;
Debug.Log(_zoomInGo + " " + " " + _zoomOutGo);
yield return null;
}
yield return mainCamera.fieldOfView = _zoomValue;
Debug.Log("Coroutine completed");
}
The logic here is confusing. It’s not helped by the combination of = and += in the same line (which, while valid, is generally considered confusing, unnecessary, and bad coding style). But fundamentally, the key bit of what happens in the loop is:
- _currentFOV’s value increases by _zoomValue
- you check to see if _zoomValue <= _currentFOV
- Since _zoomValue never increases and _currentFOV never decreases, this will never stop evaluating to “true” - their values will only get further away from each other
It seems like you’re using _zoomValue in one place more than is appropriate, but with a name as vague as “zoomValue” I can’t tell what the intention behind it is. Is it the desired final FOV? Is it the amount the FOV should change each frame? Try renaming _zoomValue to something more descriptive, and I suspect the problems with the logic will become more apparent. For example, if it’s the desired final FOV, then it should be clear that that’s not the value that should be getting added to _currentFOV, since that’s kind of nonsensical.
1 Like
Hey StarManta,
Thanks for the great response! I’m still very new to coding so I don’t know best practices for anything – I’m struggling with just getting things to work but I definitely appreciate knowing how to improve. What is a better way to write line 9?
To answer your question, _zoomValue is the product of adding an increment or decrement value to to _currentFOV, depending on if the zoom in or out UI button is clicked. I have a coroutine for each function and the CameraZoomOut() method works which is pretty much just the inverse of what I posted previously. (Adding the entire script below)
using System;
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using UnityEngine.Experimental.PlayerLoop;
public class CameraZoom : MonoBehaviour
{
public Camera mainCamera;
private float _currentFOV = 40.0f;
private const float _maxFOV = 40.0f;
private const float _minFOV = 20.0f;
private bool _zoomInGo = false;
private bool _zoomOutGo = false;
private float _zoomValue;
private void Start()
{
mainCamera.fieldOfView = _currentFOV;
}
private void Update()
{
if (_zoomInGo == true) { ZoomIn(); }
else if (_zoomOutGo == true) { ZoomOut(); }
}
private void ZoomIn()
{
const float zoomInValue = -5.0f;
if (mainCamera == null) return;
if (_currentFOV >= _minFOV)
{
_zoomValue = _zoomValue += zoomInValue;
StartCoroutine(CameraZoomIn(_zoomValue));
Debug.Log("ZoomInValue = " + _zoomValue);
}
else if (_currentFOV <= _minFOV)
{
_zoomValue = _minFOV;
StartCoroutine(CameraZoomIn(_zoomValue));
Debug.Log("ZoomInValue minFOV = " + _zoomValue);
}
}
private void ZoomOut()
{
const float zoomOutValue = 5.0f;
if (mainCamera == null) return;
if (_currentFOV <= _maxFOV)
{
_zoomValue = _zoomValue += zoomOutValue;
StartCoroutine(CameraZoomOut(_zoomValue));
Debug.Log("ZoomOutValue = " + _zoomValue);
}
else if (_currentFOV <= _maxFOV)
{
_zoomValue = _maxFOV;
StartCoroutine(CameraZoomOut(_zoomValue));
Debug.Log("ZoomOutValue maxFOV = " + _zoomValue);
}
}
IEnumerator CameraZoomIn(float _zoomValue)
{
_zoomInGo = false;
_zoomOutGo = false;
while (_zoomValue <= _currentFOV)
{
mainCamera.fieldOfView = _currentFOV += _zoomValue * Time.deltaTime;
Debug.Log(_zoomInGo + " " + " " + _zoomOutGo);
yield return null;
}
yield return mainCamera.fieldOfView = _zoomValue;
Debug.Log("Coroutine completed");
}
IEnumerator CameraZoomOut(float _zoomValue)
{
_zoomInGo = false;
_zoomOutGo = false;
while (_zoomValue >= _currentFOV)
{
mainCamera.fieldOfView = _currentFOV += _zoomValue * Time.deltaTime;
Debug.Log(_zoomInGo + " " + " " + _zoomOutGo);
yield return null;
}
yield return mainCamera.fieldOfView = _zoomValue;
Debug.Log("Coroutine completed");
}
public void ZoomInGo() { _zoomInGo = true; }
public void ZoomOutGo() { _zoomOutGo = true; }
}
Just split it into two lines: one that increases _currentFOV, then the second one assigns that value to mainCamera.fieldOfView.
I think this needs to be clarified further. When I’m in the middle of this function, I don’t need to know or care how _zoomValue was arrived at - I care what this function is using it for. It sounds like it’s the desired FOV, in which case, its usage in line 9 is incorrect. Seems like that should be a constant value, (made negative in one of the in/out functions), that represents the speed of the zoom.
BTW, if the only difference between the functions is moving up or down, you could easily combine the two into one function that, instead of using += or -=, just uses MoveTowards. This will have the added advantage of actually stopping right on the exact value once it’s reached it (rather than slightly overshooting), which means you can safely use != in your “while” condition.
1 Like
I wanted to post a quick update and say thanks again StarManta, I finally figured it out. You were right, my naming convention was confusing and I was doubling up on variables that I didn’t need. I renamed my variables _currentFOV and _newFOV and created logic to get the appropriate value for _newFOV and then set the _currentFOV to that. I ended up using just a Mathf.Lerp to apply the new value in a coroutine instead of the MoveTowards method, but it works bug free and that’s what matters, right?
Going from 3D art to scripting is a pretty difficult transition, but I really like being able to make stuff happen now. Thanks again for taking the time to help me out and have a great day!
2 Likes