-
Notifications
You must be signed in to change notification settings - Fork 554
Fix: avoid invalid spline velocity after slow application #751
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: master
Are you sure you want to change the base?
Conversation
fixed Errors |
args.velocity *= args.slowed; | ||
} | ||
if (args.velocity <= 0.f) | ||
{ |
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.
It's a nitpick, but this block of code is less indented than the surrounding code.
{ | ||
args.velocity = unit.GetSpeed(MovementInfo::GetSpeedType(MovementFlags(moveFlags & ~(MOVEFLAG_FLYING | MOVEFLAG_SWIMMING)))); | ||
|
||
// Sicherheitscheck: slowed muss im Bereich > 0.0 und = 1.0 liegen |
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.
I'd also recommend that this comment be translated to English, for consistency with the rest of the code base: "Safety check: slowed must be in the range > 0.0 and ≤ 1.0"
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.
I would honestly go the route of adding a warning here and figuring out instead where its being caused. Not preventing the problem.
args.velocity = unit.GetSpeed(MovementInfo::GetSpeedType(MovementFlags(moveFlags & ~(MOVEFLAG_FLYING | MOVEFLAG_SWIMMING)))); | ||
|
||
// Sicherheitscheck: slowed muss im Bereich > 0.0 und = 1.0 liegen | ||
if (args.slowed <= 0.f || args.slowed > 1.0f) |
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.
Would it not, in some way, make sense to replace Lines 96-101 with something similar to:
// Safety check: if unit is rooted, it shouldn't be slowed
if (unit.IsRooted())
{
args.slowed = 1.0f;
}
else if (args.slowed <= 0.f || args.slowed > 1.0f)
{
args.slowed = 1.0f;
}
In this way, rooted units bypass the slowdown logic entirely, which would make sense since, if a unit is rooted, they are already immobile.
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.
Rooted units with slowed filled should never reach this point
I'm sorry, I'm completely new to github. You could also limit the error messages so that they only appear once per npc id and only in debug mode, then at least they wouldn't disappear in the background, as a suggestion |
🍰 Pullrequest
Proof
Issues
How2Test
Todo / Checklist