-
Notifications
You must be signed in to change notification settings - Fork 173
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
Improved function for calculating firing solutions (relaunched). #2003
Conversation
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.
Not saying I fully understand the new code yet, I think I need to draw it out to follow it. But in general it looks good, just two nitpicks on code reuse and potential stability.
@@ -306,63 +306,108 @@ string WeaponTube::getTubeName() | |||
return "?" + string(direction); | |||
} | |||
|
|||
// Coordinate transform from global reference frame (X: right/east, Y: down/south) | |||
// to weapon tube reference frame (X: right of tube, Y: direction of fire) | |||
static glm::vec2 transform_global_to_tube(glm::vec2 vec, float angle) |
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.
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 had considered using rotateVec2, and I would normally go with reusing a function as the preferable solution. But, I decided against it for the following reasons:
- rotateVec2 takes angles in degrees. But, there are various other operations inside the iterative loop where it would be much more convenient to keep the angles in radians by default. Thus, I thought it would be clearer if I consistently used radians inside the main body of the iterative loop, and only converted back into degrees at the very end.
- The game seems to work in a left-handed coordinate system, which I was struggling with during testing. So I wanted to switch to a right-handed system which I am more familiar with, for only the internal calculations of the function. This means a rotation + reflection transform. I thought it would be simpler to implement it directly instead of chaining rotateVec2 with a reflection operation.
{ | ||
const float x = turn_direction * aim_position.x; | ||
const float aim_radius = glm::length(aim_position - turn_direction*glm::vec2(turn_radius, 0.0f)); | ||
const float a = glm::acos((turn_radius - x) / aim_radius); |
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.
Instead of acos could atan2 be used? Or https://github.com/daid/SeriousProton/blob/master/src/vectorUtils.h#L15C26-L15C26
acos has a pretty high risk of a domain error causing NaN
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.
Good point. I will update the code to try this.
Latest commit addresses feedback on acos behaviour. Remaining acos NaN risk in |
Is there a particular part that you would like an explanation for? Perhaps one of the following parts:
|
…n to transform to tube-centric reference frame.
Note that I haven't forgotten about this, I just haven't had the mental ability yet to dive into it. I do apricate the work you put into this a LOT! |
Okay. I will work on a technical note to help explain how it works.
…On Fri, 28 Jul 2023, 08:59 daid, ***@***.***> wrote:
Note that I haven't forgotten about this, I just haven't had the mental
ability yet to dive into it. I do apricate the work you put into this a LOT!
I want to understand it before I merge it, also because I need to port it
to the ECS branch. But overall it does look good now from a general "scan".
—
Reply to this email directly, view it on GitHub
<#2003 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BASTUJIQUSZHNW5ULTNDWYDXSNPLLANCNFSM6AAAAAA2MHJHLU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
technical_note_firing_solution.pdf Et voila ! |
Holy, that is a clever bit of geometry. I wouldn't have been able to pick that up directly from the code, but the picture at step 3 with the explanation was really helpful in understanding this. |
…d#2003) * Improve weapon tube function for calculating firing solutions. * Fixed bug where firing solutions are poor when the parent ship is moving. * Make turn angle calculation more robust against acos NaN behaviour. * Switch to left-handed coordinate system and re-use rotateVec2 function to transform to tube-centric reference frame.
Proposed improvement to the WeaponTube::calculateFiringSolution function for faster and more reliable convergence:
Relaunched pull request because the previous one had a bug that made the firing solution incorrect when the parent ship is moving. Bug is now corrected. Contact Chimp on discord for more information.
Fixes #1996