Question Best way to handle projectiles (top down scrolling shooter)

Game example:

Summary:
I'm at a point in my game where I can't really wrap my head around how to handle projectiles. After refactoring most everything else using composition in C#, I'm not sure how to achieve the functionality I want (or what topics to search for to do that).

My current system can be summarized in this way:

- I use the namespace Player to represent classes associated with the player

- This namespace currently consists of the static classes Attributes and State, as well as Monobehaviour classes Combat, Defense, and Movement.

- All movement input is taken from the new input system using an input action asset and player input component and is processed in the Movement class:

Code (CSharp):

1.     public class Movement : MonoBehaviour
2.     {
3.  
4.         public void OnMove(InputAction.CallbackContext context)
5.         {
6.             if (context.started)
7.             {
8.                 State.IsMoving = true;
9.                 //print("MOVEMENT: Started");
10.             }
11.             else if (context.performed)
12.             {
13.                 var direction = context.ReadValue<Vector2>();
14.                 State.Direction = new Vector3(direction.x, direction.y, 0f);
15.             }
16.             else if (context.canceled)
17.             {
18.                 State.IsMoving = false;
19.                 //print("MOVEMENT: Ended");
20.             }
21.         }
22.  
23.         private void Move(Vector3 direction, Vector3 boundary, float speed)
24.         {
25.             // boundary calculations
26.             if ((transform.position.y >= boundary.y)&& (direction.y > 0f))
27.                 direction.y = 0f;
28.             if ((transform.position.y <= 0) && (direction.y < 0f))
29.                 direction.y = 0f;
30.             if ((transform.position.x >= boundary.x) && (direction.x > 0f))
31.                 direction.x = 0f;
32.             if ((transform.position.x <= -boundary.x) && (direction.x < 0f))
33.                 direction.x = 0f;
34.             // movement
35.             transform.position += direction * Time.deltaTime * speed;
36.         }
37.  
38.         private void OnTriggerEnter(Collider collider)
39.         {
40.             if (collider.gameObject.tag == "Boundary")
41.             State.SetBounds(collider.bounds.extents);
42.         }
43.  
44.         private void Update()
45.         {
46.             if (!State.IsMoving) return;
47.             Move(State.Direction, State.Boundary, Attributes.Speed);
48.         }
49.     }

- All defensive input is handled in the Defense class, which currently consists of two methods: OnShield() and Shield(). OnShield() processes the right mouse click, which enables a shield object on started event, and disables it on canceled event. So holding down the right mouse click will keep the shield up, and releasing will disable it:

Code (CSharp):

1.     public class Defense : MonoBehaviour
2.     {
3.         private GameObject Shield;
4.  
5.         private void Awake()
6.         {
7.             Shield = transform.GetChild(0).gameObject;
8.             Shield.SetActive(false);
9.         }
10.  
11.         public void OnShield(InputAction.CallbackContext context)
12.         {
13.             if (context.started)
14.             {
15.                 Shield.SetActive(true);
16.                 State.IsShielded = true;
17.                 print("DEFENSE: Activated");
18.             }
19.             else if (context.canceled)
20.             {
21.                 Shield.SetActive(false);
22.                 State.IsShielded = false;
23.                 print("DEFENSE: Deactivated");
24.             }
25.         }
26.     }

- The class Combat is handled in much the same way, with two methods OnShoot() and Shoot(), but these are set to the left mouse click. This allows the player to initiate a charging phase on left mouse click, then initiate the shot on left mouse release:

Code (CSharp):

1.         public GameObject Projectile;
2.         public void OnShoot(InputAction.CallbackContext context)
3.         {
4.             if (context.started)
5.             {
6.                 State.IsCharging = true;
7.                 //print("COMBAT: Charging");
8.             }
9.             if (context.canceled)
10.             {
11.                 State.IsCharging = false;
12.                 Shoot();
13.                 //print("COMBAT: Shooting");
14.             }
15.         }
16.  
17.         private void Shoot()
18.         {
19.             Instantiate(Projectile, transform.position, Quaternion.identity);
20.         }
21.     }

The Problem

As you can see, shooting instantiates a projectile prefab that is referenced in script via the inspector. I need this projectile to damage the enemy based on values in the static class Attributes. The Projectile namespace currently consists of one class Dart, which will represent a projectile weapon that increases in speed the longer you charge it (left mouse hold). I want this weapon to be able to be cycled based upon what is currently active in the Player.State class. Not only that, I want to be able to use the charging mechanism provided by Player.Combat.OnShoot() to alter the values of the damage to the enemy, as well as other things like speed of the projectile. I believe that I could achieve this with coroutines. But the former issue of communicating these values between the Player namespace classes and an instantiated projectile object which uses a Monobehaviour class is where things get really murky for me because projectiles may or may not always be instantiated by the player... meaning, most will probably be instantiated by enemies AT the player so I will need to implement some kind of way to establish ownership of the projectile itself.

I'm just not sure what I should be looking for here to implement projectile combat... whether it be using Unity events, or some kind of game manager class or combat manager class, or if I should be instantiating the projectile class each shot rather than just utilizing its inherited Monobehaviour methods. Is my design pattern flawed? I'm just not sure. Any ideas?

Also, I'd appreciate any other feedback on my general approach and coding ie. if you see any code smells or dumb things I'm doing lol

 

Why don't you create a general "Bullet" class? From that, you derive "LaserBeam", "Regular" and other special types of bullets you might wish to have.

When you Shoot(), you want to instantiate a bullet based on the weapon you have equipped. That's quite straightforward. Just store a dictionary of related weapons and bullets somewhere.
For example:
Laser - LaserBeam
MachineGun - Regular
Then, reference that dictionary with the weapon you have equipped.

Instantiate(bulletsByWeapons[Player.State.equpiedWeapon.type], ...)

That way, you will be spawning a proper bullet type.

Now, you might want to keep the "Damage" variable on your weapon, since, it really does depend on the weapon you are using. When you Instantiate your bullet, use GetComponent<Bullet> from it and set it's "Damage" variable to the same value as your weapon. For example:

Code (CSharp):

1. var tempGo = Instantiate(bulletsByWeapons[Player.State.equpiedWeapon.type], ...);
2. Bullet bullet = tempGo.GetComponent<Bullet>();
3. bullet.Damage = Player.State.equpiedWeapon.Damage;

When your bullet hits, you have the information on how much damage it should deal.

====
For the actual movement of your projectile, that's your own decision. You could either use Update() or a coroutine, either one will do the job and should be fine either way.

===
When you say, projectiles may or may not be always instantiated by the player; does it really matter?
When you create a bullet, set its tag (or whatever) based on the parent that shot it.

Code (CSharp):

1. var tempGo = Instantiate(bulletsByWeapons[Player.State.equpiedWeapon.type], ...);
2. Bullet bullet = tempGo.GetComponent<Bullet>();
3. bullet.Damage = Player.State.equpiedWeapon.Damage;
4. bullet.Tag = "Player";

That way, when the bullet hits something, you know if the player shot it or something else

===
Take the opportunity of Instantiating the bullet to fill it with as much information as it needs for the rest of its lifetime. You probably won't have another opportunity.


===========================
Now, as for the code: I love how you're actually thinking about this and not just winging it as a lot of people do. Here are some things that I believe you could improve.

1)

Shield = transform.GetChild(0).gameObject;

- Don't use GetChild(), almost ever. Seriously. There is no guarantee that your hierarchy will remain unchanged for the entire duration of your project.
- Instead, just reference what you need from the inspector

2)

if (collider.gameObject.tag == "Boundary")

- Could be replaced with collider.gameObject.CompareTag()
- https://docs.unity3d.com/ScriptReference/Component.CompareTag.html
- It's a bit better for performance and a lot for readable.

3)

Code (CSharp):

1. if (collider.gameObject.tag == "Boundary")
2. State.SetBounds(collider.bounds.extents);

- Don't do this. Single line if's are understandable (even though I hate them personally), but always indent your code.

4)

Code (CSharp):

1. Shield.SetActive(false);
2. State.IsShielded = false;

- Should be a method.

5)

public GameObject Projectile;

- Should probably be replaced with

[SerializeField] private GameObject Projectile = default;

- If you are only using public to reveal something in the inspector, just use [SerializeField] instead. It will save you hours of headaches later.

 

Thank you so much for your post. I'm going through it bit by bit (I need to learn dictionaries now lol). I've since refactored it some and implemented an event system so there are some extra things to consider, but your suggestions are like little golden nuggets of info that are so hard to find just by wading through the swamp of tutorial hell. And yeah I agree with you on the if statement one-liner indentation thing. Pretty sure I just did that to get around the word wrap here in the forums, but I've since implemented more statements there so its a code block now anyways and I will keep your suggestion in mind in the future.

This piqued my curiosity. I'm wondering why you suggested these two tiny little lines should be their own function. Is it because they are both relying on the same boolean value, and that could be consolidated into one and delivered as a parameter? (not a how question, more like a why question).

 

Not my code but if I had to guess at @Dextozz reasoning, it is because generally if two things have to happen together (your notion of the .IsShielded boolean and the Unity-facing status of the shield), wrap them up so that it won't be possible to forget one or the other in the future.

 

Yeup, as always, @Kurt-Dekker is correct