-
-
Notifications
You must be signed in to change notification settings - Fork 105
[Vanilla Fix] Fix BalloonHover incorrectly considering ground factors when pathfinding #1661
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
base: develop
Are you sure you want to change the base?
[Vanilla Fix] Fix BalloonHover incorrectly considering ground factors when pathfinding #1661
Conversation
Nightly build for this pull request:
This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build. |
What does this impact? Maybe the fix could be on by default? |
What did this fix |
Looks to me like the fix could be on by default, and would just have to add migration notice to disable it if undesired. |
I placed the IsInAir check at the end, which may slightly improve performance. |
using a naked hook may improve the situation |
Dunno how to do it. Any example? |
Search for DEFINE_NAKED_HOOK in code. |
Only one use case: DEFINE_NAKED_HOOK(0x7CD8EA, _ExeTerminate)
} Should I write asm like that? |
You're not really limited to assembly. You should just keep in mind that a naked hook is simply a jump to the code you write, no prolog, epilog or anything like that, so you should care to not mess up the stack, registers etc. |
If you want to look at naked hooks, you could look at Vinifera. It's all naked hooks there :P |
This reverts commit 0483fbe.
Too stupid to write asm. Can't finish it myself. |
there's another way if the calculation can't be optimized: instead of checking the toggle inside the hooks, we can make it patch down these hooks directly if disabled. See 2ff45a0 for reference |
f6fb18c
to
a6c8a46
Compare
a6c8a46
to
782b3b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kerb said this option should be enabled by default🤔
now that this is proven to have impact on performance, it's not really good to enable it by default. Also this fix will change the behavior of aircrafts which might affect players who's getting used to current behavior |
@ZivDero maybe you could assist with a naked hook? |
Fixed the bug where technos with
BalloonHover=yes
incorrectly considered ground factors when setting the destination and distributing moving commands. Use[General]->BalloonHoverPathingFix=true
to enable this fix.