You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I really liked the introduction of fractions instead of numbers with long decimal sequences. It cought me by surprise and put a smile on my face. That feature is the very definition of quality. Really good job!
But, there's always room for improvement. :-)
I think it would make sense to give preference to the following divisors:
2^n for n in (8,10,12-16, 20)
2^n-1 for n in (8,10,12,15,16)
Some rationale:
255 (2^8-1) is very common when converting (u)byte (f.ex. RGB) <-> normalized float.
2^10 and 2^20 is common when converting larger numbers to computer-K and -M.
I saw a long color initialization sequence where the decompiler preferred 51 as divisor, where 255 would have been more obvious (example below).
Coming to think of it, should negative numbers become handled, maybe even 2^7 and 2^7-1 should be considered.
I have also seen code normalizing values in f.ex the ranges +/- 4K, 16K, 32K and 64K, so maybe negative numbers should be considered too (at least for a/this small subset of potential divisors)?
I believe that would allow better/nicer decompilation of some code than the following example of current decompiler output (all three should preferrably be divisor 255f): 0.882352948f, 40f / 51f, 0.5882353f.
Again, good job!
The text was updated successfully, but these errors were encountered:
2*PI is a common constant used in some types of applications.
If a type is already using something from the namespace UnityEngine,
namespace UnityEngine {
public struct Mathf
{
public const float PI = 3.14159274f;
public const float Infinity = float.PositiveInfinity;
public const float NegativeInfinity = float.NegativeInfinity;
public const float Deg2Rad = 0.0174532924f;
public const float Rad2Deg = 57.29578f;
public static readonly float Epsilon;
}}
Should UnityEngine(.Mathf?) not already be included, perhaps deg<->rad constants could be added to the "specials" list and become inlined (180f/PI) etc?
If you have the time and are interested, it would be nice, if you could take a look at the code yourself and provide some improvements as a pull request. 😄 The relevant code lives in TypeSystemAstBuilder, entry-point is the ConvertFloatingPointLiteral method. If you have any questions regarding the existing code, please feel free to ask! (Please do not forget to extend the WellKnownConstants test case.) Thank you very much!
Another improvement (adding it here so it's not forgotten) would be to identify negative constants too.
Example: float.Epsilon is currently identified, but -float.Epsilon is not.
I really liked the introduction of fractions instead of numbers with long decimal sequences. It cought me by surprise and put a smile on my face. That feature is the very definition of quality. Really good job!
But, there's always room for improvement. :-)
I think it would make sense to give preference to the following divisors:
2^n for n in (8,10,12-16, 20)
2^n-1 for n in (8,10,12,15,16)
Some rationale:
255 (2^8-1) is very common when converting (u)byte (f.ex. RGB) <-> normalized float.
2^10 and 2^20 is common when converting larger numbers to computer-K and -M.
I saw a long color initialization sequence where the decompiler preferred 51 as divisor, where 255 would have been more obvious (example below).
Coming to think of it, should negative numbers become handled, maybe even 2^7 and 2^7-1 should be considered.
I have also seen code normalizing values in f.ex the ranges +/- 4K, 16K, 32K and 64K, so maybe negative numbers should be considered too (at least for a/this small subset of potential divisors)?
I believe that would allow better/nicer decompilation of some code than the following example of current decompiler output (all three should preferrably be divisor 255f):
0.882352948f, 40f / 51f, 0.5882353f
.Again, good job!
The text was updated successfully, but these errors were encountered: