Impact Attribute Damage #109
Conversation
Deal damage to an attribute when a rigid body impacts something else
| @export var next_impact_time : float = 0.3 | ||
|
|
||
| var time_passed : float |
There was a problem hiding this comment.
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.
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.
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.
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.
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.
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.
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.
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.
|
I do have a performance concern with using length on every impact. |
For this sort of thing length_squared is all you need. The post-timer check would also be nice as you say |
Made the timer check first, and switched to length_squared to improve performance.
|
I have updated to use length_squared and to check the timer first. I still would like to discuss SceneTreeTimer more as I feel like I still may be missing something about it. See my comment above. |
|
Hey y'all, just wanted to say I'm not ignoring this PR, just saw that there were still changes happening and was looking into other stuff. I see the issue with checking a float in the |
|
Okay, so I just pushed an update to ImpactSounds.gd in 52612a4 This changes it to use SceneTree timer instead of the |
Okay, well... I added 680+ health potions to one scene all colliding with each other. I then tried old and new methods in the profiler as fairly as I could OLD profiled, peaked script function time: NEW profiled, peaked script function time: New technique I would say is more efficient. Though lots of SceneTreeTimer initializations at once will slow things down -- see Basically, if you do have lots of things constantly colliding, then it might be worth considering the old way, but for MOST immersive sims, the new way probably makes the most sense. :P Ultimately it's not really saving that much as you can see, considering what it took to get here, but hey, fun to experiment 😆 |
|
Thanks for doing the analytics like that. It's nice to see the data. I appreciate getting to learn from each other in this way. I also find it super interesting to see how much _on_parent_node_collision_new jumped up in time. |
Changed timing method
|
Updated with the new timing method. I think this is ready now :) |
Changed damage from int to float
|
Good catch, I didn't realize the attribute is a float. This will prevent potential conversion errors. Damage is a float now. |
Deal damage to an attribute when a rigid body impacts something.
There is a minimum velocity setting to prevent weak hits from damaging the attribute.
I made this so that I can have a potion that shatters when dropped.