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: controlled rotorcrafts shouldn't fall when loosing ground #3911

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

Vollch
Copy link
Contributor

@Vollch Vollch commented Dec 14, 2023

Purpose of change

Describe the solution

Check whether vehicle is rotorcraft before falling, if so - start flying instead.

Describe alternatives you've considered

Rewriting all flying code, it's a mess with hardcode checks for specific z levels, and tons of bugs. But for now this will work as a quick fix, it's not the best idea to jump into rewriting flying while we're in middle of obsoleting legacy zlevels.

Testing

Flied off deck on wheeled apache from #3901

Additional context

Checklist

@github-actions github-actions bot added the src changes related to source code. label Dec 14, 2023
@scarf005 scarf005 self-requested a review December 15, 2023 01:25
@scarf005 scarf005 self-assigned this Dec 15, 2023
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

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

2023-12-15_10-27-52.mp4

it still sinks.

@Vollch
Copy link
Contributor Author

Vollch commented Dec 15, 2023

Apparently vehicles doesn't always "drop", so... I moved that check where it should trigger more reliably.

Also, during re-testing i noticed that is still sinks when flying from bottom deck. That's, kinda, intended, as it doesn't really fall in that case, but goes directly to /dev/null.
Same will happen if you'll try to descent down to water surface at any other moment. Fixing that would require rewriting landing check, which probably require rewriting whole flying detection, and then adding some check to detect submerging... all that will snowball too far for a "quick fix". But for decks at z1 and z2(where helicopters are naturally spawned) it should work with this fix.

@Vollch Vollch requested a review from scarf005 December 15, 2023 09:06
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

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

2023-12-15_18-46-20.mp4

LGTM.

image

tiny bug: rotorcraft flown this way will erroneously warn it will crash when you try to stop the engine even if it's technically landed. at some point we should migrate from is_falling and is_flying to proper state machine with sum types.

@scarf005 scarf005 merged commit b2eddc0 into cataclysmbnteam:main Dec 15, 2023
14 checks passed
@Vollch
Copy link
Contributor Author

Vollch commented Dec 15, 2023

Yeah, i know. That's actually an old bug, one of those "tons of bugs" mentioned in first post.

@Vollch Vollch deleted the helidrop branch December 15, 2023 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wheeled helicopter falls down when there's nothing below
2 participants