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

Update collision.cpp #3096

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

Update collision.cpp #3096

wants to merge 2 commits into from

Conversation

Iceyshell
Copy link

Fix ceiling slope hang so which was fixed by others too.However,Ensure that the code is consistent in determining the validity of the p1 coordinate relative to the area range

Fix ceiling slope hang so which was fixed by others too.However,Ensure that the code is consistent in determining the validity of the p1 coordinate relative to the area range
@tobbi
Copy link
Member

tobbi commented Nov 9, 2024

@SuperTux/developers I'm not too familiar with our collision system. Will changing rdelta negatively affect anything else?

Copy link
Member

@MatusGuy MatusGuy left a comment

Choose a reason for hiding this comment

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

Seems like this PR does the exact same as #3057 and iirc it didn't work out.

This time, the depth detection logic has been modified to reduce the offset value appropriately so that the player can move more smoothly when close to the ceiling
Also, adjusted the constraints to add additional handling of top collisions to make sure players don't stay in the ceiling area
Hope this time everything will be fine
@Iceyshell
Copy link
Author

I reworked it and was able to implement it locally, so hopefully it will be useful for the main supertux branch!

@Frostwithasideofsalt
Copy link
Member

image
image

okay, so. this pr works... but not really. it makes it less common? but it still happens.

@Frostwithasideofsalt
Copy link
Member

i'll hold off on approving until it's fully fixed

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.

4 participants