-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
Reduce the amount of casting required for floating points in C# #5403
Comments
cc @neikeq |
For context this behavior was changed by godotengine/godot#65168 in order to fix issue godotengine/godot#65139. Related issues:
The new behavior is correct since it uses the right types as defined by core and this is also how godot-cpp generates its bindings (see https://github.com/godotengine/godot-cpp/blob/24f97739a189c6edef5ba2516b29abe60f97b004/binding_generator.py#L1801-L1829). Let me address some of your suggestions:
This would make vector types use double which is incorrect, in core scalars use 64-bit types but vectors use 32-bit types (unless the engine is built with
real_t is not a type, it gets replaced by double or float on compilation depending on if you build with To quote the .NET guidelines on conversion operators:
This would be essentially the same as implicitly converting the double we get from core to a float, which is again a lossy conversion and would be a bad idea as explained above. The only difference with the previous suggestions is the conversion is done by the engine and not on C#'s side but that doesn't change the fact that you lose precision. My two cents, there is nothing wrong with using explicit conversion when you know the loss of precision is acceptable and being explicit is much better because otherwise you might accidentally and unintentionally lose precision without realizing it. You might have a point about |
It might be worth undoing the recent changes in godotengine/godot#65168, so that |
The problem about reverting that PR is that there are other APIs that can be broken with int or float. I plan to make it possible to specify size and sign for virtual methods and signals, just as it's done with normal methods and properties. That should solve some issues, but not BTW you cast once into a field, to avoid repeating: public override void _Process(double delta)
{
float deltaf = (float)delta;
speed += acceleration * deltaf;
Position += new Vector2(speed, 0f) * deltaf;
Rotation += Mathf.RadToDeg(turnSpeed) * deltaf;
} |
Well, there's the real problem! Why do scalars and vectors use different precisions in the first place? That's pretty counter-intuitive.
Is there really any precision being lost, though? If you're multiplying a vector by a scalar, it's the vector that's getting changed, not the scalar. The vector isn't losing any precision, because it was already limited to 32 bits. The only reason this would catch someone by surprise is if they assumed
If it doesn't affect IMO, Godot 4 can't be considered ready for release until this is fixed. It's an extremely common use case that has seen a very obvious regression. One of these things needs to happen:
Those last two options are probably out of the question, given that they'd have consequence that I can't fully comprehend. I seriously suggest giving options 1 and 2 a real look, though. I think they would be an excellent compromise. |
That's a solution I'm considering. One thing that worries me is that it may cause confusion about the call order if for some reason a class overrides the |
It's not quite that simple. If they supported only multiplying by doubles, then that would be a lot of unnecessary casting. If they supported both types in the operators, then other numeric types will not work because the cast will be ambiguous (ex: you wouldn't be able to do
This can be done by compiling Godot with double-precision support. Aside from this proposal, this is the preferred option for games that need precise calculations or to handle large scales, so you may want to use this anyway.
This won't be done. However, note that in Godot 3.x, this is how it was exposed in C#, with the process method using |
IMO there can be different names for the override, like Since C# users have long been using |
Not familiar with Godot, but wouldn't this be a problem in the future or a redundant feature in the engine? It seems to me that it would be much better to make it possible to perform vector operations on both float and double scalars, so precision will not be lost if the engine uses double precision for vectors, and you do not have to pull another method like _Process |
Describe the project you are working on
Experiments with the master branch and CSharp
Describe the problem or limitation you are having in your project
Old behaviour: every floating point is
float
New behaviour: methods use
double
and everything else usesfloat
Old
Variant 1: my code uses
double
Variant 2: my code uses
float
No matter what type I use I need to cast alot, which does not feel nice and pollutes the code.
Describe the feature / enhancement and how it helps to overcome the problem or limitation
Solutions
Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams
New
Problem
Casting from
double
to float can't be added to csharp, soreal_t
must be an opaque struct or every constructur/member includes a copy withdouble
If this enhancement will not be used often, can it be worked around with a few lines of script?
Not possible
Is there a reason why this should be core and not an add-on in the asset library?
Not possible
The text was updated successfully, but these errors were encountered: