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

Add part bonus to rotor diameter in thrust calculations #79328

Merged
merged 4 commits into from
Jan 27, 2025

Conversation

raycrasher
Copy link
Contributor

@raycrasher raycrasher commented Jan 24, 2025

Summary

Features "Add part bonus to rotor diameter in thrust calculations"

Purpose of change

This allows creation of new lifting parts that have a smaller danger zone like ducted fans or VTOL thrusters.

Describe the solution

Small change to vehicle::lift_thrust_of_rotorcraft that adds the part bonus to the diameter. This does not affect collision diameter.
So if rotor_diameter = 5 and bonus = 10 then the rotor will have a thrust of a 15-diameter rotor but have a collision diameter of just 5.

Describe alternatives you've considered

Increasing rotor area (and thus the collision zone) is currently the only way to improve lift.

Testing

Game compiles.

Old rotors function as normal.

Replace "small_civilian_rotors" JSON:

"rotor_diameter": 1,
"bonus": 7

The 2seater2 helicopter still flies as normal.
Rotor collision area should be reduced.

Additional context

@github-actions github-actions bot added Vehicles Vehicles, parts, mechanics & interactions [C++] Changes (can be) made in C++. Previously named `Code` <Enhancement / Feature> New features, or enhancements on existing new contributor labels Jan 24, 2025
@github-actions github-actions bot added <Documentation> Design documents, internal info, guides and help. [Markdown] Markdown issues and PRs labels Jan 24, 2025
@raycrasher raycrasher marked this pull request as ready for review January 24, 2025 21:28
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Jan 25, 2025
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 26, 2025
@Maleclypse Maleclypse merged commit 34f7c27 into CleverRaven:master Jan 27, 2025
21 of 27 checks passed
@Maleclypse
Copy link
Member

@Maleclypse tagging myself here to have an email that I can easily pull up for the mad genius to make a personal helicopter design.

@moxian
Copy link
Contributor

moxian commented Jan 27, 2025

I probably should have said something earlier, but now that this is merged, there's little point. Still, I think it's worth raising some concerns (even if those don't change anything). Two concerns, and a nitpick:

  • This is adding a code path that is never excercised in game (neither in base game nor in the mainline mods). Why? I don't have a quote handy, but I belive we try pretty hard to only add the code that is used, and this one is not. If this came bundled with a ducted fan vehicle part (and a ducted fan-using vehicle), that would be fair, but right now, it's just code for code's sake?
  • How exactly do you calculcate the "additional rotor diameter during lift calculations, in meters"? If a ducted fan has a rotor_diameter of 15 and a bonus of 5 [meters], where do those "5 meters" come from? I doubt it's anywhere on the physical part? If we wanted to have an arbitrary "this part magically generates more lift without increasing the danger area", we should have made the bonus a unitless scaling factor (so "bonus": 0.33 in the 15m/5m example)
  • This is more of a nitpick, but reusing the generic "bonus" field for this just feels very bad. It should have a nice readable name like "rotor_effective_diameter_bonus" or (if we got with factor scaling route) "lift_factor"/"lift_bonus_factor".). And if we stick to "extra diameter, in meters", then that "in meters" should really be part of the json, not tucked away into the docs. It should be specified as "bonus": "5m", so that people don't get confused on what exactly that 5 means. But in your defence, rotor_diameter is already not following the best practices here, so this is underestandable.

Are you planning on making followup PRs adding the parts, or did you add it for your personal mod, or some third option?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` <Documentation> Design documents, internal info, guides and help. <Enhancement / Feature> New features, or enhancements on existing json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs new contributor Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants