Studio 2: Code Review

Following on from my previous post,  this post will be a code review for A Place to Call, a game I’ve spend the last few weeks working on.

I’ll be focusing on the two Manager scripts that I used in A Place to Call, primarily because these are the scripts that could use the most improvement in terms of code quality and efficiency.

  • gameManager – The script controlling the game overall gamestate
  • uiManager – Obviously controls the UI

GameManager

Alongside the UImanager, audioManager, and cameraZoom scripts, the gameManager was a static class that could be accessed by all other scripts. This gameManager follows the following functions:

  1. End the game when the variables of warmth and comfort have been met
  2. Start the game again
  3. Pause and unpause the game
  4. Execute the pause menu functions
    1. unpause game
    2. restart game
    3. quit game
  5. Save and load the positions and rotations of all moveable objects (covered here)

1/2. Ending game and Starting again

The following code is called when the win states are met

        if (currComfort >= maxComfort && currHeat >= maxHeat && !Input.GetMouseButton(0))
{
currComfort = 0;
currHeat = 0;
zoomAway = true;
}

The mouse button code is simply so that when the player is dragging an object, they have to drop it first.

In this game I elected to be checking a variable instead of calling a function, in this case, zoomAway.

        if (zoomAway)
{
player.canMove = false;
UIManager.UI.showImages = false;
UIManager.UI.RemoveAll();
ZoomOut();
}

This stops the player from moving, removes all images, and begins the zoom out function.

    public void ZoomOut()
{
cameraZoom.CZ.zoomTarget = 20;

if (Camera.main.orthographicSize > (cameraZoom.CZ.zoomTarget – 2f))
{
UIManager.UI.fade = false;
}
}

Which then tells the camera to change its target.

What might have been more efficient than this code here, which does a lot of checks between static objects, is to have the camera itself know when its reached its zoom target, and have the camera tell the functions it needs to that its reached its destination.

When the camera target has been met the UI will start fading to black. Over in the UIManager, if fade is true, it removes the alpha from the image. When false it adds it again, making it opaque. Here, I do some dodgy things.

 public void Fade()
{
Color temp = fadeImage.color;

if (fade && temp.a > 0)
{
temp.a -= .5f * .02f;
fadeImage.color = temp;
}

else if (temp.a < 1f) { temp.a += .5f * .02f; fadeImage.color = temp; if (menuFade && temp.a > .95f)
{
gameManager.GM.menuCanvas.gameObject.SetActive(false);
Time.timeScale = 1;
menuFade = false;
fade = true;
}

if (gameManager.GM.zoomAway && temp.a > .95f)
{
gameManager.GM.EndGame();

if (restartFade)
{
gameManager.GM.menuCanvas.gameObject.SetActive(true);
Time.timeScale = 0;
restartFade = false;
}
}
}

So this fade image function checks the alpha of the image to do certain functions. When fading to black, it will check bools to do call certain things. The “menufade” bool is the fade that occurs between fading in and out of the main menu to the gameplay, which starts the game.

The “restart fade” is the fading back INTO the menu from gameplay (a poorly named variable I know). The game always ends when the fade to black is finished however, so that this function can fade in and out of black without going to the main menu.

What I SHOULD have done, is have this fade its own IEnumerator function, that can to a fade in and out with certain parameters, such as time. The systems that would call it would simply tell the ui to do a fade in and out, and wait for when it finishes its fade before doing anything.

Going back to the gameManager, EndGame() simply serialises the gameobjects positions and resets all the game variables that need to be reset.

    public void StartGame()
{
UIManager.UI.fade = true;//remove fade
flow.SetActive(true);//enable fungus
UIManager.UI.showImages = true;//ui is allowed to show images
player.timeTillMove = 8;
cameraZoom.CZ.zoomTarget = cameraZoom.CZ.origZoom;//reset camera pos

}

public void EndGame()
{
//SceneManager.LoadScene(SceneManager.GetActiveScene().buildIndex);
//Application.Quit();
Load();
player.gameObject.transform.position = player.startingPos;
player.dir = Vector3.zero;
player.addImage = true;//the first images on starting game
//variables to reset
hasShownFire = true; //the “fire is done” bool
flow.SetActive(false);//resets all the fungus stuff
zoomAway = false;//resets the “end game zoom”
player.canMove = false; //player can’t move during soom
player.isDragging = -1; //incase the player is dragging while the game ends
UIManager.UI.tute = true;//resets the “pull and break” bools
UIManager.UI.tuteTimer = 15f;//removes first images
hasFire = -1;//resets the current fire

StartGame();
}

Not much else to say on this part of the code. The main thing I would change is the UI fade and how it interacts with other functions, but the rest is fairly straightforward.

3/4. Pausing, and pause menu

The main thing here is that most of this code should have been in the UIManager.

        if (Input.GetMouseButtonDown(2) && !paused && !menuCanvas.isActiveAndEnabled)//so we don’t pause during the menu
{
//Debug.Log(“paused”);
Time.timeScale = 0f;
pauseCanvas.gameObject.SetActive(true);
paused = true;
return;
}

Instead of having the gameMangager cache the canvas that has the pausemenu, I should have just had the code on the UImanager, which would make more sense in the responsibilities of the UIManager. I could still do this and maintain functionality of the functions by letting the buttons still call from the UImanage using their event system.

    public void GameStartFromMenu()//called from main menu, starts the main menu specific fade
{
UIManager.UI.fade = false;
UIManager.UI.menuFade = true;
}

public void ResumeGame()//simply starts timescale again and disables the menu
{
Time.timeScale = 1f;
pauseCanvas.gameObject.SetActive(false);
paused = false;
}

public void RestartGame()
{
Time.timeScale = 1f; //unpause
pauseCanvas.gameObject.SetActive(false);
currComfort = 0; //reset values (these should be in the endgame function)
currHeat = 0;
paused = false;
zoomAway = true; //start the end game process
}

public void QuitGame()//same as restarting, except with the ui knowing the restart fade boo we covered before
{
Time.timeScale = 1f;
pauseCanvas.gameObject.SetActive(false);
paused = false;
zoomAway = true;
currHeat = 0;
currComfort = 0;
UIManager.UI.restartFade = true;
}

The “paused” bool doesn’t actually have to be there at all. I use this bool to check the mouse click, however I could simply do a check as to whether or not the pause menu is active.

We’ll segway into the UIManager from here, as I’ve talked a fair amount of it already.

UIManager

So the UImanager was an interesting one for me. This one doesn’t simply just have UI elements like the number for heat and comfort, this manager also creates and deletes images that appear when the player gets near them. To do this, I used a system where each image created knows its parent via the hashcode of the object that wants to create the image.

Each image has a script on it that simply lerps towards the object target, and has an offset postion relative to the ui space which is set in the inspector. To create a specific image, an object has to call whatever image they want to create (typically the moveableobjects do this).

    public void AddFireImage(Transform _pos, int _parentGO, int _objHeat)
{
if (showImages && _objHeat > 0)//if we’re allowed to make images and if there is a value even worth showing
AddImage(fireImage, _parentGO, _pos, _objHeat, 0, false, false);
}

By creating images this way, we can keep the “AddImage” function generic and keep code lines down.

_pos is the transform with the target position needed for the image

_parentGo is the int that the image keeps as a reference to its parent. We use an int as opposed to actually making the image a child of an object, because we need the image to be parented to the canvas to be properly displayedd.

_objHeat and _objComfort are the values shown on each object below the image. Now for the AddImage function

private void AddImage(GameObject go, int _parentGO, Transform _pos, int _heat, int _comfort, bool _fire, bool _tute)

Some of these variables are passed from the previous function and do the same thing, so here’s the new ones:

go is the image as a gameObject allocated in the inspector.

_fire bool is the indicator to show that the image being created is from a fire place, and should therefore show the “FirePit” text instead of a number

_tute is the drag and break images that follows the player when first starting the game.

        GameObject temp = Instantiate(go, _pos.position, Quaternion.identity, transform) as GameObject; //create image and get reference

UIInteraction temp2 = temp.GetComponent(); //get the script on the object

temp2.target = _pos; //give it its target position
temp2.goParent = _parentGO;//the int of the gameobject

currImages.Add(temp); //add to the list of created images

This creates the image, gives it the values needed, then adds it to a list so the uimanager can keep track of it.

Now we need to make some text fields to display the numbers of heat and warmth. While I should have made a prefab on the image GameObject, I instead made the text object through code to prevent myself from dealing with getting child components.

GameObject textGO = new GameObject();//instantiate object
textGO.transform.parent = temp.transform;//parent to image
textGO.transform.localPosition = new Vector3(0, -40, 0);//offset from image
Text textRef = textGO.AddComponent();//add text component

Now if here the tute bool comes into play.

if (!_tute)//a regular image
{
if (_fire)
textRef.text = “Fire Pit”;//if a fire pit

else
{
if (_heat > 0)//if we know this is showing a heat value
textRef.text = _heat.ToString();
else//if its not a heat value we know its a comfort value
textRef.text = _comfort.ToString();
}
}

And for the  tutorial images I decided not to make any more variables, rather hack in using the ones already made. Here, if the “heat” variable is more than 1, then the text reads left click.

        else
{
if (_heat > 0)//shh
textRef.text = “Left Click”;
else
textRef.text =”Right Click”;
}

And then we do the text formatting

        textRef.alignment = TextAnchor.MiddleCenter;
textRef.font = arial;//cached in awake
textRef.fontSize = 12;

Now for the methods for removing them. Now because they’re in a list, we can search through them easily. However, searching for a specific game object can be slow, which is why we used the hashes from the gameobjects to find the images we wanted.

    public void RemoveImage(int _parentGO)
{
for (int index = currImages.Count – 1; index >= 0; –index)//reverse loop for removals
{
//get objects parent value
UIInteraction temp = currImages[index].GetComponent();
if (temp.goParent == _parentGO)
{
currImages.RemoveAt(index);//remove from list
Destroy(temp.gameObject);//delete object
}
}
}

This code allows for an object to pass in its own gameobject hash, which will then delete every image that is “parented” to it. While the getcomponents can be slow, this system is flexible in the manner of creating and destroying images. I could create any game object, and they could have an image overlay created for them as long as they have the image for it.

I also added functionality for various other searches:

    public bool CheckForImage(int _parentGO) //check if the gameobject even has an image
{
foreach(GameObject go in currImages)
{
UIInteraction temp = go.GetComponent();
if (temp.goParent == _parentGO)
return true;
}
return false;
}

public void RemoveAllButThis(int _parentGO)//remove all images but the ones on the gameobject
{
for (int index = currImages.Count – 1; index >= 0; –index)//reverse loop for removals
{
//get objects parent value
UIInteraction temp = currImages[index].GetComponent();
if (temp.goParent != _parentGO)
{
currImages.RemoveAt(index);//remove from list
Destroy(temp.gameObject);//delete object
}
}
}

public void RemoveAll()//kill them all Johnny
{
foreach(GameObject img in currImages)
{
Destroy(img.gameObject);
}
currImages.Clear();
}

Besides what I’ve already said about the Fade function, I’m not sure what could obviously be changed with the UIManager.

Leave a Reply