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

Feature/maroon 424 planetary system #442

Open
wants to merge 143 commits into
base: develop
Choose a base branch
from

Conversation

Lowfi001
Copy link
Collaborator

@Lowfi001 Lowfi001 commented Jun 1, 2023

Closes #424

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please rename it to e.g. planet_box or something similar.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This object has many vertices. Please try to reduce the number of vertices for this model.

Copy link
Collaborator

Choose a reason for hiding this comment

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

same, many vertices for a simple object

Copy link
Collaborator

Choose a reason for hiding this comment

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

i think we can remove the detailed version (telescope_pars)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use a name without "reduced" after deleting the detailed version.

*/
#region PlanetInfoInstance
private static PlanetInfo _instance;
public static PlanetInfo Instance
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need a static instance property for this class?
FindObjectOfType returns the first object in the scene and there are multiple planets with this class...

{
//sun source: NASA.gov https://nssdc.gsfc.nasa.gov/planetary/factsheet/sunfact.html
case PlanetInformation.sun_0:
mass = 1988500f;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not via the inspector?

float planetRotationPerSecond = 0;

//every 0.02 sec = 50x per sec
void FixedUpdate()
Copy link
Collaborator

Choose a reason for hiding this comment

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

add private

//360� rotation for each earth day (24h) in 1 FixedUpdate(0.02) * 50 for 1 sec
planetRotationPerSecond = (360 / -planetInfo.rotationPeriod) * (ROTATION_PERIOD_SCALE_FACTOR / 50);
transform.Rotate(new Vector3(0, planetRotationPerSecond, 0));
//transform.Rotate(new Vector3(0, ((360 / -planetInfo.rotationPeriod) * (ROTATION_PERIOD_SCALE_FACTOR)), 0) * Time.deltaTime);
Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove unused code

#endregion Slider

#region UIToggleButtons
[SerializeField] private Toggle toggleAllTrajectories;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to remove all of these UI dependencies from the planetary controller

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.

Planetary System Experiment
2 participants