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

VectorExtensions.Zero(this Vector3 v) doesn't work #268

Open
gotmachine opened this issue Oct 15, 2024 · 2 comments
Open

VectorExtensions.Zero(this Vector3 v) doesn't work #268

gotmachine opened this issue Oct 15, 2024 · 2 comments
Labels
kspBug Identified KSP issue

Comments

@gotmachine
Copy link
Contributor

The following stock method in VectorExtensions :

public static void Zero(this Vector3 v)
{
	v.x = 0f;
	v.y = 0f;
	v.z = 0f;
}

doesn't work as it doesn't pass the vector by ref.

That method is used in a variety of stock code paths :
image

Not much we can do as we can't change the method signature, but well, keeping this here as a reminder...

@gotmachine gotmachine added the kspBug Identified KSP issue label Oct 15, 2024
@JonnyOThan
Copy link
Contributor

Yikes. How many of those actually matter? We could probably transpile out some of them.

@gotmachine
Copy link
Contributor Author

From a quick review, I haven't see anything that would cause real issues, most of the time the method is used to set a field to zero when that field is meaningless in the current context, but it could theoretically lead to other pieces of code making wrong assumptions. The only case I would somewhat worry about is the VesselPrecalculate.CalculatePhysicsState() one, and we are fixing it already through the flight perf patches.

The CenterOfLift / CenterOfThrust cases might be responsible for some weirdness with the CoL/CoT markers in the editor, IDK.

One case that look like it could be introducing some weirdness is thePartBuyoancy.lastBuoyantForce field, but in the end it probably doesn't matter too much. Note that we are already prefixing PartBuyoancy.FixedUpdate(), so it might be worth it to just override the whole method if we choose to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kspBug Identified KSP issue
Development

No branches or pull requests

2 participants