daid / EmptyEpsilon

Open source bridge simulator. Build with the SeriousProton engine.
https://daid.github.io/EmptyEpsilon/
GNU General Public License v2.0
522 stars 173 forks source link

Improved function for calculating firing solutions (relaunched). #2003

Closed andrewchimkh closed 11 months ago

andrewchimkh commented 1 year ago

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

andrewchimkh commented 1 year ago

Latest commit addresses feedback on acos behaviour. Remaining acos NaN risk in calculateTurnAngle is mitigated by the d >= turn_radius check.

andrewchimkh commented 1 year ago

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.

Is there a particular part that you would like an explanation for? Perhaps one of the following parts:

daid commented 11 months ago

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".

andrewchimkh commented 11 months ago

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 https://github.com/daid/EmptyEpsilon/pull/2003#issuecomment-1655134845, or unsubscribe https://github.com/notifications/unsubscribe-auth/BASTUJIQUSZHNW5ULTNDWYDXSNPLLANCNFSM6AAAAAA2MHJHLU . You are receiving this because you authored the thread.Message ID: @.***>

andrewchimkh commented 11 months ago

technical_note_firing_solution.pdf

Et voila !

daid commented 11 months ago

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.