Phazorknight / Cogito

Immersive Sim Template Project for GODOT 4
MIT License
717 stars 81 forks source link

Impact Attribute Damage #109

Closed ac-arcana closed 4 months ago

ac-arcana commented 4 months ago

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.

ac-arcana commented 4 months ago

https://youtu.be/9vJ79VjiU2Q

ac-arcana commented 4 months ago

I do have a performance concern with using length on every impact. Potentially I can change it to length_squared. Also if I can check the timer first and fail out of the check it might save some performance.

FailSpy commented 4 months ago

I do have a performance concern with using length on every impact. Potentially I can change it to length_squared. Also if I can check the timer first and fail out of the check it might save some performance.

For this sort of thing length_squared is all you need. length doesn't majorly affect performance, but can only help to do length_squared instead

The post-timer check would also be nice as you say

ac-arcana commented 4 months ago

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.

Phazorknight commented 4 months ago

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 _physics_process on every object like this (especially since I think components like this will be used a lot), so I'll look into improving this on the ImpactSounds component and see what I can come up with.

Phazorknight commented 4 months ago

Okay, so I just pushed an update to ImpactSounds.gd in https://github.com/Phazorknight/Cogito/commit/52612a406e75a745ba1d9e54f01cb0f8430745e7

This changes it to use SceneTree timer instead of the _physics_process. @ac-arcana , @FailSpy , would love to see if y'all think perfomance is better.

FailSpy commented 4 months ago

Okay, so I just pushed an update to ImpactSounds.gd in 52612a4

This changes it to use SceneTree timer instead of the _physics_process. @ac-arcana , @FailSpy , would love to see if y'all think perfomance is better.

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: image

NEW profiled, peaked script function time: image

New technique I would say is more efficient. Though lots of SceneTreeTimer initializations at once will slow things down -- see _on_parent_node_collision_new taking longer despite being fewer calls; in theory, you won't have 450+ ImpactSounds hitting each other all at once initializing that many SceneTreeTimers

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 :laughing:

ac-arcana commented 4 months ago

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. I definitely see the performance improvement. I agree that both should be done this way. Thanks for the discourse!

ac-arcana commented 4 months ago

Updated with the new timing method. I think this is ready now :)

ac-arcana commented 4 months ago

Good catch, I didn't realize the attribute is a float. This will prevent potential conversion errors. Damage is a float now.