Crash on level load & terrain painter open - FIXED

#1 *posted in the thread that this was linked instead

#2 Jason was saying the code change doesn't fix the problem for him.
(terrain painter being left open, causing crash on mission load)

I'm gona install VS on my laptop and try this myself.

btw, i've found even an easier way to make it crash.

Start T3D, goto world editor, select terrain painter, then
open a mission.. can be any mission, even the one your currently
editing.

deleting, or editing the prefs.cs (see below) is still a workaround for now.

change this:

$pref::WorldEditor::currentEditor = "TerrainPainterPlugin";

to this:

$pref::WorldEditor::currentEditor = "WorldEditorInspectorPlugin";

#3 Ok, done a compile myself.

jaimi's fix DOES enable me to load a level if 'TerrainPainterPlugin'
is set in the prefs, but only when first running T3D, or from loading
a mission in T3D, so long as the terrain painter is not open.

BUT, the problem still exits if you open a mission
with the terrain painter open, as I described above:

Quote:
Start T3D, goto world editor, select terrain painter, then
open a mission.. can be any mission, even the one your currently
editing.

I can confirm that this 'new' problem can be reproduced in beta 1,2,3 & 4
using the FULL template, using the method I described above.

Hewster

#4 Would be great if someone could figure out the "common" cause for this because I still can't reproduce this crash.

#5 Here is a vid of how I create the crash:

My machine:
HP Pavilion laptop
Vista(SP2) Home Premium 32bit
3Gb ram
Nivdia 9600GT M with 512mb dedicated vram

Will post my call stack later..

#6 What if you cleared $pref::WorldEditor::currentEditor on initial start up and forced the editors to close before opening a new level?

#7 If I can duplicate the issue, I'll see if I can figure it out later this evening. The above fix is for the issue that if you try to load a terrain while in the terrain editor (the original complaint) it could crash. The mFile member variable was never initialized, and there was code checking to see if it was NULL before working on it. Unfortunately, since it wasn't initialized, sometimes it would be NULL (correct) and work, and sometimes it would just have random crap in it (crashes).

#8 Steven Garcia asked me to post more info regarding this crash, so here goes :)

Things to note about the following vid:
1) was done in VS2008ex in a VMware Virtual machine running WinxpSP3 32bit
this prolly accounts for the dark terrain you see (their DX9 support is
limited ATM)
2) The project I'm debugging is from the FULL template
3) I've speeded up the vid somewhat to allow easier viewing

Download HERE (DivX codec)

Note the 2 "Vectors: verts, norms, tverts, tangents should all be the same size"
asserts.

The log file is HERE

and the 3 call stacks can be found in THIS file.

Let me know if you need any more info.

Hewster

#9 @Hewster's vid.
Ah, now doing it like that I get crash.

#10

Quote:
Note the 2 "Vectors: verts, norms, tverts, tangents should all be the same size"
asserts.

FYI - These two asserts are appearing while loading this file:
arts/shapes/weapons/crossbow/weapon.dts
while executing this script file:
art/datablocks/weapons/crossbow.cs

The assert is happening in this function:

void TSMesh::_convertToAlignedMeshData

in this file:

TsMesh.cpp

Later on, it looks like it is actually OK for TVerts to have a smaller size, as it is checked in the copy loop:

if(i < tverts.size())
v.tvert(tverts[i]);

So, at least for now, you can just comment that out in this function:

void TSMesh::_convertToAlignedMeshData( TSMeshVertexArray &vertexData, const Vector<Point3F> &_verts, const Vector<Point3F> &_norms )

AssertFatal(_verts.size() == _norms.size() && /*_verts.size() == tverts.size()  &&*/
               _verts.size() == tangents.size(), "Vectors: verts, norms, tverts, tangents should all be the same size");

#11 The crash is happening here:

TerrainBlock* TerrainEditor::getClientTerrain( TerrainBlock *serverTerrain ) const
{
   if ( !serverTerrain && !mActiveTerrain )
      return NULL;

   serverTerrain = mActiveTerrain;

   // do the client..

   NetConnection * toServer = NetConnection::getConnectionToServer();
   NetConnection * toClient = NetConnection::getLocalClientConnection();

   S32 index = toClient->getGhostIndex(serverTerrain);

   return(dynamic_cast<TerrainBlock*>(toServer->resolveGhost(index)));
}

This line:

S32 index = toClient->getGhostIndex(serverTerrain);

is returning -1, and then it's dynamic cast fails because resolveGhost cannot find it:

S32 NetConnection::getGhostIndex(NetObject *obj)
{
   if(!isGhostingFrom())
      return obj->mNetIndex;
   S32 index = obj->getId() & (GhostLookupTableSize - 1);

   for(GhostInfo *gptr = mGhostLookupTable[index]; gptr; gptr = gptr->nextLookupInfo)
   {
      if(gptr->obj == obj && (gptr->flags & (GhostInfo::KillingGhost | GhostInfo::Ghosting | GhostInfo::NotYetGhosted | GhostInfo::KillGhost)) == 0)
            return gptr->index;
   }
   return -1;
}

I'm assuming it's because the terrain has not been ghosted yet. I am not familiar enough with this code to whip out a quick fix.

(beyond just inserting a check right before the dynamic cast).

if (index == -1)
return NULL;

However, I do not know how this return will propagate up the chain, perhaps causing other issues.

#12 Ok: Modifying TerrainEditor::getClientTerrain as specified above fixed the crash (FOR ME) while loading with the terrain painter open. Here is the new function:

TerrainBlock* TerrainEditor::getClientTerrain( TerrainBlock *serverTerrain ) const
{
   if ( !serverTerrain && !mActiveTerrain )
      return NULL;

   serverTerrain = mActiveTerrain;

   // do the client..

   NetConnection * toServer = NetConnection::getConnectionToServer();
   NetConnection * toClient = NetConnection::getLocalClientConnection();

   S32 index = toClient->getGhostIndex(serverTerrain);

   if (index == -1)   // Added this
      return NULL;    // Added this

   return(dynamic_cast<TerrainBlock*>(toServer->resolveGhost(index)));
}

Note: There may be other crashes. If so, please post the exact steps to fix them, and I will look at those also.

#13 Looks like a good fix, however I am now seeing Terrain -> Smooth Heightmap twice in the editor button toolbar when using Terrain Editor (not Terrain Painter). Pretty sure it wasn't like that before.

Terrain Editor
File - Edit - View - Terrain - Terrain - Camera - Editors - Lighting - Help

#14 @Jaimi, re: crossbow.. I remember reading a thread about that issue,
odd that they added that check, I wonder if a dev can chime in as to why
this seems to be an unnecessary check. Thanks for the workaround :)

Re: terrain painter.. nice workaround here too, but I wonder if the real
problem is that editor loading is happening too early; perhaps studying
where the painter code is loaded when you click terrain painter, might give
a clue to why it craps out when you load a mission with the terrain painter
already active... I dunno, just 'free thinking' here !

I would think that, what ever dev implemented this would work this out
pretty quickly.

Seems odd the devs are keeping out of this bug, maybe its too low priority
for them, or the dev in question is on vacation.

Also odd that one of the members (who I believe) was experiencing this,
or the other painter bug, as gone silent, after such a hooharr about
getting it fixed! hayho.. kay sera sera

#15 Amusingly after restarting T3D the issue I mentioned above is no longer there.

#16 @Jaimie - Try this function instead...

TerrainBlock* TerrainEditor::getClientTerrain( TerrainBlock *serverTerrain ) const
{
   if ( !serverTerrain && !mActiveTerrain )
      return NULL;

   serverTerrain = mActiveTerrain; // WEIRD STUFF!

   return dynamic_cast<TerrainBlock*>( serverTerrain->getClientObject() );
}

A little gem i added into NetObject... the client object can get its local server object and the server object can get its local client object. Only works on local connections... but its safer to use than the ghost index method.

Let me know if this works for everyone.

#17 @Tom.. works for me :) it also negates the need for Jaimi's fix for
the other terrain painter issue described in my first post.

So unless anyone else chimes in that its still an issue for them,
I'll move this post to resolved.

Cheers,

Hewster

#18 @Hewster - I think it's still important to initialize the mfile var to null - it looks like it's valid to be unassigned (they check it everywhere to make sure it's not null anyway)

#19 @Jaimi.. fair comment

#20 Hum... setting mFile to NULL in TerrainBlock constructor is a curious fix.

mFile is a Resource<TerrainFile> which has its own constructor which should be initializing the resource handle to NULL already. If this does indeed fix a crash or bug... that means we have a bigger problem going on... memory corruption or some sort of template failure.

Does someone have repro steps that clearly shows this fixes a crash or bug?