Fix CTD under Release configuration.

[awdavies]

CTD occurs when compiling under Release configuration.

Ideally this shouldn't happen, as we would like to have a debug free binary for releases :)

This occurs during the scheduler's update function call (stack trace in comments).

[awdavies]

Stack trace:

0023:06A20C69 xrGame.dll, CEntity::shedule_Update(), d:\src\xrgame\entity.cpp, 328
0023:06A2371D xrGame.dll, CEntityAlive::shedule_Update(), d:\src\xrgame\entity_alive.cpp, 210
0023:068A5EFA xrGame.dll, CActor::shedule_Update(), d:\src\xrgame\actor.cpp, 1222
0023:00457965 xrEngine.exe, CSheduler::Update(), d:\src\xrengine\xrsheduler.cpp, 454
0023:06A3A700 xrGame.dll, CGamePersistent::OnFrame(), d:\src\xrgame\gamepersistent.cpp, 662
0023:0040C111 xrEngine.exe, CRegistrator::Process(), d:\src\xrengine\pure.h, 86
0023:00435AAA xrEngine.exe, CRenderDevice::FrameMove(), d:\src\xrengine\device.cpp, 486
0023:00435D09 xrEngine.exe, CRenderDevice::on_idle(), d:\src\xrengine\device.cpp, 263
0023:00435C3A xrEngine.exe, CRenderDevice::message_loop(), d:\src\xrengine\device.cpp, 380
0023:00435B8E xrEngine.exe, CRenderDevice::Run(), d:\src\xrengine\device.cpp, 424
0023:0046A92D xrEngine.exe, Startup(), d:\src\xrengine\x_ray.cpp, 383
0023:00469685 xrEngine.exe, WinMain_impl(), d:\src\xrengine\x_ray.cpp, 873
0023:004691D6 xrEngine.exe, WinMain(), d:\src\xrengine\x_ray.cpp, 942
0023:00432BF3 xrEngine.exe, __tmainCRTStartup(), f:\dd\vctools\crt\crtw32\dllstuff\crtexe.c, 618
0023:7577338A kernel32.dll
0023:77D29F72 ntdll.dll
0023:77D29F45 ntdll.dll

[error][ 87] : The parameter is incorrect.

[andrew-boyarshin]

Pushing workaround for that problem...

[ghost]

@STALKER2010 : Before the repo revert, I tried your workaround in the game, it worked perfectly.

[andrew-boyarshin]

@nitrocaster requires us to write only top-quality code. Well, my solution
wasn't perfect, but it worked. And worked stable. But now - waiting for
those, who can write it better, than me.
On Oct 5, 2014 11:15 PM, "Bangalore1010" notifications@github.com wrote:

@STALKER2010 https://github.com/STALKER2010 : Before the repo revert, I
tried your workaround in the game, it worked perfectly.

—
Reply to this email directly or view it on GitHub
#11 (comment).

[ghost]

Oooops.. i have to take that back, something is not working perfectly, and i'm not sure what caused this error. The compile with release config went fine, but I'm using a mod, that comes with extra patch files (textures packed into xpatch_03.db, xpatch_04.db etc. files). The game engine doesn't read these db files, and doesnt find any of the packed textures.
Ofc I tested with vanilla+other binaries, and all is working good, but with that reverted commit this happened.

[andrew-boyarshin]

Are you sure, that it is caused by my commit? Try compiling without it. It
should affect only killing entities(mutants and NPCs), if it should at all.
Anyway, I'm not responsible if your computer will burn down or at all
anything bad happens. ;)
On Oct 5, 2014 11:38 PM, "Bangalore1010" notifications@github.com wrote:

Oooops.. i have to take that back, something is not working perfectly, and
i'm not sure what caused this error. The compile with release config went
fine, but I'm using a mod, that comes with extra patch files (textures
packed into xpatch_03.db, xpatch_04.db etc. files). The game engine doesn't
read these db files, and doesnt find any of the packed textures.
Ofc I tested with vanilla+other binaries, and all is working good, but
with that reverted commit this happened.

—
Reply to this email directly or view it on GitHub
#11 (comment).

[ghost]

I try to compile with the latest commit, and i didn't say your commit caused it. ;)

[ghost]

@STALKER2010, it's not your commit's fault.

I have still the same error with the latest commit:
Vanilla COP: same xrGame crash at new game, as above.
Modded game with added xpatch__.db files: in the main menu, missing textures in the log, the engine doesn't read the extra xpatch__.db files with textures, at new game xrGame or missing textures crash.

Modded game with @STALKER2010's xrGame crash fix: in the main menu, missing textures in the log, the engine doesn't read the extra xpatch_*.db files, i can't start a new game and load a map because of missing textures.

Am i the only one with this error?

[nitrocaster]

Modded game with @STALKER2010's xrGame crash fix: in the main menu, missing textures in the log, the engine doesn't read the extra xpatch_*.db files, i can't start a new game, and load a map because of missing textures.

You can't start new game because of CTD being discussed here or for some other reason?

I've build release by myself and got the same CTD. I've debugged it and found some problems that may cause such crash. Will elaborate on that soon.

[ghost]

@STALKER2010's fix worked like charm, but i couldn't start a new game, because of engine doesn't read my xpatch_03.db-xpatch_07.db files (which results missing textures ctd on not-vanilla map load).
I didn't have problem with other binaries, they are reading in the xpatch_03.db-xpatch_07.db files.
I made these db files with COP's xrCompress.

[awdavies]

Investigating with a couple breakpoints shows that a member variable m_entity_condition in Entity.h somehow is either set to NULL, or is never initialized. The error is essentially a segfault.

Checking through the stack trace to see whether this value is set on accident or if there's a potential race condition.

[awdavies]

Alright, I think I found it. There appears to be a logic bug in CEntity::create_entity_condition

When m_entity_condition is created inside of the DLL, it looks like it's casted to an entity condition class only when the parameter passed to the function is NULL, which would naturally be A Bad Thing ™

[nitrocaster]

There appears to be a logic bug in CEntity::create_entity_condition

If so, why doesn't it occur in Debug configuration?

[awdavies]

I'm not yet sure. I'm looking around at the header files within the stack trace that have

#ifdef DEBUG
    /* some stuff */
#endif

[nitrocaster]

Note that debug configuration hides plain dynamic_cast under smart_cast.

[awdavies]

Scratch that, I misread the code :p

I'm now not really sure what causes the issue. It looks like either smart_cast does it in the creation function, or it's something to do with the inherited schedule_update function.

[awdavies]

Okay, so the issue appears to be something wrong with smart_cast

It's right here. I'm going to take a stab at refactoring the code a tiny bit so as not to use smart_cast since its use in this case is not very clean as far as coding conventions go (casting to a subclass shouldn't be necessary, especially since the pointer is expecting the super class interface anyway).

I'll let you all know how the patch fares in Debug compilation as well as when running Dedi.

[awdavies]

Okay, so reformatting the code to a slightly more sane interface hasn't really fixed the problem. Due to the fact that I'd rather not spend several days looking at how the scheduler fetches/creates/initializes objects, I think it might just be sufficient for now to do a NULL check on m_entity_condition.

My gut tells me this is just a race condition, and that if we defer the death check upon starting a new game in the shedule_Update function while m_entity_condition is NULL, we should be fine for now.

[nitrocaster]

@Bangalore1010, have you managed with that problem with .db files?

[ghost]

@nitrocaster : no, i didn't work with/on this repo.

[nitrocaster]

@Bangalore1010 There's a special switch -auto_load_arch that may help. Try it.

[ghost]

Finally the problem is solved, the engine from new bins can read db files, ofc.
In a mod i used a custom main menu, which reads the texture from the ui_mainmenu3.xml file, but the engine seek the texture from ui_mainmenu.xml file, and this made the new menu texture+background video disappear. It wasn't a problem with vanilla bins or with VS2008 bins, the new menu appeared right.
It was a false alarm about "not reading db".

[BeaconDev]

Bangalore, will you be committing yours and Alun's source changes to this project? They would be helpful for all.

[avoitishin]

Here is what was causing this issue (and potentially many others):

m_textures.insert(mk_pair(id,info));

This line in UITextureMaster.cpp is responsible for adding texture information to global m_textures storage. This storage is a derivative of standard C++ data structure std::map (basically a key-value pair) and by design insert method will NOT override values if specific key is already present in the map. The only way to overcome this limitation is to explicitly test for key presence by using std::map::find method and then either use [key] operator to update value for key or insert to insert new key-value pair. Also in C++11 there is more efficient method for inserting into map - std::map:emplace which removes the need to call mk_pair method and construct std::pair object.

/* avo: fix issue when values were not updated (silently skipped) when same key is encountered more than once. This is how std::map is designed. Also used more efficient C++11 std::map::emplace method instead of outdated std::pair::make_pair */
/* XXX: avo: note that map.insert(mk_pair(v1,v2)) pattern is used extensively throughout solution so there is a good potential for other bug fixes/improvements */
   if (m_textures.find(id) == m_textures.end())
       m_textures.emplace(id, info);
   else
       m_textures[id] = info;
   //m_textures.insert(mk_pair(id,info)); // original GSC insert call
   /* avo: end */

[nitrocaster]

To @avoitishin : That's unrelated to the problem that we discussed here, though it have to be fixed.

[avoitishin]

This was in response to Bangalore's post about ui_mainmenuX.xml not picking up texture information. Already fixed in working brach of my fork and I agree with you that it should be separated into it's own issue