Question Projectiles pierce fork and chain

Hello, Everyone!

I've made a small script which allows projectiles to interact with objects (future enemies) by piercing, forking and chaining. The example is in the GIF below:


The fact is that I'm a pretty new in programming (not even sure that I have a junior level) but I want to be unity developer in the future. That's why I need your help. Can you review my code please? What's can be done better or another way? Right now I'm sure that my code is bad but I don't know wthat the exact problem because my expierence in programming is too low.

Code (CSharp):

1. public class ProjBehaviour : MonoBehaviour
2. {
3.     public int pierceRemaining;
4.     public int forkRemaining;
5.     public float forkRotationDegree = 45.0f;
6.     public int chainRemaining;
7.     public GameObject objectToIgnoreCollision;
8.     public GameObject projectilePrefab;
9.     public List<GameObject> objectsToIgnoreChaining = new List<GameObject>();
10.  
11.     public void InitProjectile(int pierceRemaining, int forkRemaining, int chainRemaining, GameObject objectToIgnoreCollision, GameObject projectilePrefab)
12.     {
13.         // Simulates regular C# constructor for the new projectiles
14.         this.pierceRemaining = pierceRemaining;
15.         this.forkRemaining = forkRemaining;
16.         this.chainRemaining = chainRemaining;
17.         this.objectToIgnoreCollision = objectToIgnoreCollision;
18.         if (objectToIgnoreCollision != null)
19.         {
20.             // Disable collision detection with enemy it's already was collided
21.             Physics.IgnoreCollision(GetComponent<Collider>(), objectToIgnoreCollision.GetComponent<Collider>());
22.         }
23.  
24.         this.projectilePrefab = projectilePrefab;
25.     }
26.  
27.     public void InitProjectile(int pierceRemaining, int forkRemaining, int chainRemaining, GameObject objectToIgnoreCollision, GameObject projectilePrefab, List<GameObject> objectsToIgnoreChaining)
28.     {
29.         // Simulates regular C# constructor for the new projectiles
30.         this.pierceRemaining = pierceRemaining;
31.         this.forkRemaining = forkRemaining;
32.         this.chainRemaining = chainRemaining;
33.         this.objectToIgnoreCollision = objectToIgnoreCollision;
34.         if (objectToIgnoreCollision != null)
35.         {
36.             // Disable collision detection with enemy it's already was collided
37.             Physics.IgnoreCollision(GetComponent<Collider>(), objectToIgnoreCollision.GetComponent<Collider>());
38.         }
39.         this.projectilePrefab = projectilePrefab;
40.         this.objectsToIgnoreChaining.AddRange(objectsToIgnoreChaining);
41.     }
42.  
43.     private void OnTriggerEnter(Collider other)
44.     {
45.         if (other.tag == "Projectile" || other.tag == "Player")
46.             return;
47.         OnProjectileHit(other);
48.      
49.     }
50.  
51.     private void OnProjectileHit(Collider other)
52.     {
53.         if (pierceRemaining >= 1)
54.         {
55.             OnProjectilePierce(other);
56.         }
57.  
58.         if (forkRemaining >= 1)
59.         {
60.             OnProjectileFork(other);
61.         }
62.  
63.         if (chainRemaining >= 1)
64.         {
65.             OnProjectileChain(other);
66.         }
67.        
68.         if (other != objectToIgnoreCollision)
69.         {
70.             Destroy(gameObject);
71.         }
72.     }
73.  
74.     private void OnProjectilePierce(Collider other)
75.     {
76.         GameObject childProj = Instantiate(projectilePrefab, transform.position, transform.rotation);
77.         childProj.GetComponent<ProjBehaviour>().InitProjectile(--pierceRemaining, forkRemaining, chainRemaining, other.gameObject, projectilePrefab);
78.  
79.         //Debug by color
80.         childProj.GetComponent<MeshRenderer>().material.color = Color.red;
81.     }
82.  
83.     private void OnProjectileFork(Collider other)
84.     {
85.         forkRemaining--;
86.         GameObject childProjLeft = Instantiate(projectilePrefab, transform.position, transform.rotation);
87.         GameObject childProjRight = Instantiate(projectilePrefab, transform.position, transform.rotation);
88.  
89.         childProjLeft.GetComponent<ProjBehaviour>().InitProjectile(pierceRemaining, forkRemaining, chainRemaining, other.gameObject, projectilePrefab);
90.         childProjRight.GetComponent<ProjBehaviour>().InitProjectile(pierceRemaining, forkRemaining, chainRemaining, other.gameObject, projectilePrefab);
91.  
92.         childProjLeft.transform.Rotate(new Vector3(transform.rotation.x, transform.rotation.x + forkRotationDegree, transform.rotation.z));
93.         childProjRight.transform.Rotate(new Vector3(transform.rotation.x, transform.rotation.x - forkRotationDegree, transform.rotation.z));
94.  
95.         //Debug by color
96.         childProjLeft.GetComponent<MeshRenderer>().material.color = Color.blue;
97.         childProjRight.GetComponent<MeshRenderer>().material.color = Color.blue;
98.     }
99.  
100.     private void OnProjectileChain(Collider other)
101.     {
102.         GameObject childProj = Instantiate(projectilePrefab, transform.position, transform.rotation);
103.         objectsToIgnoreChaining.Add(other.gameObject);
104.         childProj.GetComponent<ProjBehaviour>().InitProjectile(pierceRemaining, forkRemaining, --chainRemaining, other.gameObject, projectilePrefab, objectsToIgnoreChaining);
105.  
106.         Vector3 enemyPosition = GetClosestEnemyPositionForProjectile(childProj);
107.         if (enemyPosition != null)
108.             childProj.transform.LookAt(enemyPosition, Vector3.up);
109.      
110.         // Debug by color
111.         childProj.GetComponent<MeshRenderer>().material.color = Color.yellow;
112.     }
113.  
114.     private Vector3 GetClosestEnemyPositionForProjectile(GameObject projectile)
115.     {
116.         GameObject[] enemies = GameObject.FindGameObjectsWithTag("Enemy");
117.         GameObject closestEnemy = null;
118.         //ProjBehaviour projBehaviour = projectile.GetComponent<ProjBehaviour>();
119.         float currDistance = Mathf.Infinity;
120.  
121.         foreach (var enemy in enemies)
122.         {
123.             float distanceToEnemy = Vector3.Distance(transform.position, enemy.transform.position);
124.                    
125.             if (distanceToEnemy < currDistance && !objectsToIgnoreChaining.Contains(enemy))
126.             {
127.                 currDistance = distanceToEnemy;
128.                 closestEnemy = enemy;
129.             }
130.         }
131.  
132.         objectsToIgnoreChaining.Add(closestEnemy);
133.  
134.         if (closestEnemy)
135.         {
136.             return closestEnemy.transform.position;
137.         }
138.         else
139.         {
140.             // Destroying projectile if there are no targets left. Owerwhise it's working like a pierce.
141.             Destroy(projectile);
142.             return projectile.transform.position;
143.         }
144.     }
145. }

Thank you in advance!

P.S: Please, let me know if it's not a right place to post the questions like this, because I'm new at this forum.

 

Code (CSharp):

1. childProjLeft.transform.Rotate(new Vector3(transform.rotation.x, transform.rotation.x + forkRotationDegree, transform.rotation.z));
2. childProjRight.transform.Rotate(new Vector3(transform.rotation.x, transform.rotation.x - forkRotationDegree, transform.rotation.z));

This code is definitely not correct. Transform.rotation is a Quaternion. You're treating the x y and z components of the Quaternion as Euler angles, but that is not correct. You should read up on Quaternions, because those values are not what you are expecting them to be.

 

I am a strong defender of the single-responsibility principle and your code does a lot of different jobs at once. In programming, the principle "divide and conquer" is very important, dividing a problem into smaller problems, solving these little problems and then therefore solving the original problem.

Casually looking through your code, these are some of the things that are handled by your single component:

• Does the projectile hit something?
  
• If the projectile hits something, should the projectile ignore the object or confirm the hit?
  
• If the projectile hits something, should it pierce and if yes, do the piercing
  
• If the projectile hits something, should it fork and if yes, do the forking
• If the projectile hits something, should it chain and if yes, do the chaining
• Get the closest enemy position for this projectile

That's a lot for a single object, for a single component.

Think about how different projectiles could look like, some may fork, but not chain, others may pierce, but not fork and so on. All three behaviours, pierce, fork and chain, are unique behaviours, thus they could be their own MonoBehaviour components, allowing you to piece your different projectiles together, as the GameObject represents the projectile, defined by the components.

This would reduce the responsibilities of each component to:

• Does the projectile hit something?
  
• If the projectile hits something, should the projectile ignore the object or confirm the hit?
  
• If the projectile hits something, should it execute its unique behaviour?
  
• Get the closest enemy position for this projectile

If you want to get fancy, you can move the check for the closest enemy position into a so-called service. A service provides a, well, service to other objects, such as giving you the closest enemy to a position of another object. The huge advantage is that you don't have to copypaste the function if you need it elsewhere and you have one single place with that logic for every other object.

It might be overkill, but you could go even further and write one component that checks for collision, allowing other components to listen to it with an event. Another component listening to it would then handle whether it should confirm the collision or ignore it, providing yet another event to listen to, which the unique behaviours of your projectile, pierce, chain and fork then listen to and are notified if a valid collision has occured.

 

Thank you very much for this! It's the most helpful advice I've ever got.
I've spend a few hours at code refractoring the and got the following:

Code (CSharp):

1. public class ProjOnHitPublisher : MonoBehaviour
2. {
3.     public delegate void onHitHandler(Collider collider);
4.     public event onHitHandler OnHitEvents;
5.  
6.     private void OnTriggerEnter(Collider other)
7.     {
8.         OnHitEvents(other);
9.     }
10. }

Code (CSharp):

1. public class ProjOnHitBehaviourPublisher : MonoBehaviour
2. {
3.     public delegate void onConfirmedHitHandler(Collider collider);
4.     public event onConfirmedHitHandler OnConfirmedHitEvent;
5.  
6.     public List<string> tagsToIgnore = new List<string>() { "Projectile", "Player" };
7.     private ProjStats projBehaviourRef;
8.  
9.     private void Start()
10.     {
11.         var projOnHitPblisher = GetComponent<ProjOnHitPublisher>();
12.         projOnHitPblisher.OnHitEvents += OnProjectileHitHandler;
13.  
14.         projBehaviourRef = GetComponent<ProjStats>();
15.     }
16.  
17.     private void OnProjectileHitHandler(Collider other)
18.     {
19.         if (!tagsToIgnore.Contains(other.tag) && other != projBehaviourRef.objectToIgnoreCollision)
20.         {
21.             OnConfirmedHitEvent(other);
22.         }
23.     }
24. }

Code (CSharp):

1. public class ProjectileChain : MonoBehaviour
2. {
3.     private ProjStats projStats;
4.     private ProjOnHitBehaviourPublisher projOnHitBehaviourPublisher;
5.  
6.     private void Start()
7.     {
8.         projStats = GetComponent<ProjStats>();
9.         projOnHitBehaviourPublisher = GetComponent<ProjOnHitBehaviourPublisher>();
10.         projOnHitBehaviourPublisher.OnConfirmedHitEvent += OnProjectilePierceHandler;
11.     }
12.  
13.     private void OnProjectilePierceHandler(Collider other)
14.     {
15.         if (projStats.chainRemaining >=1)
16.         {
17.             GameObject childProj = Instantiate(projStats.projectilePrefab, transform.position, transform.rotation);
18.             projStats.objectsToIgnoreChaining.Add(other.gameObject);
19.             childProj.GetComponent<ProjStats>().InitProjectile(projStats.pierceRemaining, projStats.forkRemaining, --projStats.chainRemaining, other.gameObject, projStats.projectilePrefab, projStats.objectsToIgnoreChaining);
20.  
21.             GameObject closestEnemy = ObjectFinder.FindClosetTargetByTag(childProj, "Enemy", childProj.GetComponent<ProjStats>().objectsToIgnoreChaining);
22.             if (closestEnemy)
23.                 childProj.transform.LookAt(closestEnemy.transform.position, Vector3.up);
24.  
25.             // Debug by color.
26.             childProj.GetComponent<MeshRenderer>().material.color = Color.yellow;
27.         }
28.        
29.         Destroy(gameObject);
30.     }
31. }

Code (CSharp):

1. public class ProjStats : MonoBehaviour
2. {
3.     public int pierceRemaining;
4.     public int forkRemaining;
5.     public float forkRotationDegree = 45.0f;
6.     public int chainRemaining;
7.  
8.     public GameObject objectToIgnoreCollision;
9.     public GameObject projectilePrefab;
10.     public List<GameObject> objectsToIgnoreChaining = new List<GameObject>();
11.  
12.     public void InitProjectile(int pierceRemaining, int forkRemaining, int chainRemaining, GameObject objectToIgnoreCollision, GameObject projectilePrefab)
13.     {
14.         // Simulates regular C# constructor for the new projectiles
15.         this.pierceRemaining = pierceRemaining;
16.         this.forkRemaining = forkRemaining;
17.         this.chainRemaining = chainRemaining;
18.         this.objectToIgnoreCollision = objectToIgnoreCollision;
19.         if (objectToIgnoreCollision != null)
20.         {
21.             // Disable collision detection with enemy it's already was collided
22.             Physics.IgnoreCollision(GetComponent<Collider>(), objectToIgnoreCollision.GetComponent<Collider>());
23.         }
24.         this.projectilePrefab = projectilePrefab;
25.     }
26.  
27.     public void InitProjectile(int pierceRemaining, int forkRemaining, int chainRemaining, GameObject objectToIgnoreCollision, GameObject projectilePrefab, List<GameObject> objectsToIgnoreChaining)
28.     {
29.         // Simulates regular C# constructor for the new projectiles
30.         this.pierceRemaining = pierceRemaining;
31.         this.forkRemaining = forkRemaining;
32.         this.chainRemaining = chainRemaining;
33.         this.objectToIgnoreCollision = objectToIgnoreCollision;
34.         if (objectToIgnoreCollision != null)
35.         {
36.             // Disable collision detection with enemy it's already was collided
37.             Physics.IgnoreCollision(GetComponent<Collider>(), objectToIgnoreCollision.GetComponent<Collider>());
38.         }
39.         this.projectilePrefab = projectilePrefab;
40.         this.objectsToIgnoreChaining.AddRange(objectsToIgnoreChaining);
41.     }
42.  
43. }

Now the most of components are responsible for single in-game function and it makes the future code management much easier than before. Nevertheless some minor problems (like huge number of components in inspector) are still left but now I have some thoughts how to deal with them.

Probably I would try to implement an Interface for all of "behaviour" components like forking, piercing etc. and store them in array inside new component which also can subscribe all of them to ProjOnHitBehaviourPublisher.

Also some arguments became larger because I have to use GetComponent<> to acess variables from other scripts. But theese problems are nothing versus benefits I've got

My best thanks to you, Sir!

 

Thank you.

Changed this part to:

Code (CSharp):

1. Quaternion rightRotation = Quaternion.Euler(transform.rotation.x, transform.rotation.y + projStats.forkRotationDegree, transform.rotation.z);
2.             Quaternion leftRotation = Quaternion.Euler(transform.rotation.x, transform.rotation.y - projStats.forkRotationDegree, transform.rotation.z);
3.  
4.             GameObject childProjLeft = Instantiate(projStats.projectilePrefab, transform.position, rightRotation);
5.             GameObject childProjRight = Instantiate(projStats.projectilePrefab, transform.position, leftRotation);

The strange part is that I didn't notice any difference despite that fact that previous code had

transform.rotation.x - forkRotationDegree

on Y-axis

The forking stuff is still working as I planned.

 

I see that you use vanilla C# events, in Unity, we get a solution to events with UnityEvent. The huge advantage of using the Unity event over the C# event is that we can use the component inspector to manually subscribe listeners to a publisher and serialize that relationship, which also makes debugging easier. This eliminates the need for doing it via code as you do it in various

Start()

methods of your components.

 

This is not what you think it is. Quaternions have x,y,z,w internal components and they are NOT angles or anything like them. In fact there is almost never a reason to care about those internal values.

If you're looking for the degrees rotated around the X axis, that could be found in

transform.eulerAngles.x

, but in a general case you do not want to read those values. The reason not to read them is because they can change in unexpected "gimbal-locky" ways when any axis of rotation goes past 90-degree increments.

Always best to track your own angles and drive them explicitly.

Code (csharp):

1. private float MyAngle;

And to rotate around the X axis at 100 degrees per second:

Code (csharp):

1. MyAngle += 100 * Time.deltaTime;
2. transform.rotation = Quaternion.Euler( MyAngle, 0, 0);

 

Thank you!

I read a bit about UnityEvents and tried to implement them instead vanilla ones. The problem is that my projectile's behaivour scripts are all have the same signature on their "on hit" method like:

private void OnProjectilePierceHandler(Collider other)

It means that I should use UnityEvent<T> which is not serrializable in inspector.
Making custom serrializable class which inherits from UnityEvent<T> might be a sulotion but I'm not sure that it's a good solution.
On the other hand, without serrialization I still need subscribe listeners throught Start() method and almost everything is left the same.

Can you clarify this moment, please? Is making a custom class for every generic UnityEvent a common practice in Unity?

 

In newer versions of Unity, the engine supports the serialization of generic classes under certain circumstances, so we no longer have to inherit into non-generic child classes. This includes UnityEvent<T>, although I think that it was always supported in the editor, similar to List<T> and T[].

With

using UnityEngine.Events

, you can write this:

Code (CSharp):

1. public class CollisionProvider : MonoBehaviour
2. {
3.     [SerializeField]
4.     private UnityEvent<Collider> myEvent;
5. }

Code (CSharp):

1. public class CollisionListener : MonoBehaviour
2. {
3.     public void OnCollision(Collider other)
4.     {
5.         //...
6.     }
7. }

Then, in the editor, you can link them together:


Whenever

myEvent

in

ColliderProvider

is triggered, it then runs

OnCollision

of the listener, as you have serialized the relationship between the two.