Starfixes #233
Starfixes #233
Conversation
|
this could be considered a bug fix, but I will let you merge it @StollD because I added something that might be considered a new feature: killing star rotation in the RnD if you want we could add a parameter like |
| @@ -40,6 +40,18 @@ namespace Kopernicus | |||
| /// </summary> | |||
| public class RnDFixer : MonoBehaviour | |||
| { | |||
| List<RDPlanetListItemContainer> stars = new List<RDPlanetListItemContainer>(); | |||
There was a problem hiding this comment.
Please add an explicit private and move the initialization to Start()
| void LateUpdate() | ||
| { | ||
| // Block the orientation of all stars | ||
| for (int i = 0; i < stars?.Count(); i++) |
There was a problem hiding this comment.
stars is never null so you can remove the ? (I am suprised that this compiles, since you are comparing an int to null)
There was a problem hiding this comment.
an int cannot be lower than a null so it returns false
I had an issue once where a null list was breaking my code, since then I've always used ? to make sure that even if I have a null the .Count() method doesn't trigger a nullref
is there a downside to using the ? ?
There was a problem hiding this comment.
It is extra code that gets executed every frame and that isn't neccessary.
| // Block the orientation of all stars | ||
| for (int i = 0; i < stars?.Count(); i++) | ||
| { | ||
| RDPlanetListItemContainer star = stars.ElementAt(i); |
There was a problem hiding this comment.
stars[i] - No need to use Linq and waste memory.
| @@ -50,8 +50,8 @@ static StarComponent() | |||
| void Start() | |||
| { | |||
| // Find the celestial body we are attached to | |||
| celestialBody = PSystemManager.Instance.localBodies.First(body => body.scaledBody == gameObject); | |||
| Logger.Default.Log("StarLightSwitcher.Start() => " + celestialBody.bodyName); | |||
| celestialBody = PSystemManager.Instance.localBodies.FirstOrDefault(body => body.scaledBody == gameObject); | |||
There was a problem hiding this comment.
While I agree that this shouldn't throw the message it currently throws, it should throw something. Maybe something like this
if (celestialBody == null)
throw new InvalidOperationException("The StarComponent isn't attached to the ScaledSpace object of a body.");There was a problem hiding this comment.
the problem is that in the RnD all stars have a StarComponent
I don't think they should throw an error everytime you enter the RnD
add customizeable option RDRotation
|
ok, I think I've addressed everything you are still using .ElementAt in the RnDFixer.cs |
| [ParserTarget("RDVisibility")] | ||
| public EnumParser<RDVisibility> hiddenRD |
There was a problem hiding this comment.
You can just add two ParserTargets to one property ;)
|
Yes, that would be cool |
some fixes for stars in the RnD