Skip to content

Conversation

@nicholasgower
Copy link
Collaborator

I have added an additional step to PlanetsLib:extend that reduces the amount of work needed to integrate a planet into PlanetsLib. In the current version of PlanetsLib, all PlanetsLib planets must have a fully defined orbit field, so this change will not break compatibility with any planets currently using PlanetsLib.

If config.distance and config.orientation exist, but config.orbit is undefined, this code concludes that its orbit should go around the sun like all non-PlanetsLib planets, and generates an orbit field with that assumption, deleting config.distance and config.orientation.

This change will make it easier to integrate PlanetsLib into existing planets by eliminating the need to learn how the orbit field must be defined. With this change, only bodies that have an unusual orbit will need to have a fully-defined orbit according to PlanetsLib conventions.

…orbit information if formatted according to vanilla conventions. Makes it easier to integrate PlanetsLib into existing planets.
@danielmartin0
Copy link
Owner

danielmartin0 commented Feb 18, 2025

Copying my response on Discord here for the benefit of Github readers.

I'm concerned that removing this hurdle is helpful for us pros, but:

  • If somebody defined a moon the normal way and then switched to using PlanetsLib without hitting this error, they avoided learning something important.
  • Having two APIs for the same functionality can add confusion about how the functionality works. I would no longer be able to say to people 'this is how orbit fields work in PlanetsLib' and would instead have to start talking about the internals of the library.

As discussed, a more explicit API like PlanetsLib.convert_vanilla_planet(planet) would address my concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants