Hello! I work with Andrew (Thy Reaper), and have reported a couple of bugs through him before. This one seemed easy enough to fix so I decided to submit a patch instead.

I bound float-infinity as a constant global to the scripts, and found out that because of the way AS does its comparisons, INFINITY != INFINITY, which is counter to the behaviour of infinities in C/C++. If that's intended feel free to ignore the patch, but I can't think of a reason why it would be.

To me, at least, it looks like the way the comparisons are done is a premature optimization. The compiler can figure out how to optimize it better, and from what I can tell from a trivial decompile, it might very well do better on the plain version that I changed it to (two compares versus a subtract and two compares). That is, of course, all highly machine- and compiler-specific speculation, but in general I think it's better to leave it to the C++ compiler to decide what's best.

[source lang="cpp"]Index: sdk/angelscript/source/as_context.cpp
===================================================================
--- sdk/angelscript/source/as_context.cpp (revision 1413)
+++ sdk/angelscript/source/as_context.cpp (working copy)
@@ -2149,10 +2149,11 @@
// Comparisons
case asBC_CMPd:
{
- double dbl = *(double*)(l_fp - asBC_SWORDARG0(l_bc)) - *(double*)(l_fp - asBC_SWORDARG1(l_bc));
- if( dbl == 0 ) *(int*)&m_regs.valueRegister = 0;
- else if( dbl < 0 ) *(int*)&m_regs.valueRegister = -1;
- else *(int*)&m_regs.valueRegister = 1;
+ double dbl1 = *(double*)(l_fp - asBC_SWORDARG0(l_bc));
+ double dbl2 = *(double*)(l_fp - asBC_SWORDARG1(l_bc));
+ if( dbl1 == dbl2 ) *(int*)&m_regs.valueRegister = 0;
+ else if( dbl1 < dbl2 ) *(int*)&m_regs.valueRegister = -1;
+ else *(int*)&m_regs.valueRegister = 1;
l_bc += 2;
}
break;
@@ -2170,20 +2171,22 @@

case asBC_CMPf:
{
- float f = *(float*)(l_fp - asBC_SWORDARG0(l_bc)) - *(float*)(l_fp - asBC_SWORDARG1(l_bc));
- if( f == 0 ) *(int*)&m_regs.valueRegister = 0;
- else if( f < 0 ) *(int*)&m_regs.valueRegister = -1;
- else *(int*)&m_regs.valueRegister = 1;
+ float f1 = *(float*)(l_fp - asBC_SWORDARG0(l_bc));
+ float f2 = *(float*)(l_fp - asBC_SWORDARG1(l_bc));
+ if( f1 == f2 ) *(int*)&m_regs.valueRegister = 0;
+ else if( f1 < f2 ) *(int*)&m_regs.valueRegister = -1;
+ else *(int*)&m_regs.valueRegister = 1;
l_bc += 2;
}
break;

case asBC_CMPi:
{
- int i = *(int*)(l_fp - asBC_SWORDARG0(l_bc)) - *(int*)(l_fp - asBC_SWORDARG1(l_bc));
- if( i == 0 ) *(int*)&m_regs.valueRegister = 0;
- else if( i < 0 ) *(int*)&m_regs.valueRegister = -1;
- else *(int*)&m_regs.valueRegister = 1;
+ int i1 = *(int*)(l_fp - asBC_SWORDARG0(l_bc));
+ int i2 = *(int*)(l_fp - asBC_SWORDARG1(l_bc));
+ if( i1 == i2 ) *(int*)&m_regs.valueRegister = 0;
+ else if( i1 < i2 ) *(int*)&m_regs.valueRegister = -1;
+ else *(int*)&m_regs.valueRegister = 1;
l_bc += 2;
}
break;
@@ -2192,20 +2195,22 @@
// Comparisons with constant value
case asBC_CMPIi:
{
- int i = *(int*)(l_fp - asBC_SWORDARG0(l_bc)) - asBC_INTARG(l_bc);
- if( i == 0 ) *(int*)&m_regs.valueRegister = 0;
- else if( i < 0 ) *(int*)&m_regs.valueRegister = -1;
- else *(int*)&m_regs.valueRegister = 1;
+ int i1 = *(int*)(l_fp - asBC_SWORDARG0(l_bc));
+ int i2 = asBC_INTARG(l_bc);
+ if( i1 == i2 ) *(int*)&m_regs.valueRegister = 0;
+ else if( i1 < i2 ) *(int*)&m_regs.valueRegister = -1;
+ else *(int*)&m_regs.valueRegister = 1;
l_bc += 2;
}
break;

case asBC_CMPIf:
{
- float f = *(float*)(l_fp - asBC_SWORDARG0(l_bc)) - asBC_FLOATARG(l_bc);
- if( f == 0 ) *(int*)&m_regs.valueRegister = 0;
- else if( f < 0 ) *(int*)&m_regs.valueRegister = -1;
- else *(int*)&m_regs.valueRegister = 1;
+ float f1 = *(float*)(l_fp - asBC_SWORDARG0(l_bc));
+ float f2 = asBC_FLOATARG(l_bc);
+ if( f1 == f2 ) *(int*)&m_regs.valueRegister = 0;
+ else if( f1 < f2 ) *(int*)&m_regs.valueRegister = -1;
+ else *(int*)&m_regs.valueRegister = 1;
l_bc += 2;
}
break;
[/source]


PS. If you feel like setting one up, github is actually quite amazing for open source projects - it makes contributing a much smoother process on both ends. As a contributor I could clone the official repository and use version control myself instead of having to submit loose patches, and as a developer all you need to do is look at the pull request, inspect the patches and press the nice accept button instead of having to apply a bunch of separate patches and create commits. Just a suggestion.

This was definitely not intentional from my part, nor was it an attempt to optimize the code.

Thanks for the patch. I'll have it applied in the next check-in.


Maybe I'll look into using github in the future, others have made the same suggestion before. It's just that I'm comfortable with how things are now and don't really feel a need to change it myself. The act of manually merging the patches I receive gives me a better chance to actually analyse what is being modified, to think about the impacts it will have.

I've checked in the patch to revision 1414.

Thanks,
Andreas