I need guidance please.

panim0_unity · Jan 16, 2020

Hi, the code below is kind of long way of doing what im trying to do. There is 3 allies and 3 enemies right now but I'll add more in future so i need to add more agents and enemyplayer gameobjects to this script.
I need an advice here to shorten these lines, any suggestion,guide me a bit please. Basically you select which ally will attack to which enemy when round starts and repeats it 2 times.

Code (CSharp):

using System.Collections;

using System.Collections.Generic;

using UnityEngine;

using UnityEngine.AI;

using UnityEngine.UI;

public class PlayerControls : MonoBehaviour

{

GameObject closest;

NavMeshAgent agent1;

GameObject agent1G;

bool agent1select;

NavMeshAgent agent2;

GameObject agent2G;

bool agent2select;

NavMeshAgent agent3;

GameObject agent3G;

bool agent3select;

GameObject selectionQuad1;

GameObject selectionQuad2;

GameObject selectionQuad3;

GameObject enemyPlayer1;

bool enemy1select;

GameObject enemyPlayer2;

bool enemy2select;

GameObject enemyPlayer3;

bool enemy3select;

Combat combat1;

Combat combat2;

Combat combat3;

Vector3 pos1;

Vector3 pos2;

Vector3 pos3;

Quaternion rot1;

Quaternion rot2;

Quaternion rot3;

bool canClick;

[SerializeField]

private Text waveTimer;

Animator animator1;

Animator animator2;

Animator animator3;

bool timeToAttack;

void Start()

{

agent1select = false;

agent2select = false;

agent3select = false;

enemy1select = false;

enemy2select = false;

enemy3select = false;

canClick = true;

timeToAttack = false;

StartCoroutine(Countdown(15));

}

void FixedUpdate()

{

if (Input.GetMouseButtonDown(0)&&canClick==true)

{

RaycastHit hit = new RaycastHit();

bool hitb = Physics.Raycast(Camera.main.ScreenPointToRay(Input.mousePosition), out hit);

if (hitb)

{

Debug.Log("Hit " + hit.transform.gameObject.name);

if (hit.transform.gameObject.tag == "Player1")

{

Debug.Log("Working until here");

if (agent1select == false)

{

Debug.Log("Clicked");

agent1G = hit.transform.gameObject;

animator1 = agent1G.GetComponent<Animator>();

agent1 = hit.transform.gameObject.GetComponent<NavMeshAgent>();

pos1 = agent1G.transform.position;

rot1 = agent1G.transform.rotation;

Debug.Log("agent 1 selected");

agent1select = true;

selectionQuad1 = agent1G.transform.GetChild(2).gameObject;

selectionQuad1.SetActive(true);

}

else if (agent1select && agent2select == false)

{

agent2G = hit.transform.gameObject;

animator2 = agent2G.GetComponent<Animator>();

agent2 = hit.transform.gameObject.GetComponent<NavMeshAgent>();

pos2 = agent2G.transform.position;

rot2 = agent2G.transform.rotation;

Debug.Log("agent 2 selected");

agent2select = true;

selectionQuad1.SetActive(false);

selectionQuad2 = agent2G.transform.GetChild(2).gameObject;

selectionQuad2.SetActive(true);

}

else if (agent2select && agent3select == false)

{

agent3G = hit.transform.gameObject;

animator3 = agent3G.GetComponent<Animator>();

agent3 = hit.transform.gameObject.GetComponent<NavMeshAgent>();

pos3 = agent3G.transform.position;

rot3 = agent3G.transform.rotation;

Debug.Log("agent 3 selected");

agent3select = true;

selectionQuad2.SetActive(false);

selectionQuad3 = agent3G.transform.GetChild(2).gameObject;

selectionQuad3.SetActive(true);

}

}

else

{

Debug.Log("no hit");

}

}

}

if (Input.GetMouseButtonDown(0)&&canClick==true)

{

RaycastHit hit2 = new RaycastHit();

if (Physics.Raycast(Camera.main.ScreenPointToRay(Input.mousePosition), out hit2))

{

if (hit2.transform.gameObject.tag == "Player")

{

if (enemy1select == false&&agent1select==true)

{

enemyPlayer1 = hit2.transform.gameObject;

combat1 = enemyPlayer1.GetComponent<Combat>();

Debug.Log("Enemy1 Clicked");

enemy1select = true;

}

else if (enemy1select && enemy2select == false&&agent2select==true)

{

enemyPlayer2 = hit2.transform.gameObject;

combat2 = enemyPlayer2.GetComponent<Combat>();

Debug.Log("Enemy2 Clicked");

enemy2select = true;

}

else if (enemy2select && enemy3select == false&&agent3select==true)

{

enemyPlayer3 = hit2.transform.gameObject;

combat3 = enemyPlayer3.GetComponent<Combat>();

Debug.Log("Enemy 3 Clicked");

enemy3select = true;

selectionQuad3.SetActive(false);

}

}

}

}

if (timeToAttack==true)

{

if (enemy1select)

{

agent1.SetDestination(enemyPlayer1.transform.position);

FaceTarget1();

if (combat1.isDeath == true)

{

enemyPlayer1 = null;

enemyPlayer1 = FindClosestEnemy().transform.gameObject;

agent1.SetDestination(enemyPlayer1.transform.position);

}

}

if (enemy2select)

{

agent2.SetDestination(enemyPlayer2.transform.position);

FaceTarget2();

if (combat2.isDeath == true)

{

enemyPlayer2 = null;

enemyPlayer2 = FindClosestEnemy().transform.gameObject;

agent2.SetDestination(FindClosestEnemy().transform.position);

}

}

if (enemy3select)

{

agent3.SetDestination(enemyPlayer3.transform.position);

FaceTarget3();

if (combat3.isDeath == true)

{

enemyPlayer3 = null;

enemyPlayer3 = FindClosestEnemy().transform.gameObject;

agent3.SetDestination(FindClosestEnemy().transform.position);

}

}

}

if (timeToAttack)

{

Debug.Log("It is false");

}

if (enemyPlayer1 != null && agent1G != null)

{

float distance1 = Vector3.Distance(enemyPlayer1.transform.position, agent1G.transform.position);

if (distance1 <= 7)

{

animator1.SetBool("Attack", true);

animator1.speed = 1.5f;

}

}

if (enemyPlayer2 != null && agent2G != null)

{

float distance2 = Vector3.Distance(enemyPlayer2.transform.position, agent2G.transform.position);

if (distance2 <= 7)

{

animator2.SetBool("Attack", true);

animator2.speed = 1.5f;

}

}

if (enemyPlayer3 != null && agent3G != null)

{

float distance3 = Vector3.Distance(enemyPlayer3.transform.position, agent3G.transform.position);

if (distance3 <= 7)

{

animator3.SetBool("Attack", true);

animator3.speed = 1.5f;

}

}

}

IEnumerator Countdown(int seconds)

{

canClick = true;

waveTimer = GameObject.Find("Canvas/Timer").GetComponent<Text>();

int counter = seconds;

int counter2 = seconds+5;

while (counter > 0)

{

yield return new WaitForSeconds(1);

counter--;

waveTimer.text = "" + counter;

}

if (counter == 0)

{

timeToAttack = true;

canClick = false;

counter = seconds+5;

}

while (counter > 0)

{

yield return new WaitForSeconds(1);

counter--;

waveTimer.text = "" + counter;

if (counter == 0)

{

timeToAttack = false;

Debug.Log("timeToAttack= "+timeToAttack);

canClick = true;

Debug.Log("Fight Over!");

if (agent1G != null)

{

agent1.isStopped = true;

animator1.SetBool("Attack", false);

agent1G.transform.position = pos1;

agent1G.transform.rotation = rot1;

}

if (agent2G != null)

{

agent2.isStopped = true;

animator2.SetBool("Attack", false);

agent2G.transform.position = pos2;

agent2G.transform.rotation = rot2;

}

if (agent3G != null)

{

agent3.isStopped = true;

animator3.SetBool("Attack", false);

agent3G.transform.position = pos3;

agent3G.transform.rotation = rot3;

}

agent1 = null;

agent1G = null;

agent1select = false;

agent2 = null;

agent2G = null;

agent2select = false;

agent3 = null;

agent3G = null;

agent3select = false;

enemyPlayer1 = null;

enemy1select = false;

enemyPlayer2 = null;

enemy2select = false;

enemyPlayer3 = null;

enemy3select = false;

counter = seconds;

}

}

while (counter > 0)

{

yield return new WaitForSeconds(1);

counter--;

waveTimer.text = "" + counter;

}

if (counter == 0)

{

timeToAttack = true;

canClick = false;

counter = seconds+5;

}

while (counter > 0)

{

yield return new WaitForSeconds(1);

counter--;

waveTimer.text = "" + counter;

if (counter == 0)

{

timeToAttack = false;

Debug.Log("timeToAttack= " + timeToAttack);

canClick = true;

Debug.Log("Fight Over!");

if (agent1G != null)

{

agent1.isStopped = true;

animator1.SetBool("Attack", false);

agent1G.transform.position = pos1;

agent1G.transform.rotation = rot1;

}

if (agent2G != null)

{

agent2.isStopped = true;

animator2.SetBool("Attack", false);

agent2G.transform.position = pos2;

agent2G.transform.rotation = rot2;

}

if (agent3G != null)

{

agent3.isStopped = true;

animator3.SetBool("Attack", false);

agent3G.transform.position = pos3;

agent3G.transform.rotation = rot3;

}

agent1 = null;

agent1G = null;

agent1select = false;

agent2 = null;

agent2G = null;

agent2select = false;

agent3 = null;

agent3G = null;

agent3select = false;

enemyPlayer1 = null;

enemy1select = false;

enemyPlayer2 = null;

enemy2select = false;

enemyPlayer3 = null;

enemy3select = false;

counter = seconds;

}

}

}

#region faceTarget

void FaceTarget1()

{

Vector3 direction = (enemyPlayer1.transform.position - agent1.transform.position).normalized;

Quaternion lookRotation = Quaternion.LookRotation(new Vector3(direction.x, 0, direction.z));

agent1.transform.rotation = Quaternion.Slerp(agent1.transform.rotation, lookRotation, Time.deltaTime * 10f);

}

void FaceTarget2()

{

Vector3 direction = (enemyPlayer2.transform.position - agent2.transform.position).normalized;

Quaternion lookRotation = Quaternion.LookRotation(new Vector3(direction.x, 0, direction.z));

agent2.transform.rotation = Quaternion.Slerp(agent2.transform.rotation, lookRotation, Time.deltaTime * 10f);

}

void FaceTarget3()

{

Vector3 direction = (enemyPlayer3.transform.position - agent3.transform.position).normalized;

Quaternion lookRotation = Quaternion.LookRotation(new Vector3(direction.x, 0, direction.z));

agent3.transform.rotation = Quaternion.Slerp(agent3.transform.rotation, lookRotation, Time.deltaTime * 10f);

}

#endregion

#region findClosest

GameObject FindClosestEnemy()

{

GameObject[] gos;

gos = GameObject.FindGameObjectsWithTag("Player");

float distance = Mathf.Infinity;

Vector3 position = transform.position;

foreach (GameObject go in gos)

{

Vector3 diff = go.transform.position - position;

float curDistance = diff.sqrMagnitude;

if (curDistance < distance)

{

closest = go;

distance = curDistance;

}

}

return closest ;

}

#endregion

}

kdgalla · Jan 16, 2020

Having different numbered variables for each entity and then writing tons of redundant code for each combination is completely unnecessary and will make your code impossible to work with. There are programming tools for working with groups of the same object.
You should write a class or a struct that has all of the information for one single ally or enemy and then make a list or an array of these.

You should research C# arrays:
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/arrays/
and lists:
https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.list-1?view=netframework-4.8

The array example shows how to use an array of ints but you can just as well make an array of GameObjects or whatever script your ally or enemy uses.

panim0_unity · Jan 16, 2020

kdgalla said: ↑

Having different numbered variables for each entity and then writing tons of redundant code for each combination is completely unnecessary and will make your code impossible to work with. There are programming tools for working with groups of the same object.
You should write a class or a struct that has all of the information for one single ally or enemy and then make a list or an array of these.

You should research C# arrays:
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/arrays/
and lists:
https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.list-1?view=netframework-4.8

The array example shows how to use an array of ints but you can just as well make an array of GameObjects or whatever script your ally or enemy uses.
Click to expand...

I thought about that too but just I could not figure out how to make this click and select thing happen using arrays or list.
so maybe rather than selecting wich to wich I will make auto select by using find closest. but if you may suggest a way of doşng selection that would be great. Sorry I am actuallny not new to this but I have two teams to handle so my braind kinda stopped working

StarManta · Jan 16, 2020

A few notes not related to your specific question:

1) Don't used FixedUpdate for anything that's not physics-related. For input specifically, it will cause a lot of problems (as just one example, GetMouseButtonDown may return true for multiple FixedUpdates in a row if these FixedUpdates happen on the same frame.... or it may miss input entirely if your framerate is high enough that FixedUpdate isn't getting run every frame). Besides the input issues, anything in FixedUpdate has the potential to slow your game down exponentially, because the more the game slows down the more times FU needs to be called each frame. A more detailed explanation of this here. Long story short: only used FixedUpdate for physics-related code, period.
2) GameObject tags suck. It's a badly designed, non-extensible system, and if you use it you'll just code yourself into corners. Avoid.

For the actual question:
I was going to write a way to redo your existing logic with a class and an array, but honestly I think the problem isn't really with your data structures but with your algorithm itself. (I mean, having those data structures would help make your algorithm more readable and extensible, but the things you need to do here don't really call for that, IMO.)

STEP 1: First thing I'd do is to make a script that goes on each player and enemy object. If you want, you can use inheritance so that they share a type but also have their own type. You may already have a script like this, and if so, you can use it. And this script doesn't need to actually have any functionality of its own (yet) - here I'm mostly going to use it as fodder for GetComponent and the like. (I find this kind of thing much more useful as a "tag" system than Unity's GameObject.tag system, because A) it allows you to apply multiple tags per object, B) you can find objects tagged in the children of a given object, and C) the tags can use inheritance. The downside is that it's very very slightly slower, but as long as checking for tags isn't something that happens a thousand times per frame this isn't a difference that matters, and the added functionality is more than worth it.)

So let's do this:

Code (csharp):

//each of these in its own script file

public class SelectableUnit : MonoBehaviour {

}

public class SelectablePlayer : SelectableUnit {

}

public class SelectableEnemy : SelectableUnit {

}

Attach the appropriate script of these to each of your enemy and player objects - if your units have multiple parented GameObjects, attach the script to the one that contains the collider that you use for raycasting. (Probably the same GameObject you have tagged as Player1, Player2, etc)

STEP 2: Now let's look at your selection code. You have a lot of different variables, and I think those are all pretty unnecessary. You should be able to boil this down to two things:
1) Which, if any, friendly unit is selected?
2) Which, if any, enemy unit is selected?
Which we can represent as:

Code (csharp):

public SelectablePlayer selectedFriendlyUnit;

public SelectableEnemy selectedEnemyUnit;

STEP 3: Now let's take a look at Update. You do a raycast, good. Now, instead of checking the gameobject's tag, let's use the scripts we attached in step 1, like this:

Code (csharp):

if (Input.GetMouseButtonDown(0)&&canClick==true)

{

RaycastHit hit = new RaycastHit();

bool hitb = Physics.Raycast(Camera.main.ScreenPointToRay(Input.mousePosition), out hit);

if (hitb)

{

Debug.Log("Hit " + hit.transform.gameObject.name);

SelectablePlayer hitUnit = hit.gameObject.GetComponent<SelectablePlayer>();

if (hitUnit != null)

{

selectedFriendlyUnit = hitUnit;

}

And just like that, you have selected an object - any object! At this point, if you want to do stuff like show an indicator of being selected, you can add a function to SelectableUnit to handle this, and just call it here using selectedFriendlyUnit.SetSelectionQuadVisible(true) or something like that. If it's on SelectableUnit, you don't need multiple copies of code.

STEP 4: Now that we have an object selected, you can use the same logic to select the enemy you want to attack. You should be able to use the same logic we've used already to select this unit, so I'm going to leave that as an exercise for you.

STEP 5: When it's time to attack, you should now be able to just use selectedFriendlyUnit and selectedEnemyUnit directly.

Does all this make sense?

panim0_unity · Jan 16, 2020

StarManta said: ↑

A few notes not related to your specific question:

1) Don't used FixedUpdate for anything that's not physics-related. For input specifically, it will cause a lot of problems (as just one example, GetMouseButtonDown may return true for multiple FixedUpdates in a row if these FixedUpdates happen on the same frame.... or it may miss input entirely if your framerate is high enough that FixedUpdate isn't getting run every frame). Besides the input issues, anything in FixedUpdate has the potential to slow your game down exponentially, because the more the game slows down the more times FU needs to be called each frame. A more detailed explanation of this here. Long story short: only used FixedUpdate for physics-related code, period.
2) GameObject tags suck. It's a badly designed, non-extensible system, and if you use it you'll just code yourself into corners. Avoid.

For the actual question:
I was going to write a way to redo your existing logic with a class and an array, but honestly I think the problem isn't really with your data structures but with your algorithm itself. (I mean, having those data structures would help make your algorithm more readable and extensible, but the things you need to do here don't really call for that, IMO.)

STEP 1: First thing I'd do is to make a script that goes on each player and enemy object. If you want, you can use inheritance so that they share a type but also have their own type. You may already have a script like this, and if so, you can use it. And this script doesn't need to actually have any functionality of its own (yet) - here I'm mostly going to use it as fodder for GetComponent and the like. (I find this kind of thing much more useful as a "tag" system than Unity's GameObject.tag system, because A) it allows you to apply multiple tags per object, B) you can find objects tagged in the children of a given object, and C) the tags can use inheritance. The downside is that it's very very slightly slower, but as long as checking for tags isn't something that happens a thousand times per frame this isn't a difference that matters, and the added functionality is more than worth it.)

So let's do this:

Code (csharp):

//each of these in its own script file

public class SelectableUnit : MonoBehaviour {

}

public class SelectablePlayer : SelectableUnit {

}

public class SelectableEnemy : SelectableUnit {

}

Attach the appropriate script of these to each of your enemy and player objects - if your units have multiple parented GameObjects, attach the script to the one that contains the collider that you use for raycasting. (Probably the same GameObject you have tagged as Player1, Player2, etc)

STEP 2: Now let's look at your selection code. You have a lot of different variables, and I think those are all pretty unnecessary. You should be able to boil this down to two things:
1) Which, if any, friendly unit is selected?
2) Which, if any, enemy unit is selected?
Which we can represent as:

Code (csharp):

public SelectablePlayer selectedFriendlyUnit;

public SelectableEnemy selectedEnemyUnit;

STEP 3: Now let's take a look at Update. You do a raycast, good. Now, instead of checking the gameobject's tag, let's use the scripts we attached in step 1, like this:

Code (csharp):

if (Input.GetMouseButtonDown(0)&&canClick==true)

{

RaycastHit hit = new RaycastHit();

bool hitb = Physics.Raycast(Camera.main.ScreenPointToRay(Input.mousePosition), out hit);

if (hitb)

{

Debug.Log("Hit " + hit.transform.gameObject.name);

SelectablePlayer hitUnit = hit.gameObject.GetComponent<SelectablePlayer>();

if (hitUnit != null)

{

selectedFriendlyUnit = hitUnit;

}

And just like that, you have selected an object - any object! At this point, if you want to do stuff like show an indicator of being selected, you can add a function to SelectableUnit to handle this, and just call it here using selectedFriendlyUnit.SetSelectionQuadVisible(true) or something like that. If it's on SelectableUnit, you don't need multiple copies of code.

STEP 4: Now that we have an object selected, you can use the same logic to select the enemy you want to attack. You should be able to use the same logic we've used already to select this unit, so I'm going to leave that as an exercise for you.

STEP 5: When it's time to attack, you should now be able to just use selectedFriendlyUnit and selectedEnemyUnit directly.

Does all this make sense?
Click to expand...

much appreciated, REALLY...

panim0_unity · Jan 16, 2020

StarManta said: ↑

A few notes not related to your specific question:

1) Don't used FixedUpdate for anything that's not physics-related. For input specifically, it will cause a lot of problems (as just one example, GetMouseButtonDown may return true for multiple FixedUpdates in a row if these FixedUpdates happen on the same frame.... or it may miss input entirely if your framerate is high enough that FixedUpdate isn't getting run every frame). Besides the input issues, anything in FixedUpdate has the potential to slow your game down exponentially, because the more the game slows down the more times FU needs to be called each frame. A more detailed explanation of this here. Long story short: only used FixedUpdate for physics-related code, period.
2) GameObject tags suck. It's a badly designed, non-extensible system, and if you use it you'll just code yourself into corners. Avoid.

For the actual question:
I was going to write a way to redo your existing logic with a class and an array, but honestly I think the problem isn't really with your data structures but with your algorithm itself. (I mean, having those data structures would help make your algorithm more readable and extensible, but the things you need to do here don't really call for that, IMO.)

STEP 1: First thing I'd do is to make a script that goes on each player and enemy object. If you want, you can use inheritance so that they share a type but also have their own type. You may already have a script like this, and if so, you can use it. And this script doesn't need to actually have any functionality of its own (yet) - here I'm mostly going to use it as fodder for GetComponent and the like. (I find this kind of thing much more useful as a "tag" system than Unity's GameObject.tag system, because A) it allows you to apply multiple tags per object, B) you can find objects tagged in the children of a given object, and C) the tags can use inheritance. The downside is that it's very very slightly slower, but as long as checking for tags isn't something that happens a thousand times per frame this isn't a difference that matters, and the added functionality is more than worth it.)

So let's do this:

Code (csharp):

//each of these in its own script file

public class SelectableUnit : MonoBehaviour {

}

public class SelectablePlayer : SelectableUnit {

}

public class SelectableEnemy : SelectableUnit {

}

Attach the appropriate script of these to each of your enemy and player objects - if your units have multiple parented GameObjects, attach the script to the one that contains the collider that you use for raycasting. (Probably the same GameObject you have tagged as Player1, Player2, etc)

STEP 2: Now let's look at your selection code. You have a lot of different variables, and I think those are all pretty unnecessary. You should be able to boil this down to two things:
1) Which, if any, friendly unit is selected?
2) Which, if any, enemy unit is selected?
Which we can represent as:

Code (csharp):

public SelectablePlayer selectedFriendlyUnit;

public SelectableEnemy selectedEnemyUnit;

STEP 3: Now let's take a look at Update. You do a raycast, good. Now, instead of checking the gameobject's tag, let's use the scripts we attached in step 1, like this:

Code (csharp):

if (Input.GetMouseButtonDown(0)&&canClick==true)

{

RaycastHit hit = new RaycastHit();

bool hitb = Physics.Raycast(Camera.main.ScreenPointToRay(Input.mousePosition), out hit);

if (hitb)

{

Debug.Log("Hit " + hit.transform.gameObject.name);

SelectablePlayer hitUnit = hit.gameObject.GetComponent<SelectablePlayer>();

if (hitUnit != null)

{

selectedFriendlyUnit = hitUnit;

}

And just like that, you have selected an object - any object! At this point, if you want to do stuff like show an indicator of being selected, you can add a function to SelectableUnit to handle this, and just call it here using selectedFriendlyUnit.SetSelectionQuadVisible(true) or something like that. If it's on SelectableUnit, you don't need multiple copies of code.

STEP 4: Now that we have an object selected, you can use the same logic to select the enemy you want to attack. You should be able to use the same logic we've used already to select this unit, so I'm going to leave that as an exercise for you.

STEP 5: When it's time to attack, you should now be able to just use selectedFriendlyUnit and selectedEnemyUnit directly.

Does all this make sense?
Click to expand...

You said I should not use GameObject.tag system but I forgot to mention that this is going to be a multiplayer game so enemy units are somebodys friendly units, I use photon 2 like "if(photon.IsMine) tag=player1" like this. I deleted photon parts in this script, Im really appriciated for your time here but I guess I made you waste it for this so, really sorry about it

Search Unity

Unity ID

Useful Searches

I need guidance please.

panim0_unity

kdgalla

panim0_unity

StarManta

panim0_unity

panim0_unity