Don't think I'm handling objects properly...

Started by
13 comments, last by WitchLord 10 years, 1 month ago

So here's the rub. I'm working on a json/angelscript combo system as part of weapon design. The JSON defining the script block and function to call, which uses the signature "void call(gentity_t@)". The problem is, I'm not sure if I'm handling the object correctly. gentity_t is just a simple struct (but it isn't asOBJ_POD as it contains pointers to some other things within). The script will modify certain aspects of the gentity_t, which is meant to be passed in as a pointer, as well as execute several global functions which take the gentity_t as an argument.

Right, so here's how I'm registering my object:

scrpEngine->RegisterObjectType("gentity_t", sizeof(gentity_t), asOBJ_REF | asOBJ_NOCOUNT);

and here's how I'm calling the script:

bool TryNewDempRoutine(gentity_t *ent) {
    if(ent->s.weapon != WP_DEMP2) {
        return false;
    }
    G_ExecuteASVoid("void DEMP2Test(gentity_t@ ent)", 1, ent);
    return true;
}

(here's G_ExecuteASVoid: )

void G_ExecuteASVoid(const char* funcDeclaration, int numArgs, ...) {
    string s = funcDeclaration;
    auto paramStart = s.find_first_of('(')+1;
    auto paramEnd = s.find_last_of(')')-paramStart;
    string params = s.substr(paramStart, paramEnd);
    vector<string> arguments;

    asIScriptFunction *func = scrpEngine->GetModule(0)->GetFunctionByDecl(funcDeclaration);
    scrpContext->Prepare(func);
    auto argSplitter = params.find_first_of(',');
    while(argSplitter != params.npos) {
        string thisArgument = params.substr(0, argSplitter-1);
        arguments.push_back(thisArgument);
        params+=argSplitter;
        argSplitter = params.find_first_of(',')+1;
    }
    arguments.push_back(params);

    va_list        argptr;
    va_start(argptr, numArgs);

    void* arg;
    for(int i = 2; i < numArgs+2; i++) {
        arg = va_arg(argptr, void*);
        AS_SetArgBasedOnType(&arg, arguments.at(i-2), i-2);
    }
    scrpContext->Execute();
}

Here's my angelscript file:

void FireDemp2(gentity_t@ ent) {
    vec3_t start;
    int damage = GetWeaponDamage(WP_DEMP2);
    Com_Print("test4\n");
    VectorCopy(wpMuzzle, start);    // Copy value of wpMuzzle to start
    Com_Print("test5\n");
    WP_TraceSetStart(ent, start, vec3_origin, vec3_origin);
    Com_Print("test6\n");
    gentity_t@ missile = CreateMissile(start, wpFwd, 1800, 10000, ent);
    Com_Print("test7\n");
    missile.s.weapon = WP_DEMP2;
    Com_Print("test8\n");
    VectorSet( missile.maxs, 2.0, 2.0, 2.0 );
    Com_Print("test9\n");
    VectorScale( missile.maxs, -1, missile.mins );
    Com_Print("test10\n");
    
    missile.damage = damage;
    Com_Print("test11\n");
    missile.dflags = 128; // Lazy, don't feel like creating an enum for this right now
    Com_Print("test12\n");
    missile.methodOfDeath = 12; // Also lazy and don't feel like creating an enum for this right now either
    Com_Print("test13\n");
    missile.clipmask = 263041; // BLARGH SO MUCH TO DO OMG
    Com_Print("test14\n");
}

void DEMP2Test(gentity_t@ ent) {

    // Increase number of shots fired (for loading screen statistics
    ent.client.ps.accuracy_shots++;
    
    // From our viewangles, generate three vectors -> wpFwd, wpVright, wpUp (all are global)
    Com_Print("test\n");
    AngleVectors(ent.client.ps.viewangles, wpFwd, wpVright, wpUp);
    Com_Print("test2\n");
    // Given the three vectors, calculate our muzzle point and stuff it into wpMuzzle (also global)
    CalcMuzzlePoint(ent, wpFwd, wpVright, wpUp, wpMuzzle, 0);
    Com_Print("test3\n");
    // Call our funtion which fires the DEMP2
    FireDemp2(ent);
}

The script only gets as far as "test2" before it fails on CalcMuzzlePoint, that function being a global function:

void CalcMuzzlePoint( gentity_t *const ent, vec3_t wpFwd, vec3_t right, vec3_t wpUp, vec3_t muzzlePoint, float lead_in )
....
scrpEngine->RegisterGlobalFunction("void CalcMuzzlePoint(gentity_t@ ent, vec3_t fwd, vec3_t right, vec3_t up, vec3_t muzzle, float lead_in)", asFUNCTION(CalculateMuzzlePoint_AS), asCALL_CDECL);

I suspect that I'm doing this completely wrong and that maybe I should be using something like CScriptHandle, but I'm not too sure where to go with this.

Advertisement

What you're doing is not completely wrong (though it is quite inefficient, in that you do a lot of string processing with each call). The problem is probably some small detail, e.g. a missing dereference of the pointer to the object.

Can you show the implementation of AS_SetArgBasedOnType?

Based on how you're calling it, it should be doing something like this in order to be correct:

void AS_SetArgBasedOnType(void **arg, const string &type, int index)
{
   // check if the parameter is a handle (it's not really enough to do a find, but you get the idea)
   if( string.find("@") != string::npos ) 
   {
      // Call SetArgObject and inform the address of the object
      scrpContext->SetArgObject(index, *arg);
   }
 
   ...
}

For handles it would also be valid to use SetArgAddress.

Is the CalcMuzzlePoint() function called from the script, or does it crash before that? If it is called, is the pointer you receive the expected one?

Curiosity: Why is your loop that calls AS_SetArgBasedOnType going from 2 to numArgs + 2, when you subtract 2 whenever you use the index? ;)

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

https://github.com/eezstreet/OpenJK/blob/jk2hd/codeJK2/game/g_angelscript.cpp#L26-#L41

^ AS_SetArgBasedOnType, as well as the other code.

CalcMuzzlePoint is called from the script, and the first arg (a pointer to a gentity_t*, which SHOULD be the same pointer as to what gets passed into the script) is an invalid pointer -- as I recall from debugging it was pointing to 0xcccccccc (Windows magic number for unallocated heap memory I believe) and therefore crashes inside of CalcMuzzlePoint.

(CalcMuzzlePoint is in g_weapon.cpp)

Yeah, the string processing is a little slow, I will admit that. I mainly did it that way so I can insert a large batch of functions without having to customize the calls too much. smile.png

And yeah, thanks for pointing that out.

As I suspected. Your AS_SetArgBasedOnType is not dereferencing the pointer when calling SetArgAddress.


void AS_SetArgBasedOnType(void* arg, const string& argument, int argNum) {
  if(!argument.compare("byte") || !argument.compare("int8") || !argument.compare("uint8") || !argument.compare("bool"))
    scrpContext->SetArgByte(argNum, *(byte*)arg);
  else if(!argument.compare("double"))
    scrpContext->SetArgDouble(argNum, *(double*)arg);
  else if(!argument.compare("dword") || !argument.compare("int") || !argument.compare("uint"))
    scrpContext->SetArgDWord(argNum, *(DWORD*)arg);
  else if(!argument.compare("float"))
    scrpContext->SetArgFloat(argNum, *(float*)arg);
  else if(!argument.compare("int16") || !argument.compare("uint16"))
    scrpContext->SetArgWord(argNum, *(short*)arg);
  else if(argument.find_first_of('@') != argument.npos || argument.find_first_of('&') != argument.npos)
    scrpContext->SetArgAddress(argNum, *(void**)arg);  // Must dereference to give the address of the object
  else
    scrpContext->SetArgObject(argNum, *(void**)arg); // Must dereference to give the address of the object
}

?

?

Try making the change I did above and let me know if it works.?

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

Works without a hitch. However, the other arguments being passed into CalcMuzzlePoint (vec3_t global variables) are null pointers. :<

Also, the accuracy_shots property seems to be connected to the wrong property (connected to stats[STAT_HEALTH] instead of persistant[PERS_ACCURACY_SHOTS])

How is the vec3_t declared? You've registered it with asOBJ_APP_CLASS, with which you're telling AngelScript that this type doesn't have any default constructor nor any copy constructor. Is that really the case? If you don't register the value type with the correct asOBJ_APP_xxx flags AngelScript will not know how the type should be passed by value to native functions. If you're using a C++11 capable compiler you can use the GetTypeTraits<T>() function from scripthelpers add-on to automatically get the correct asOBJ_APP_xxx flags.

How is the playerState_t declared? The asOFFSET(playerState_t, persistant[PERS_ACCURACY_SHOTS]) macro expands to the following:

((size_t)(&reinterpret_cast<playerState_t*>(100000)->persistant[PERS_ACCURACY_SHOTS])-100000)

Is that giving the correct offset? I assume the persistant member is a static array, and not something like std::vector.

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

vec3_t is just a typedef float[3].

playerState_t is defined here:

https://github.com/eezstreet/OpenJK/blob/jk2hd/code/qcommon/q_shared.h#L1911-2212

(__NO_JK2 preprocessor is disabled)

Hmm. Now I'm not so sure.

What compiler and target platform (OS, CPU) are you using? Which AngelScript version are you using?

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

MSVC 2010 and latest angelscript.

I did some tests, and unfortunately vec3_t is handled completely differently from any other type in C++ when passed by value to a function. All other types are passed by value as-is in MSVC, but the vec3_t is internally passed by reference.

It is necessary to implement a completely new asOBJ_APP_xxx flag and the accompanying code to be able to support this type in native calling conventions.

You can trick AngelScript, by registering the function like this:

engine->RegisterGlobalFunction("void CalcMuzzlePoint( gentity_t@ ent, const vec3_t &in wpFwd, const vec3_t &in right, const vec3_t &in wpUp, const vec3_t &in muzzlePoint, float lead_in )", asFUNCTION(CalcMuzzlePoint), asCALL_CDECL);

This is not portable though, because other compilers or platforms will possibly handle the vec3_t type differently.

If you want a portable solution, you can either change the vec3_t type to be a struct instead of an array, or you can use wrappers to avoid passing the vec3_t by value.

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

This topic is closed to new replies.

Advertisement