luciusDXL / TheForceEngine

Modern "Jedi Engine" replacement supporting Dark Forces, mods, and in the future Outlaws.
https://TheForceEngine.github.io
GNU General Public License v2.0
994 stars 73 forks source link

[Crash] Ruins of Talos II: Phantom Point Crashes on load #443

Open DatMaxNub opened 2 months ago

DatMaxNub commented 2 months ago

Version: 1.10

Game: Dark Forces

Mod: https://df-21.net/downloads/missions.php?viewid=2086487251

Level crashes TFE at the load screen.

crashdump.dmp

mlauss2 commented 2 months ago

This patch fixes it for me, but I have absolutely no idea whether it's the right thing to do. The level loads, but I haven't played it to completion yet.

index 855739e2..57738f3d 100644
--- a/TheForceEngine/TFE_Jedi/Level/level.cpp
+++ b/TheForceEngine/TFE_Jedi/Level/level.cpp
@@ -103,7 +103,7 @@ namespace TFE_Jedi
                        for (s32 w = 0; w < sector->wallCount; w++, wall++)
                        {
                                RSector* nextSector = wall->nextSector;
-                               if (nextSector)
+                               if (nextSector && nextSector->walls && wall->mirror != -1 && nextSector->wallCount > wall->mirror)
                                {
                                        RWall* mirror = &nextSector->walls[wall->mirror];
                                        wall->mirrorWall = mirror;
@@ -111,6 +111,12 @@ namespace TFE_Jedi
                                        wall->flags3 |= (mirror->flags3 & 0x0f);
                                        mirror->flags3 |= (wall->flags3 & 0x0f);
                                }
+                               else
+                               {
+                                       // disable invalid/nonsensical mirror
+                                       wall->nextSector = nullptr;
+                                       wall->mirror = -1;
+                               }
                        }
                        sector_setupWallDrawFlags(sector);
                        sector_adjustHeights(sector, 0, 0, 0);

EDIT: I was able to complete the level with this patch applied. There's a wall in the underground mining shaft one has to step through, shortly before the big wampa outside behind a bush blocking the way, is that intentional? The automap revealed the way forward, so it wasn't an issue.

here: image

luciusDXL commented 2 months ago

Thanks for the report, I will look at the data. But if the above fixes it, it is probably an issue with the level data (another case where it should crash in DOS but doesn't for some reason).

luciusDXL commented 2 months ago

@DatMaxNub So I looked at the level in the editor - there is an area with a waterfall with some really wonky adjoins - I'm surprised it works in vanilla, honestly.

tfe_screenshot_Mon-Aug-26-11_09_21-2024_1

For example, see that highlight adjoin - you have 3 adjoins in that spot, if it were correctly setup, the adjoin would only extend along the length of the step.

Here you can see the sector in question is degenerate, part of the reason the adjoins are so wonky: tfe_screenshot_Mon-Aug-26-11_09_21-2024_2

Anyway, the only real fix (other than fixing the level itself) is something like what @mlauss2 has. I will add something here for the next release. The original code did not have this check, but it doesn't crash in DOS even though it probably should.

After playing to that point in the level, I can see it looks glitchy even in software. I implemented a fix for the crash, which will be fixed in the next release (I will close the issue once the next release is published).

DatMaxNub commented 2 months ago

This patch fixes it for me, but I have absolutely no idea whether it's the right thing to do. The level loads, but I haven't played it to completion yet.

index 855739e2..57738f3d 100644
--- a/TheForceEngine/TFE_Jedi/Level/level.cpp
+++ b/TheForceEngine/TFE_Jedi/Level/level.cpp
@@ -103,7 +103,7 @@ namespace TFE_Jedi
                        for (s32 w = 0; w < sector->wallCount; w++, wall++)
                        {
                                RSector* nextSector = wall->nextSector;
-                               if (nextSector)
+                               if (nextSector && nextSector->walls && wall->mirror != -1 && nextSector->wallCount > wall->mirror)
                                {
                                        RWall* mirror = &nextSector->walls[wall->mirror];
                                        wall->mirrorWall = mirror;
@@ -111,6 +111,12 @@ namespace TFE_Jedi
                                        wall->flags3 |= (mirror->flags3 & 0x0f);
                                        mirror->flags3 |= (wall->flags3 & 0x0f);
                                }
+                               else
+                               {
+                                       // disable invalid/nonsensical mirror
+                                       wall->nextSector = nullptr;
+                                       wall->mirror = -1;
+                               }
                        }
                        sector_setupWallDrawFlags(sector);
                        sector_adjustHeights(sector, 0, 0, 0);

EDIT: I was able to complete the level with this patch applied. There's a wall in the underground mining shaft one has to step through, shortly before the big wampa outside behind a bush blocking the way, is that intentional? The automap revealed the way forward, so it wasn't an issue.

here: image

Yes cutting the brush was intentional. It's meant to loop you back to cabin with a red key hidden behind the fireplace

https://www.youtube.com/watch?v=bnpGiYPpWbY

10:30 mark and from there