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

Fix max drop height on slopes #2812

Merged
merged 16 commits into from
Sep 9, 2024
Merged

Conversation

MatusGuy
Copy link
Member

@MatusGuy MatusGuy commented Mar 2, 2024

for example mr tree now falls off 1 tile ledges and walking badguys cant walk up some slopes

@MatusGuy MatusGuy added priority:medium status:in-progress Progress has been done, but more is intended be done category:code type:bugfix Pull Requests that fix bugs. labels Mar 2, 2024
@MatusGuy MatusGuy mentioned this pull request Apr 4, 2024
@Rusty-Box
Copy link
Member

Few bugs I found:

  • Captain Snowball has trouble jumping of slopes that end in pits. They just drop to their doom
  • Haywire constantly attempts to jump over up-facing slopes
  • Mr. Tree, Stumpy and Big Granito turn around after the end of a slope, going left, and turn back half way going down to the right

@MatusGuy MatusGuy changed the title Fix issues regarding walking badguys and slopes Fix max drop height on slopes Aug 12, 2024
@MatusGuy MatusGuy marked this pull request as ready for review August 14, 2024 13:10
src/badguy/badguy.cpp Outdated Show resolved Hide resolved
src/badguy/badguy.cpp Outdated Show resolved Hide resolved
src/badguy/badguy.cpp Outdated Show resolved Hide resolved
src/badguy/badguy.cpp Outdated Show resolved Hide resolved
src/badguy/badguy.cpp Outdated Show resolved Hide resolved
src/badguy/badguy.cpp Outdated Show resolved Hide resolved
@@ -230,7 +230,7 @@ class BadGuy : public MovingSprite,

/** Returns true if we might soon fall at least @c height
pixels. Minimum value for height is 1 pixel */
bool might_fall(int height = 1) const;
bool might_fall(int height = 1);
Copy link
Member

Choose a reason for hiding this comment

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

Can you tell me why this method is no longer const? Do we modify the object state with the changes? And if so: Could we prevent that?

Copy link
Member Author

Choose a reason for hiding this comment

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

It changes m_detected_slope. I do this because I want to prevent having two raycasts at a time. So to fix that I alternate between them by checking if m_detected_slope is 0 or not.

@@ -99,7 +99,7 @@ void WalkingBadguy::set_ledge_behavior(LedgeBehavior behavior)
break;

case LedgeBehavior::SMART:
max_drop_height = static_cast<int>(get_bbox().get_width()) / 2;
max_drop_height = 16.f;
Copy link
Member

Choose a reason for hiding this comment

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

Does this number have any significance? Seems like a magic number. Just curious.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's half a tile

src/badguy/badguy.cpp Outdated Show resolved Hide resolved
@MatusGuy MatusGuy requested a review from tobbi August 16, 2024 20:19
@MatusGuy MatusGuy merged commit fd548f2 into SuperTux:master Sep 9, 2024
32 of 33 checks passed
MatusGuy added a commit that referenced this pull request Sep 11, 2024
@Frostwithasideofsalt
Copy link
Member

Left - how it acts with this PR
Right - how it acts without the PR/ how it should act
https://github.com/user-attachments/assets/e96b294d-5016-459f-aaa4-d09e60ab4ee2

MatusGuy added a commit that referenced this pull request Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:code priority:medium status:in-progress Progress has been done, but more is intended be done type:bugfix Pull Requests that fix bugs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants