View Issue Details

IDProjectCategoryView StatusLast Update
0005065The Dark ModCodingpublic03.02.2020 16:01
Reportergrayman Assigned Tocabalistic  
PrioritynormalSeveritycrashReproducibilityalways
Status resolvedResolutionfixed 
Product VersionSVN 
Target VersionTDM 2.08Fixed in VersionTDM 2.08 
Summary0005065: target_endlevel causes a crash
DescriptionI'm trying to transition from one map to another using the target_endlevel entity.

It works in 2.07, but crashes in the latest SVN (15740/8423).
Steps To ReproduceUse the attached pk4 to test.

The player starts behind two friendly AI (switchmaps1.map).

Walk around them to the door and frob it.

The game should switch to a different map (switchmaps2.map).

In 2.07, this works fine. In SVN, the crash occurs before the second map appears.
TagsNo tags attached.

Activities

grayman

grayman

28.11.2019 16:35

administrator  

switchmaps.pk4 (24,803 bytes)
stgatilov

stgatilov

08.12.2019 04:27

developer   ~0011898

I checked it some time ago, and as far as I remember, there crash was in VertexCache.
Assigned it to Duzenko, since he was the only one to change it after 2.07.
duzenko

duzenko

29.12.2019 09:14

developer   ~0011958

With com_smp 1 I get a hang in frontend wait
With com_smp 0 I get a crash with stack trace
> TheDarkModx64NoTools.exe!R_ReallyFreeStaticTriSurf(srfTriangles_s * tri) Line 377 C++
     TheDarkModx64NoTools.exe!R_FreeDeferredTriSurfs(frameData_t * frame) Line 471 C++
     TheDarkModx64NoTools.exe!R_ToggleSmpFrame() Line 187 C++
     TheDarkModx64NoTools.exe!idRenderSystemLocal::EndFrame(int * frontEndMsec, int * backEndMsec) Line 688 C++
     TheDarkModx64NoTools.exe!idSessionLocal::UpdateScreen(bool outOfSequence) Line 2740 C++
     TheDarkModx64NoTools.exe!idCommonLocal::Frame() Line 2512 C++
Both ways I'm not sure on a fix
Am I correct that fm's like NHAT should be broken now?
grayman

grayman

29.12.2019 10:48

administrator   ~0011965

No, NHAT is three different missions. Each mission knows nothing about the next.

I checked released missions, and the only one using target_endlevel is the "Sound Trainer". The first map includes helmet-less guards to test your blackjacking skills. Next to your starting position, a lever sends you to the second map, where you can test with helmeted guards.

That mission works fine in 2.07, but crashes with the current SVN, so it's a second data point.
cabalistic

cabalistic

29.12.2019 11:36

developer   ~0011969

If it crashes in "ReallyFreeStaticTriSurf" in one of the allocators, it may not have anything to do with the VertexCache at all, but potentially your threading changes in the frontend. I remember vaguely that in my own attempts to thread the frontend, I had removed/replaced all of these allocators because they were inherently not thread-safe. Of course, since you probably used a different approach, my findings from way back then may not apply, but it's just an idea. Either way, I don't think I can help you with this.
stgatilov

stgatilov

29.12.2019 11:50

developer   ~0011971

I think by default all parallel stuff is disabled. Especially with com_smp 0. Let's fix it first.
duzenko

duzenko

29.12.2019 12:31

developer   ~0011974

The revision 8477 seems to resolve the single thread case but I still think @cabalistic needs to look at frontend sync when changing maps on the fly
grayman

grayman

01.02.2020 01:32

administrator   ~0012160

As of 15781/8548, there's no crash with 'com_smp 0'.

It hangs during the second map load when 'com_smp 1'.

Since I was using 'com_smp 0' when I filed the report, I'll assume that something got fixed after that that at least allows 'com_smp 0' to work around the problem.
stgatilov

stgatilov

01.02.2020 11:11

developer   ~0012166

Last edited: 01.02.2020 11:11

View 2 revisions

I suggest replacing:
  MoveToNewMap( args.Argv(1) );
with:
  cmdSystem->BufferCommandText( CMD_EXEC_INSERT, ret.sessionCommand );
in idSessionLocal::RunGameTic.

This way map-changing commands produced by game code will be put for delayed execution as a console command.
Then map change will start from main thread instead of frontend.

The only problem is that game continues running for some time on frontend thread, and it continues to issue map change repeatedly.
Something has to be done to avoid it.
Another issue is that this way mission-loading GUI is displayed, while with direct call to MoveToNewMap picture freezes for some time, then jumps straight to "Hit Attack to start".

cabalistic

cabalistic

01.02.2020 16:06

developer   ~0012167

I decided not to change the logic, but instead give the Session a generic way to delay execution of certain code until after the current frame has finished and the two threads are joined. It's not super elegant, but I think it is a safer approach and might come in handy if we encounter another edge case.
(I do wonder why it was working in 2.07, though.)

Btw, is the absence of a loading screen really desired? Particularly if the load takes a while, I would prefer and expect a loading screen..
duzenko

duzenko

01.02.2020 17:15

developer   ~0012168

I vote in favor of the loading GUI for this. It could potentially take minutes to load the assets for the 'other' map from slow storage.
grayman

grayman

02.02.2020 02:32

administrator   ~0012169

If the mapper wants to have no loading screen appear for maps beyond the first, they can include a unique loading screen w/o an image or loading bar.

That's what I plan to do for mine, since the appearance of a loading screen "gets in the way" of what's happening. Perhaps a GUI that just says "Time passes ...".
stgatilov

stgatilov

02.02.2020 15:51

developer   ~0012170

Last edited: 03.02.2020 16:01

View 5 revisions

Well, I suggested using "console command" as a way to delay-execute code, but this will also do.

Two comments about fix:
1) The case with "died" command already worked normally, because it turns on GUI. I hope you checked that it still works with com_smp on and off.
2) Putting std::function-s into idList is the worst thing to do. Better put idStr or idCmdArgs instead. Passing callbacks long way and between threads feels bad.

P.S. I think idStrList should be stored, one string per command.

EDIT: I reworked it myself in svn rev 8650. I hope you don't mind.

Issue History

Date Modified Username Field Change
28.11.2019 16:35 grayman New Issue
28.11.2019 16:35 grayman File Added: switchmaps.pk4
28.11.2019 16:36 grayman Description Updated View Revisions
28.11.2019 16:36 grayman Steps to Reproduce Updated View Revisions
05.12.2019 21:54 grayman Severity normal => crash
08.12.2019 04:23 stgatilov Assigned To => duzenko
08.12.2019 04:23 stgatilov Status new => assigned
08.12.2019 04:27 stgatilov Note Added: 0011898
29.12.2019 09:14 duzenko Note Added: 0011958
29.12.2019 09:16 duzenko Assigned To duzenko => cabalistic
29.12.2019 10:48 grayman Note Added: 0011965
29.12.2019 11:36 cabalistic Note Added: 0011969
29.12.2019 11:37 cabalistic Assigned To cabalistic => duzenko
29.12.2019 11:50 stgatilov Note Added: 0011971
29.12.2019 12:31 duzenko Note Added: 0011974
01.02.2020 01:32 grayman Note Added: 0012160
01.02.2020 11:11 stgatilov Note Added: 0012166
01.02.2020 11:11 stgatilov Note Edited: 0012166 View Revisions
01.02.2020 13:22 cabalistic Assigned To duzenko => cabalistic
01.02.2020 16:06 cabalistic Note Added: 0012167
01.02.2020 16:07 cabalistic Status assigned => resolved
01.02.2020 16:07 cabalistic Resolution open => fixed
01.02.2020 16:07 cabalistic Fixed in Version => TDM 2.08
01.02.2020 17:15 duzenko Note Added: 0012168
02.02.2020 02:32 grayman Note Added: 0012169
02.02.2020 15:51 stgatilov Note Added: 0012170
02.02.2020 15:52 stgatilov Note Edited: 0012170 View Revisions
02.02.2020 15:57 stgatilov Note Edited: 0012170 View Revisions
02.02.2020 15:57 stgatilov Note Edited: 0012170 View Revisions
03.02.2020 16:01 stgatilov Note Edited: 0012170 View Revisions