Skip to content
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

Various crusher fixes #3047

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Various crusher fixes #3047

wants to merge 5 commits into from

Conversation

tobbi
Copy link
Member

@tobbi tobbi commented Aug 21, 2024

This also includes some code style improvements to the Crusher::collision and relieves some of the mental pain I have when looking at the SuperTux source code.

Fixes #3024

@bruhmoent
Copy link
Member

Will test tm.

@tobbi tobbi changed the title Hurt player when hit by a sideways crusher instead of killing Various crusher fixed Aug 27, 2024
@tobbi tobbi changed the title Various crusher fixed Various crusher fixes Aug 27, 2024
@tobbi
Copy link
Member Author

tobbi commented Aug 27, 2024

I've added a fix for an issue found by frost, where roots would spawn in the wrong direction.

@tobbi
Copy link
Member Author

tobbi commented Aug 28, 2024

@bruhmoent Could you test as well?

@bruhmoent
Copy link
Member

Yes. I will soon.

Copy link
Member

@bruhmoent bruhmoent left a comment

Choose a reason for hiding this comment

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

Tested

@tobbi
Copy link
Member Author

tobbi commented Aug 30, 2024

I moved some code out but I don't know what phase_factor or amplitude do. So, well...

@tobbi
Copy link
Member Author

tobbi commented Sep 22, 2024

@Vankata453 @MatusGuy Can you guys test the code changes and merge maybe?

Comment on lines +2328 to +2334
if ((hit.left || hit.right)) {
if (m_ignore_sideways_crush) {
m_ignore_sideways_crush = false;
}
else {
kill(true);
}
Copy link
Member

Choose a reason for hiding this comment

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

If tux isn't killed immediately when crushed vertically, then why can't it also happen here? This way you remove the ignore_sidways_crush thing. But I might be missing something

src/badguy/crusher.cpp Show resolved Hide resolved
Copy link
Member

@Vankata453 Vankata453 left a comment

Choose a reason for hiding this comment

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

Tested the sideways crushing behaviour, sometimes gets Tux stuck in the wall when crushed, but he's invincible so it should be fine. I'm just not sure if that might open the door for some glitchy skips in levels.

Comment on lines 132 to 133
// If the other object is the player, and the collision is at the
// bottom of the crusher, hurt the player.
Copy link
Member

Choose a reason for hiding this comment

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

This comment doesn't reflect the changes (sideways collision also now hurts the player).

Comment on lines +2328 to +2334
if ((hit.left || hit.right)) {
if (m_ignore_sideways_crush) {
m_ignore_sideways_crush = false;
}
else {
kill(true);
}
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't we make it so that despite of the side where Tux was hit from, kill(false) is performed? Is there a reason for sideways crushing to instantly kill Tux with kill(true)?

If we do that, m_ignore_sideways_crush wouldn't be needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sideways Crusher should not insta-kill players
5 participants