Add ProjectileEffect base class and basic functionality #2

Checkroth · 2024-08-17T01:50:38Z

Adds a new Node type and class called ProjectileEffect. Includes the usage of this class in the player and projectile scenes.

Will also include the following projectile effects:

Double laser
Big Laser
Fast Laser

units/player.gd

units/projectile.gd

campenr

🚢 but with some random questions that occured to me while reading the PR

Checkroth · 2024-08-17T04:21:32Z

units/player.gd

+	#    utility functions will get called regardless of its presense in the scene
+	#    tree, as long as thy're in the top-level projectile_effects Array.
+	add_child(double_laser.instantiate())
+	projectile_effects.append(double_laser)


Note:

This is not actually appending anything to the top-level projectile_effects array. Later instances where this is accessed see an empty array for some reason 🤷

Checkroth · 2024-08-18T10:05:06Z

Re-requesting review because the PR has evolved significantly and now is now officially "done"

Checkroth · 2024-08-18T10:08:55Z

projectiles/projectile_effect.gd

+
+# Modify the logic that adds the projectile as a child to the scene.
+# This can be used to change spawn locations, quantities, etc.
+func modify_creation(owner: Node, projectile_effects: Array, transform: Transform2D):


🍷 🧀

I'm pretty sure the resulting type here is "Packed Scene", which currently cannot be typed better

I flip-flopped between the actual node type (ProjectileEffect) and PackedScenes and some other ideas so many times that changing every type annotation was incredibly cumbersome, so I've opted to leave this as just Array until we flesh things out and settle on something that works and won't be changed.

There's also no support for Type Variables -- I figured having a Globals script that contains this type so we can just say var EffectType = Array[PackedScene] and use that so that making a change would be trivial, but can't find a way to do it with the current gdscript version.

💬 Some interesting thoughts in this forum post on accessing more info about types when using PackedScene. A custom resource type is an interesting approach. Nothing for now though, just food for collective thought.

Classic me. No link, and now I can't find it again 🤦

Checkroth · 2024-08-18T10:12:55Z

units/player.gd

-
+	_add_test_effects()
+
+func _add_test_effects():


🍷 🧀

This, an the top-level projectile_effects array is not used (and therefore this function effectively does nothing)

This is for two reasons:

The core node type ProjectileEffect is not used now, see the comment about using PackedScene instead.

Passing just the type ProjectileEffect (or a subclass -- HugeLaser, DoubleLaser, etc.) results in an empty array for a reason I truly have no idea about. Otherwise I would keep the array, use a non-instantiated variable, and then use that instead of the PackedScene

I think ultimately, a combination of ideas might be the best path forward: A Globals script that keeps a list of all our "officially supported" effects as an enum & dictionary of that Enum to a resource path (e.g. res://projectiles/fast_laser.tscn), and then maintain the top level list of just the enum, and offload the loading/instansiating to the projectile scene.

Checkroth · 2024-08-18T10:14:37Z

projectiles/huge_laser.gd

@@ -0,0 +1,2 @@
+class_name HugeLaser
+extends ProjectileEffect


🍷 🧀

This is not actually needed with the current implementation.

It WILL be needed if we want to use the Node class type instead of a PackedScene for effects management. I.e. you can't do HugeLaser.new() unless you define the class_name in a script.
But if you load a PackedScene from the scene path, this is totally superfluous boilerplate.

Checkroth · 2024-08-18T10:16:45Z

units/projectile.gd

+	scale = new_scale
+
+func _ready():
+	if scale != Vector2.ONE:


🍷 🧀

This is a hack to allow the "scale" you set on the Projectile node itself to impact the actual scale of the base projectile.
Without this, setting "Transform -> Scale" on the base projectile node will change the size of the projectile in the 2D visual UI, but at runtime it will be completely ignored. See this godot pr, also linked in the commit message directly.

units/player.gd

campenr · 2024-08-18T19:23:12Z

projectiles/double_laser.gd

+	Projectile = load("res://units/projectile.tscn")
+# Called when the node enters the scene tree for the first time.
+
+# Called every frame. 'delta' is the elapsed time since the previous frame.


campenr

Looks good! I like that adding multiple of the same effect works as expected (i..e super super fast, super super big) 👍 👍 👍

Not going to block. But do we want to consider embracing a clean commit style approach here to make it easier to see how a thing was done start -> (a) finish?.

projectiles/double_laser.gd

campenr · 2024-08-18T19:25:23Z

projectiles/double_laser.gd

+		return not effect.is_double_laser
+
+	# TODO:: Hacky? but const on the node does not work for packed scenes
+	return effect.resource_path != "res://projectiles/double_laser.tscn"


❓ Non-blocking: can we type check the effect? I guess we're loading the scene, but we could probably type check the parent node of the scene to be able to check against a custom class_name? 🤔

projectiles/projectile_effect.gd

campenr · 2024-08-18T19:38:29Z

projectiles/projectile_effect.gd

+
+# Modify the logic that adds the projectile as a child to the scene.
+# This can be used to change spawn locations, quantities, etc.
+func modify_creation(owner: Node, projectile_effects: Array, transform: Transform2D):


💬 Some interesting thoughts in this forum post on accessing more info about types when using PackedScene. A custom resource type is an interesting approach. Nothing for now though, just food for collective thought.

campenr · 2024-08-18T19:41:43Z

units/player.gd

+func _add_test_effects():
+		# Debugging -- these should be added to the list via some user interaction instead.
+	var double_laser = load("res://projectiles/double_laser.tscn")
+	# TODO:: Not even sure if adding to the scene tree here is necessary.


👍 It's necessary for lifecycle things. If we want to listen to signals etc. 👍

units/player.gd

Checkroth · 2024-08-19T11:18:14Z

consider embracing a clean commit style approach here

I thought that's what I was doing 😆 maybe my commits aren't pristine, but I did intend to do it atomically.

There's some refactoring in here that could be squashed in to earlier commits, but it was also a lot of discovery and small changes that were rebase conflict magnets, which I opted to just not deal with

Adds a new Node type and class called ProjectileEffect. Includes the usage of this class in the player and projectile scenes. Does not include any actual projectile effects yet.

This effect adds an identical lazer ouput offset from the original laser. It also makes some modifications to how the firing mechanism and effects dispatch is handled, to make it easier to offload effect mechanism management to each effect.

This adds a projectile effect that makes lasers huge. It includes refactoring to the projectile effect engine to allow effects to directly influence the parent node's size

Adds a workaround that manually scales the children of the Projectile scene. This is due to a problem where the projectile root node scale does not propagate down to the sprite or collision shape within it. godotengine/godot#5734

This refactors the projectile effect system to use packed scenes instead of the base class node type ProjectileEffect. This is due to the requirement for these scenes to be instantiated for each projectile, not a shared resource. Without this change, effects that modify the scene tree (such as HUGE LASER) do not work.

Adds a new effect that hooks in to the physics system to make lasers go pewpew faser

This moves projectiles to a new collision layer, and adds that layer to the mask for enemies. This is to do two things: (1) Prevent lasers from colliding with themselves ("Huge" and "Double" lasers can overlap), (2) allow enemies to queue_free themselves, instead of leaving that burden on the projectile

campenr · 2024-08-20T03:54:41Z

units/projectile.tscn

@@ -8,6 +8,7 @@ radius = 4.0

 [node name="Projectile" type="Area2D"]
 position = Vector2(68, 1)
+collision_layer = 2


🤔 I kinda like the idea that projectiles can hit themselves / the player... but that'll take some thinking. For now 👍 👍

Not a terrible idea, though the huge double lasers overlapped hitboxes, so they would just immediately disappear

campenr

LGTM!!! 🚢 🚢 🚢 🚢

campenr reviewed Aug 17, 2024

View reviewed changes

units/player.gd Outdated Show resolved Hide resolved

campenr reviewed Aug 17, 2024

View reviewed changes

units/projectile.gd Show resolved Hide resolved

campenr approved these changes Aug 17, 2024

View reviewed changes

Checkroth commented Aug 17, 2024

View reviewed changes

Checkroth force-pushed the feature/projectile-effects branch 3 times, most recently from 4398d3a to 066cad0 Compare August 18, 2024 06:12

Checkroth requested a review from campenr August 18, 2024 10:04

Checkroth commented Aug 18, 2024

View reviewed changes

units/player.gd Show resolved Hide resolved

campenr reviewed Aug 18, 2024

View reviewed changes

campenr approved these changes Aug 18, 2024

View reviewed changes

Checkroth force-pushed the feature/projectile-effects branch from f264910 to 45bc4c8 Compare August 19, 2024 11:27

Checkroth added 7 commits August 19, 2024 20:28

Add ProjectileEffect base class and basic functionality

3880288

Adds a new Node type and class called ProjectileEffect. Includes the usage of this class in the player and projectile scenes. Does not include any actual projectile effects yet.

Add a double laser projectile effect.

ccca142

This effect adds an identical lazer ouput offset from the original laser. It also makes some modifications to how the firing mechanism and effects dispatch is handled, to make it easier to offload effect mechanism management to each effect.

Add Huge Laser projectile effect

756ecd3

This adds a projectile effect that makes lasers huge. It includes refactoring to the projectile effect engine to allow effects to directly influence the parent node's size

Add projectile scaling workaround

6090e51

Adds a workaround that manually scales the children of the Projectile scene. This is due to a problem where the projectile root node scale does not propagate down to the sprite or collision shape within it. godotengine/godot#5734

Add Fast Laser Effect

f08db8e

Adds a new effect that hooks in to the physics system to make lasers go pewpew faser

Checkroth force-pushed the feature/projectile-effects branch from 45bc4c8 to 82f3f7f Compare August 19, 2024 12:02

Checkroth requested a review from campenr August 19, 2024 12:02

Checkroth added the merge-when-approved label Aug 19, 2024

campenr reviewed Aug 20, 2024

View reviewed changes

campenr approved these changes Aug 20, 2024

View reviewed changes

campenr merged commit f2cd26f into main Aug 20, 2024

campenr deleted the feature/projectile-effects branch August 20, 2024 03:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ProjectileEffect base class and basic functionality #2

Add ProjectileEffect base class and basic functionality #2

Checkroth commented Aug 17, 2024

campenr left a comment

Checkroth Aug 17, 2024

Checkroth commented Aug 18, 2024

Checkroth Aug 18, 2024

campenr Aug 18, 2024

campenr Aug 19, 2024

Checkroth Aug 18, 2024 •

edited

Loading

Checkroth Aug 18, 2024

Checkroth Aug 18, 2024

campenr Aug 18, 2024

campenr left a comment

campenr Aug 18, 2024

campenr Aug 18, 2024

campenr Aug 18, 2024

Checkroth commented Aug 19, 2024

campenr Aug 20, 2024

Checkroth Aug 21, 2024

campenr left a comment

Add ProjectileEffect base class and basic functionality #2

Add ProjectileEffect base class and basic functionality #2

Conversation

Checkroth commented Aug 17, 2024

campenr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Checkroth commented Aug 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Checkroth Aug 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

campenr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Checkroth commented Aug 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

campenr left a comment

Choose a reason for hiding this comment

Checkroth Aug 18, 2024 •

edited

Loading