Skip to content

Let squished enemies fade out instead of vanishing instantly #3198

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Brockengespenst
Copy link
Contributor

Squished enemies will now fadeout instead of vanishing instantly. This looks a bit smoother and is more aligned with the melting or burning animation.

@Brockengespenst
Copy link
Contributor Author

Brockengespenst commented Feb 2, 2025

Just an idea. Looks better to me when squished Badguys do not vanish instantly. If you do not like it, I can delete this PR.

@bruhmoent
Copy link
Member

Could you please provide video footage?

@Brockengespenst
Copy link
Contributor Author

@tobbi
Copy link
Member

tobbi commented Feb 3, 2025

Looking good to me.

@tobbi tobbi requested review from MatusGuy and Vankata453 February 8, 2025 16:09
@@ -315,12 +317,18 @@ BadGuy::update(float dt_sec)
case STATE_SQUISHED:
m_is_active_flag = false;
if (m_state_timer.check()) {
remove_me();
m_alpha_before_fadeout = m_sprite->get_alpha();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An enemy at 0.5 alpha should fade out faster. In that case, you don't need this variable. You can multiply the fadeout time by the alpha in the set_state function.

Copy link
Contributor Author

@Brockengespenst Brockengespenst Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only valid for the fadeout time, the alpha value at the start of the fadeout is still needed.

if (m_state_timer.check())
remove_me();
else
m_sprite->set_alpha(m_alpha_before_fadeout - (m_alpha_before_fadeout * m_state_timer.get_progress()));
Copy link
Contributor

@MatusGuy MatusGuy Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here you'd only need to put m_state_timer.get_progress()

Suggested change
m_sprite->set_alpha(m_alpha_before_fadeout - (m_alpha_before_fadeout * m_state_timer.get_progress()));
m_sprite->set_alpha(1.f - m_state_timer.get_progress()));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If alpha was already lower than 1, the suggested change would put it up to 1 before fading out again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid point here. Although all badguys currently have alpha=1.0, it doesn't have to hold in future. If they already start at a lower value of alpha, then the suggested change won't work. Just for better readability, I would write it as:

m_alpha_before_fadeout * (1.0f - m_state_timer.get_progress())

@@ -42,6 +42,7 @@
static const float SQUISH_TIME = 2;
static const float GEAR_TIME = 2;
static const float BURN_TIME = 1;
static const float FADEOUT_TIME = 0.2f;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure because I havent tested this in game but this might be too fast?? Someone else needs to confirm.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The effect is noticeable. I wouldn't mind if it was slower but this works too.

Squished enemies will now fadeout instead of vanishing instantly. This
looks a bit smoother and is more aligned with the melting or burning
animation.
@Alasdairbugs Alasdairbugs added the status:needs-review Work needs to be reviewed by other people label Feb 28, 2025
@Alasdairbugs Alasdairbugs added this to the 0.7.0 milestone Feb 28, 2025
@tobbi
Copy link
Member

tobbi commented Mar 21, 2025

I've started the failing build again and would like to merge it one that is finished.

Copy link
Member

@Hypernoot Hypernoot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code OK
test OK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:needs-review Work needs to be reviewed by other people
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants