alliedmodders / amxmodx

AMX Mod X - Half-Life 1 Scripting and Administration
http://www.amxmodx.org/
496 stars 197 forks source link

Critical bug in Enum + Float array usage #1074

Open justgo97 opened 1 year ago

justgo97 commented 1 year ago

I'm using latest AMXX version and by chance I stumbled across this weird behavior with using enums with Float arrays causing werid outputs, can anyone elaborate on what's happening in this code:

#include <amxmodx>

enum {
    X_AXIS = 0,
    Y_AXIS = 1,
    Z_AXIS = 2
}

#define AXIS_X 0
#define AXIS_Y 1
#define AXIS_Z 2

public plugin_init() {
    server_print("Enum + Float Array test")
    new Float:VecF[3]

    VecF[X_AXIS] = 5.0;
    VecF[Y_AXIS] = 15.0;
    VecF[Z_AXIS] = 25.0;

    server_print("Original VecF:%f-%f-%f", VecF[0], VecF[1], VecF[2])

    VecF[X_AXIS] += 5.0;

    server_print("Updated VecF:%f-%f-%f", VecF[0], VecF[1], VecF[2])

    server_print("Define + Float Array test")

    VecF[AXIS_X] = 5.0;
    VecF[AXIS_Y] = 15.0;
    VecF[AXIS_Z] = 25.0;

    server_print("Original VecF:%f-%f-%f", VecF[0], VecF[1], VecF[2])

    VecF[AXIS_X] += 5.0;

    server_print("Updated VecF:%f-%f-%f", VecF[0], VecF[1], VecF[2])
}

Server output:

Enum + Float Array test
Original VecF:5.000000-15.000000-25.000000
Updated VecF:1084227584.000000-15.000000-25.000000
Define + Float Array test
Original VecF:5.000000-15.000000-25.000000
Updated VecF:10.000000-15.000000-25.000000

As you can see altering the variable with the help of the enum gives a garbage value, but with define everything works perfect, can the compiler throw a warning for such a case at least?

Giferns commented 1 year ago

Compile your code and then decompile it via https://headlinedev.xyz/lysis/ You will see

 new MaxClients;
new MapName[64];
new String:NULL_STRING[4];
new Float:NULL_VECTOR[3];
Float:operator+(Float:,_:)(Float:oper1, oper2)
{
    return floatadd(oper1, float(oper2));
}

public plugin_init()
{
    server_print("Enum + Float Array test");
    new Float:VecF[3] = 0.0;
    VecF[0] = 5.0;
    VecF[1] = 15.0;
    VecF[2] = 25.0;
    server_print("Original VecF:%f-%f-%f", VecF, VecF[1], VecF[2]);
    VecF[0] = 5.0 + VecF[0];
    server_print("Updated VecF:%f-%f-%f", VecF, VecF[1], VecF[2]);
    server_print("Define + Float Array test");
    VecF[0] = 5.0;
    VecF[1] = 15.0;
    VecF[2] = 25.0;
    server_print("Original VecF:%f-%f-%f", VecF, VecF[1], VecF[2]);
    VecF[0] = floatadd(1084227584, VecF[0]);
    server_print("Updated VecF:%f-%f-%f", VecF, VecF[1], VecF[2]);
    return 0;
}
Giferns commented 1 year ago

amxx uses natives for float calculations. enum broke it somehow (X_AXIS is non-tagged so VecF[X_AXIS] interpreted as int value), and you got wrong result

#include <amxmodx>

public plugin_init() {
    new Float:fOne = 1.0
    new Float:fTwo = 2.0
    new Float:fResult = fOne + fTwo
    server_print("%f", fResult)
}
 new MaxClients;
new MapName[64];
new String:NULL_STRING[4];
new Float:NULL_VECTOR[3];
public plugin_init()
{
    new Float:fOne = 1.0;
    new Float:fTwo = 2.0;
    new Float:fResult = floatadd(fOne, fTwo);
    server_print("%f", fResult);
    return 0;
}
dvander commented 1 year ago

Enums have very weird interactions with arrays. Try removing the tag (eg _:X_AXIS). Or, use enum structs.

justgo97 commented 1 year ago

Enums have very weird interactions with arrays. Try removing the tag (eg _:X_AXIS). Or, use enum structs.

True, and that's what I'm going to do for now, But I think a warning from the compiler about such a behavior would be great, I thought I had a bug in my code and kept rewirting stuff till I figured out it was an issue with enums.

dvander commented 1 year ago

I'll happily review a patch to add that warning/error. The enum/array code is very, very quirky though. I would not be able to write it off the top of my head. (And probably not off the bottom, either.)

Giferns commented 1 year ago

@justgo97 also you can replace

enum {
    X_AXIS = 0,
    Y_AXIS = 1,
    Z_AXIS = 2
}

by

enum {
    Float:X_AXIS = 0,
    Float:Y_AXIS = 1,
    Float:Z_AXIS = 2
}

now X_AXIS is tagged as Float so you will get proper behavior

 new MaxClients;
new MapName[64];
new String:NULL_STRING[4];
new Float:NULL_VECTOR[3];
public plugin_init()
{
    server_print("Enum + Float Array test");
    new Float:VecF[3] = 0.0;
    VecF[0] = 5.0;
    VecF[1] = 15.0;
    VecF[2] = 25.0;
    server_print("Original VecF:%f-%f-%f", VecF, VecF[1], VecF[2]);
    VecF[0] = floatadd(1084227584, VecF[0]);
    server_print("Updated VecF:%f-%f-%f", VecF, VecF[1], VecF[2]);
    server_print("Define + Float Array test");
    VecF[0] = 5.0;
    VecF[1] = 15.0;
    VecF[2] = 25.0;
    server_print("Original VecF:%f-%f-%f", VecF, VecF[1], VecF[2]);
    VecF[0] = floatadd(1084227584, VecF[0]);
    server_print("Updated VecF:%f-%f-%f", VecF, VecF[1], VecF[2]);
    return 0;
}