-
Notifications
You must be signed in to change notification settings - Fork 117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Impact Attribute Damage #109
Merged
Merged
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
ecbb043
Impact Attribute Damage
ac-arcana b345a73
Merge branch 'Phazorknight:main' into main
ac-arcana d4fc802
Update ImpactAttributeDamage.gd
ac-arcana 2a0f729
Merge branch 'Phazorknight:main' into main
ac-arcana 999db2c
Update ImpactAttributeDamage.gd
ac-arcana ff98d3f
Merge branch 'Phazorknight:main' into main
ac-arcana 6690e80
Update ImpactAttributeDamage.gd
ac-arcana File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
extends Node3D | ||
class_name ImpactAttributeDamage | ||
# Attached to a RigidBody, this components will will damage a CogitoAttribute when the rigidbody collides with something. | ||
@onready var parent_node = get_parent() | ||
|
||
## This is what will take damage | ||
@export var attribute : CogitoAttribute | ||
## The minimum velocity at time of impact to take damage. This prevents light hits from damageing the attribute | ||
@export var minimum_velocity : float = 1.0 | ||
## How much damage to do to the attribute | ||
@export var damage : int | ||
## Forced delay between impact times (in seconds). | ||
@export var next_impact_time : float = 0.3 | ||
|
||
var time_passed : float | ||
|
||
# Called when the node enters the scene tree for the first time. | ||
func _ready() -> void: | ||
if !parent_node.is_class("RigidBody3D"): | ||
print("ImpactDamage: ", parent_node, " needs to be RigidBody3D.") | ||
else: | ||
parent_node.body_entered.connect(_on_parent_node_collision) | ||
|
||
|
||
func _on_parent_node_collision(_collided_node): | ||
if parent_node.linear_velocity.length() > minimum_velocity and time_passed == 0: | ||
attribute.subtract(damage) | ||
time_passed = next_impact_time | ||
|
||
|
||
func _physics_process(delta: float) -> void: | ||
if time_passed > 0: | ||
time_passed -= delta | ||
else: | ||
time_passed = 0 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
[gd_scene load_steps=2 format=3 uid="uid://b5cjuw2emh3wq"] | ||
|
||
[ext_resource type="Script" path="res://COGITO/Components/ImpactAttributeDamage.gd" id="1_lfg44"] | ||
|
||
[node name="ImpactAttributeDamage" type="Node3D"] | ||
script = ExtResource("1_lfg44") | ||
damage = 1 |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, maybe you can replace
time_passed
with Godot's Timer functionality so this class doesn't need to track time. Can create and store a oneshot Timer dynamically in code on the _ready, and you can then reuse the single reference to the timer.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I basically copy pasted that from the impact sounds component. Is there an advantage to creating a whole new node for such a simple timer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use SceneTreeTimer
It does not create a new node.
The advantage is really that there's not a duplication of time tracking in the backend. Godot pools timers together and ticks them together, as well it also standardizes timer tracking to make it easier to hook with other scripts. So it saves a cycle or so on _physics_process
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't aware of SceneTreeTimer and I am just taking a look at it. I see that it does not create a node. However, it does still create a timer which is automatically freed once the time is elapsed. On top of this, I would probably need to store a boolean to track whether the timer has completed or not.
Isn't the timer still going to result in GC when it is freed? I'm used to thinking in C# terms, so this might not be the case for gdscript?
Maybe I'm missing a way to keep a reference to it once it has timed out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, I don't think you're thinking of it wrong. I originally had a Timer node/resource in mind, so if you're using SceneTreeTimer, scratch the reusing reference thing.
The main thing that strikes me as making it worthwhile to add a SceneTreeTimer is that currently every ImpactAttribute component in a scene will be running cycles on _physics_process at all times to check if it should decrease its timer.
With using SceneTreeTimer, only ImpactAttribute components that have recently made an impact (and thusly, a timer is actually relevant) will be tracking time in an already existing section of Godot's backend that tracks time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an extremely minor performance optimization BTW, I don't think that this is a necessary change at all, nor will really be felt by anyone. I was just throwing in this tidbit as a thing to be aware of overall.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm starting to get what you're suggesting now. If there is no physics process function at all checking a timer it will save a little bit of performance on these objects every frame.
So do you think it would be better to use SceneTreeTimer? Or would making and keeping a Timer node be better as it would prevent allocating a new timer and resulting GC for each collision?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a SceneTreeTimer is just neater if it's purely code based, given that the Timer node requires some configuration to make it one shot etc.