Improved performance on damage feedback

saugon commented 2 weeks ago

ℹ️ This PR was initially created to address issue #1855. However, due to multiple factors contributing to performance issues with H4ck’s ultimate skill, and the primary issue being related to the VFX, this PR has now been updated to focus on general performance improvements.

Motivation

We detected some performance issues on some low-end devices when H4ck's ultimate was triggered. The main issue was an incorrect haptic triggering that was running multiple times. Additionally, I refactored the color overlay implementation. It’s now cleaner, more lightweight, and optimized, though the performance impact isn’t as significant as the haptic fix.

Summary of changes

Removed unnecessary haptic feedback calls.
Updated character shader to support damage overlay color.
Refactored damage overlay implementation.

How has this been tested?

Tested with a low end device (which had significant FPS drops when the skill was used before the fix)
Playing with H4ck and using its ultimate skill on multiple targets.

The performance should not be affected by the skill.

Checklist

[x] I have tested the changes locally.
[x] I have tested the whole game after applying the changes, not only the affected areas.
[ ] I self-reviewed the changes on GitHub, line by line.
[x] I have tested the changes in another devices.
- [ ] Tested in iOS.
- [x] Tested in Android.
[ ] This change requires new documentation.
- [ ] Documentation has been added/updated.

AngieDutra commented 2 weeks ago

I've tested two Android devices with @tkz00 and they still crashed when using two hack ultis at the same time. I will keep on testing with an IOS build as well

saugon commented 2 weeks ago

I’ll change the PR to draft. Tomorrow, I’ll try to use a phone that still has issues. If I can’t, we can still merge this PR because it also improves the performance in general.

AngieDutra commented 2 weeks ago

Talked with @saugon how to review this best:

Using hack's ulti he saw a drop of frames which in this PR happens no longer
In addition he used a config to span the bots in the middle of the map
Using ngrok fixed in this PR already in main you can play with that config and damage more players with this skill and further review this drop of frames improvement

saugon commented 1 week ago

The haptic feedback for damaging an enemy or crate is completely gone in my android phone (Samsung S10+)

Good catch, i will fix that, thanks!

Also it didn't get any better in respect to performance, but I don't know if my problem is the one being targeted in this PR.

That specific issue is related to the VFX and being worked on in another other PR.

saugon commented 1 week ago

The code reorganization is a great improvement, much easier to follow, also the logic to avoid running the haptic feedback multiple times works great. That said, I'm not sure I can see a performance difference between the current state of the game and this branch, based on the fps metric on top of the screen, both still have my screen flickering issue, but it has been explained that this PR doesn't aim at solving this issue. What do you think?

The difference could be seen in two ways:

In the profiler: doing a deep profile or custom profile marks and compare the time spent in that function (unless you get a very small measurement like 0, which indicates the device can easily handle it).
In low-end devices.

@AngieDutra has already measured the performance improvement (though she hasn’t commented here yet). If this is blocking you due to the devices you have access to, don’t worry about it and let’s continue with the review. This PR is blocking others that rely on changes made in the character prefabs.

These are some comparisons to show how many times the haptic feedback has been executed in the same situation using prints.

@tkz00 Nice test! I already noticed this issue, but we have to wait until Manu’s update on player movement and actions is merged. Once that’s done, we can trigger the feedback only if there has been a change, instead of on every frame.

AngieDutra commented 1 week ago

As @saugon said, I reviewed the performance improvements yesterday in a Redmi Android device. I'll review asap the last additions and use the profiler this time to check further

lambdaclass / champions_of_mirra