MOARdV / DistantObject

Distant Object Enhancement bis plugin for Kerbal Space Program
51 stars 14 forks source link

Multiple Star compatibility for BodyFlares #43

Closed pkmniako closed 1 year ago

pkmniako commented 6 years ago

BodyFlare.Update() now calculates brightness depending on BodyFlare.starBody FlareDraw.GenerateBodyFlares() checks for "closest" star for BodyFlare.starBody FlareDraw.CheckFlare() now has a CelestialBody referenceStarBody parameter

Done to enable exoplanetary flares with custom planets https://i.imgur.com/ZKt2OJO.png

MOARdV commented 6 years ago

A couple of questions on this change.

FlareDraw.GenerateVesselFlares() checks for "closest" star for BodyFlare.starBody

I don't see where this occurs. Vessel flares unconditionally use FlightGlobals.Bodies[0] for CheckDraw().

Should non-Kerbol stars even have body flares added to the list? If I follow what you're doing correctly, the body's star is set to itself, which means a degenerate vector for targetVectorToSun, which I believe results in targetRelAngle being 0, which then leads to the star being hidden when inShadow is set true. Instead of going through the processing, shouldn't the flares simply not get added to the list in the first place?

pkmniako commented 6 years ago
  1. Regarding "FlareDraw.GenerateVesselFlares() checks for "closest" star for BodyFlare.starBody", that's a typo, it was supposed to be FlareDraw.GenerateBodyFlares(), sorry about the confusion

  2. About non-Kerbol stars, in GenerateBodyFlares I added the line (301) if (body.scaledBody.GetComponentsInChildren<SunShaderController>(true).Length > 0) { refStarBody = body.orbit.referenceBody; } else { refStarBody = body; } to automatically check if a body is a star and if so, make its refStarBody to its referenceBody, so it doesn't set itself as the body's star, and then check for the nearest referenceBody star.

MOARdV commented 6 years ago

First, please understand that I do not play with multi-star or exoplanet mods.

With respect to vessel flares: if there are multiple stars, then should the vessels also have reference stars? Having planet flares illuminated by the nearest star, but vessel flares only illuminated by the star at Bodies[0] is inconsistent behavior, and I would expect it leads to vessel flares not being visible for vessels orbiting exoplanets. Or is it expected that vessels never leave the star at Bodies[0]?

With respect to non-Kerbol stars, are the flares for these stars illuminated by their parent (body.orbit.referenceBody)? I do not understand why there should be body flares for stars, since the body flare is assumed to be illuminated by an external source. Does the mod not provide a visible proxy for the additional stars?

pkmniako commented 6 years ago

I made this pull request just to adress exoplanets flares, as it's one of the most wanted features for non-Kerbol Kopernicus systems. I decided not to include non-Kerbol Vessel flares for the previous reason and by the lack of gameplay surronding other stars exploration with multiple crafts at this moment. I still have plans to add compatibility with Vessel flares, but Body flares were more important.

And I decided to leave stars with flares are there are some Bodies that could benefit from this, like Brown Dwarfs, that emit few light but still would be nice to have flares on them.