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

Formatting fixes and mean density in the System Editor #6058

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

zonkmachine
Copy link
Member

@zonkmachine zonkmachine commented Feb 8, 2025

Some fixes in physical descriptions.

  • Introducing a space before the unit. This was in response to the formatting of objects in the system overview but Format is called elsewhere. I have looked for possible issues in other modules but so far have found none. (Images)
  • Quick fix to use m³ instead of m3.
  • Add Mean Density to the System Editor. It's good to be able to quickly assess if the mass/volume ratio is within a reasonable
    range.

Before
exampleearth

Fixed
format


Some question marks.

  • Density should probably have its own format to aid in translation. I backed out of this part for now.
  • Mean density, as I first introduce it, is described in kg/m3. It looks like g/cm3 is the way to go for astronomical objects. This is how it's done now in the System editor. At least they should show the same. I'm looking into fixing this for the system overview too. This should possibly be done directly in CalcMeanDensity().

@zonkmachine
Copy link
Member Author

Fixed up gravitational acceleration. I'm not quite sure what changes would be needed in Format for a more permanent solution.

Copy link
Contributor

@fluffyfreak fluffyfreak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these are good, I don't see any issues with the code 👍

@zonkmachine zonkmachine marked this pull request as ready for review February 15, 2025 14:16
@zonkmachine
Copy link
Member Author

I think these are good, I don't see any issues with the code 👍

Right, I agree. Still, parts of this should probably be fixed in Format but I haven't wrapped my head around it yet. It's not a new issue though and can be fixed up later.

@zonkmachine
Copy link
Member Author

I squashed it down to two commits.

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

Successfully merging this pull request may close these issues.

3 participants