[KeldorKatarn]

But aside from that problem: Anyone got an idea yet where that null/dangling pointer could be coming from?

[karajorma]

In that case I'm rather stumped at to the cause of your problem. Obviously something is wrong with your setup somewhere cause you shouldn't be able to write code that locks up the debugger in that way.

[Wanderer]

When exactly does that crash appear? Is it related to a specific ship class (ie. errors in the pof file?)

[KeldorKatarn]

As I said, I cannot easily reproduce this. It happens with a mission that I used to test a model with yes, but I checked that model in other missions before and I also see no probelms in the pof.
I cannot rule out a problem with the POF however it shouldn#t cause a null pointer crash even if it is the model, so somewhere in the code something doesn't get checked.

I'd dig further into this if I could, but the mouse lock makes that near impossible.

I always have to wait a while in that mission and run it a few times. probably a certain shield face has to get hit for this to happen so it is hard to really reproduce it. if it is an error in the pof i don't find it, the shield seems optically ok and the debug build doesn't report any parsing errors.

[KeldorKatarn]

Why is this commented out in read_model_file()??

Code: [Select]

    for ( j = 0; j < 3; j++ ) {
        pm->shield.tris[i].verts[j] = cfread_int( fp );  // read in the indices into the shield_vertex list
        /*
#ifndef NDEBUG
        if (pm->shield.tris[i].verts[j] >= nverts)
            if (!warning_displayed) {
                warning_displayed = 1;
                Warning(LOCATION, "Ship %s has a bogus shield mesh.\nOnly %i vertices, index %i found.\n", filename, nverts, pm->shield.tris[i].verts[j]);
            }
#endif
        */
    }


[KeldorKatarn]

This should possibly be:

Code: [Select]

for ( j = 0; j < 3; j++ ) {
    pm->shield.tris[i].verts[j] = cfread_int( fp ); // read in the indices into the shield_vertex list

    if (pm->shield.tris[i].verts[j] >= pm->shield.nverts) {
        Warning(LOCATION, "Ship %s has a bogus shield mesh.\nOnly %i vertices, index %i found.\n", filename, pm->shield.nverts, pm->shield.tris[i].verts[j]);
    }
}

Warning() already contains a #ifdef DEBUG, so there's no need to repeat the preprocessor directive here.

[KeldorKatarn]

Ran a debug build with this code in it. Didn't report any shield error. but this warning should stay in anyway.

I'll keep looking where this error might occur.

[portej05]

Warning() already contains a #ifdef DEBUG, so there's no need to repeat the preprocessor directive here.

I'm not familiar with this section of code, but for performance reasons it is best to avoid any debugging code whatsoever in very regularly called code, hence the ifdef.

[KeldorKatarn]

First of all this is only executed in DEBUG builds since the code of that function contains a #ifdef.

So in retail this code will simply do nothing at all and not even show up in the machine code. but in DEBUG builds this is essential to check a shield for correctness.

This wasn't the cause for the crash but this should stay in.

[portej05]

Actually, your revised version has upwards of 7 dereferences, two comparisons and that is all done 3 times over.
In critical path code, that is rather a lot, and has a lot of potential to cause cache misses and going to memory for information.
Granted, it's small, but game code generally must be optimised to hell.

EDIT: Also, it's a warning, not an error.

[Wanderer]

Can you reproduce the problem? If so check what exactly goes wrong in here (ie. values and/or validities of both 'verts' and 'stp').

Code: [Select]

sv = &verts[stp->verts[i]];

And from there trace the problematic spot and try to verify what exactly is wrong.

[Spicious]

Actually, your revised version has upwards of 7 dereferences, two comparisons and that is all done 3 times over.
In critical path code, that is rather a lot, and has a lot of potential to cause cache misses and going to memory for information.
Granted, it's small, but game code generally must be optimised to hell.

The right side looks like it was stored in a local in the original code to avoid that problem. The left side is written to immediately before so caching shouldn't be a problem. It looks like it would almost get optimised out too.

[KeldorKatarn]

Can you reproduce the problem? If so check what exactly goes wrong in here (ie. values and/or validities of both 'verts' and 'stp').

Code: [Select]

sv = &verts[stp->verts[i]];

And from there trace the problematic spot and try to verify what exactly is wrong.

You have been following the thread have you? I told you that I cannot check anything, since I cannot get the mouse focus back for Visual Studio

[Wanderer]

Does using windowed mode affect that?

[KeldorKatarn]

I'll try that. The problem, as I mentioned, is, that it is really hard to reproduce to begin with. it might be that a certain shield triangle must be hit and I don't even know on what ship yet.
So I kinda have to restart the mission over and over and watch the AI fight until it finally crashes.

[Wanderer]

Yeah... Well i hope you can get it to work... you should be able to id the model (for example) by tracing the calls via stack to create_shield_explosion() function and accessing the polymodel struct (pm).

[chief1983]

I was also going to suggest -window, it's a necessity for any serious debugging.  No need for alt-tab even when it's enabled, if a crash occurs you should automagically have full control of your mouse again.

[KeldorKatarn]

I couldn't get the game to crash again (simply too random which triangle is hit) but I managed to find out what caused the crash.

The Sabre model of Saga had a buggy shield. Somehow the pof contained one single wrong triangle neighbor index into the shield-triangles array. The value was something like 2 million something or so while there were around 650 triangles.

Loading the model in PCS2 and re-saving it seems to have corrected the error. The warning didn't show up anymore and I believe this fixes the crash also.

To prevent such undetected errors, that lead to an access violation crash, in the future I'd really recommend adding the checks that I put in the code to find them. One of those checks is right now already in the code but commented out, the neighbor indices however are not checked at all.
Such checks are necessary in the debug builds so developers can make sure the models are alright.

I attached a patch file with the suffested added checks (both of them in #ifndef DEBUG clauses so they don't harm any performance of the retail build).

[attachment has decomposed]

[chief1983]

Is that done during load or realtime in game?  I can't imagine an extra calculation is that big of a deal in debug just during load.

[KeldorKatarn]

It is done on mission load from what I can see. It is the read_model_file() function. Nothing realtime.