The sky is behaving very weird. Depending on which angle you look at, it looks very distorted, with all the clouds crumbled together. This becomes most apparent when you turn the camera fast.
The sky behavior should be fixed.

Illegal memory writes in the A* path search corrupted memory and caused crashes in completely unrelated code sections when the player died and restarted the level.

The illegal memory writes were resolved with a quickfix: the search is not performed when an illegal memory write is likely to occur. This change may have had significant impact on the dino behavior. To me, they seem less reactive to the player, and standing still more often.

It should be investigated as to whether this change had any real impact and what can be done to restore the old behavior without bringing back the crashes.

Players should be to bind cheats to keys. This matches the hotkey feature in ATX.

An obvious question is how to deal with cheats that require additional parameters. Key options include:

• Do not expose them to binding
• Cache the cheat arguments after their last command line invocation and reuse the cached data, do not execute the cheat if no cache data is present

Related to #158 and #159.

Rarely an error occurs in the destructor of CFileIO (part of Loader): the close functions causes an error because it was called with a file handle which has the value 1024.
Not closing the the handle when it is 1024 would be trivial. But it should be investigated how and why the handle becomes 1024 in the first place, as this is likely an indicator of some error or special condition.

Defines and typedefs such as BOOL, CONST, UINT and DWORD are used pretty much everywhere in the codebase.
Most of these defines where introduced a long time ago (BOOL was added before bool was thrown into C++) and now, most of them are meaningless.
Nowadays, every decent compiler should support C++'s fundamental types without any problem.
Of course, DirectX types and some other funky types (HWND, HHOOK and so on) can't be replaced but there's no excuse for more common types I've pointed out earlier.

Pros

• Less error-prone (every Microsoft specific type names are upper case)
• Makes high-level parts of the engine less dependent on MSVC / Windows API
• Improves compile/runtime speed and makes debugging tasks easier (BOOL is a int typedef whereas bool is a true type)
• Syntax highlighting / code readability is slightly improved; GitHub syntax highlighter (and probably many others) don't know about BOOL, HANDLE or DWORD but it does its job perfectly well on the standard types.

Cons

• Nothing. Really, I can't think of any drawbacks.

What to do?

There's nothing hard. We just need to search for these types and replace them by their standard
equivalent.
If you really have to use a non-standard type in a piece of code, it should be better to keep this code 'away (i.e. not in a header nor in the middle of a high-level source file)
To make things easier, here's a table with what should be considered for replacement and their equivalents.

1 - Representation of an integer type for pointer precision

• INT_PTR / LONG_PTR are mapped to a 32-bit signed integer on 32-bits systems and to a 64-bit signed integer on 64-bits systems.
• DWORD_PTR / UINT_PTR are mapped to a 32-bit unsigned integer on 32-bits systems and to a 64-bit unsigned integer on 64-bits systems.

It's clear that the easiest way to do that would be to use cstdint along with the standard types. Feel free to share anything, any ideas about all this.

The ProcessorDetect project does not compile yet because of a minor type ambiguity. It should be repaired.
Because ProcessorDetect yields a DLL, we also need a little test program for it.
Note that ProcessorDetect is not used by the game or any of the existing tools, only by the setup program.

The features and the fixes from the official patch should be reimplemented.

CPackedRaster manages trees of STextureQuadNode objects. Those nodes are "linked" to each other via pointers. When a node is deleted, those link pointers are left dangling. When such a dangling pointer is dereferenced, this raises complaints when additional memory access validation (full page heap verification) is enabled.
A quick workaround is proposed in PR #95: cache deleted pointers to avoid using them again. However, that workaround is rather "dirty" and, to an extend, also incomplete. (What if a pointer becomes valid again? When can we remove from or clear the cache?)

A clean solution should be seeked. Key options include:

• use std::weak_ptr for the linkage pointers. However, the required switch to std::shared_ptr for all the STextureQuadNode pointers could have a significant performance penalty.
• Reset link pointers when a node is about to be destroyed. This could require searching the entire tree, which is probably expensive.
• Store the linking data another way

As is common practice in games, the game version should be indicated in the main menu or settings menu.
Because the menu content is defined by game data, this would probably require some kind of hack.

All the assembly code is very difficult to maintain. Also it is often not 64bit-compatible. Furthermore, the compiler gives warnings about modifying the EBP frame pointer register.
All assembly code should be replaced with near-equivalent C/C++-code.

A subset of specified source files within each projects "CMakeLists.txt" have incorrect casing, preventing project load under Linux. Similarly, during a build on Linux, many '#include "paths"' utilise backslashes and mismatched case.

These issues should be tackled following a unification of all source files under some proposed folder and naming convention, as the first step towards Linux compilation.

It should be investigated if a suitable control scheme for gamepads can be introduced to the game.
If so, support for gamepads, at least Xbox-style controllers, should be introduced.

ATX V2.13 lists 41 cheats / console commands that were added or had their behavior modified. When sensible or applicable, those cheats should be supported by OTP as well.

One cheat, an FPS indicator, has already been implemented (see issue #24).
I suggest not reimplementing the REC cheat for screen recording. There is plenty of screen recording software and even Windows 10 itself supports that feature via the Xbox Game Bar.

Do whatever is necessary to make GUIApp usable again - i.e. find and repair crashes, significant memory leaks and other blocking issues.

Bug Steps

1. Did a fresh Install of Trespasser.
2. Grabbed the repo and created a solution via CMAKE. (I had to grab a newer version of CMAKE because the one provided would not run on my system, but I don't think that's related.)
3. Ran the game in debug mode and it defaulted to software rendering.
4. Switched the driver to hardware rendering
5. ****** The driver selection window did not detect possible screen resolutions, the dropdown was empty.
6. Relaunch the game and it crashes
7. The direct draw call ** below fails because the backbufferddsd height and width are initialized to zero, and this causes a null pointer exception later in the code.
8. I resolved by manually editing the width and height in my OpenTrespasser config. I am not sure where to make changes to fix that config, but I figure this is helpful.

Lines 1929 to 1948 in RasterVid.cpp

{
			CDDSize<DDSURFACEDESC2> backbufferddsd;
			backbufferddsd.dwFlags = DDSD_WIDTH | DDSD_HEIGHT | DDSD_CAPS;
			backbufferddsd.ddsCaps.dwCaps = DDSCAPS_OFFSCREENPLAIN | DDSCAPS_3DDEVICE;

			backbufferddsd.dwWidth = i_width;
			backbufferddsd.dwHeight = i_height;

			if (force16Bit)
				Set16BitPixelFormat(backbufferddsd);
			
			**hres = DirectDraw::pdd4->CreateSurface(&backbufferddsd, &pddsDraw4, NULL);**
			if (FAILED(hres))
			{
				PrintD3D2("Buffer creation failed %x\n", hres);
				AlwaysAssert(0);
				return false;
			}
		}

The CollisionEditor project has some linker conflicts. It redefines some functions from the Std and System libraries.
Those conflicts need to be resolved so that CollisionEditor can be compiled. The conflicting implementations may not behave the same way, so some care is required.

Replace most, if not all, of DWI's STL with the C++ STL.
This also includes replacing all of the deprecated C headers.

Do whatever is necessary to make AITest usable again - i.e. find and repair crashes, significant memory leaks and other blocking issues.

The original solution was for VS 4.1/6.0, so support for VS 2017+ is a must.

A metabuild system for OpenTrespasser should be created. With the launch of a script, all these tasks should be accomplished:

• Compile the source code in all required configurations
• Run the unit tests and collect the results
• Generate a default INI file with DefaultConfigGenerator
• Create the distributable packages

Key options for implementation include:

• CMake with its CTest and CPack subsystems
• Powershell

Sometimes the game freezes because it is stuck in an endless loop in physics code.

Steps to reproduce

1. Pick up a "storable" object like a pistol or keycard.
2. Store the object in your inventory.
3. Move so that you stand directly in front of a wall (doors work too)
4. Keep storing and retrieving the object in such a way that the hand or the object touch the wall.

Personally I use the pistols in the security entrance at the beginning of the Industrial Town level for testing this.

The bug only rarely occurs in Debug builds, but is easy to reproduce in Release builds.

The cause

The problem is caused by two loops in physics code that are supposed to normalize vectors. The code is almost the same for both. Those loops are Pelvis.cpp lines 1162 to 1187 and Xob_bc.cpp lines 3738 to 3763.

The loop is supposed to end when a float variable L becomes smaller than a certain threshold. However, the comparison does not work anymore when L becomes NaN. This happens because it was calculated from the vector entries, which are all NaN as well. Current research has shown that they were already NaN before the loop. Why this happens needs to be investigated.

Solution approaches

Ending the loop when L becomes NaN prevents the game from freezing. But the vector is still full of NaNs and new problems occur immediately:

• The player cannot jump anymore
• When moving, there are very frequent little jumps. Sometimes the player can barely move at all
• The arm cannot be used anymore
• In summary, the game is left in an unplayable state.

An alternative approach is to do the bailout described above, but also set the vector that was supposed to be normalized to a valid default value. That default value needs yet to be determined. After a first test with a quad-0.25 vector, various physics glitches with held items occur, but the game remains playable.

The best solution is to find out why the vector became NaN in the first place.

The code file lists in the CMake scripts were adapted almost verbatim from the original VS project definitions. But they are incomplete, some code files that are used are missing from these lists. That mostly affects header files, but also CPP which are included like headers, mostly in ScreenRenderDWI.
Those missing code files are difficult to find in Visual Studio.

The code file lists should be completed. All sources files used should be on the lists.
Furthermore, the lists should be deduplicated. Each code file should be owned by only one project.

All assembly code not used by the Pentium Pro variant should be removed.

Most projects have one or more directories for their source code. But sometimes they own source files from other directories as well. For example, the GeomDBase project has most of its code in Lib/GeomDBase, but also some code in Lib/Renderer and GUIApp.

The source code should be reorganized so that no two projects directly share a source code directory.

When files are moved, that should be done via Git so that their commit history is retained.

Location:
jp2_pc\Source\Lib\File\Section.hpp at line 127

Error Message:
Error C2248: 'CSectionDB::get_data': cannot access private member declared in class 'CSectionDB'

During gameplay it should be possible to save the game to a quicksave file with a single keystroke and load that save instantly with another keystroke. This feature is present in both ATX and CE.

Quicksaves should also be loadable the regular way.

Related to #158.

Commit #a0a6292 on the master branch is the last commit before actual code revisions (sort of) begin. It would be very neat if we could have a tag that state here on GitHub. (A Git tag is like an immutable branch.) The original code would be directly preserved in case someone wants to study it.

Perhaps that tag could be advertised in the root README.MD.

The code for the original GroffExp plugin is still broken and needs to be repaired. Afterwards, do whatever is necessary to make the plugin usable again - i.e. find and repair crashes, significant memory leaks and other blocking issues.

The game has a bug: Jumping while the player is standing on an object only works sporadically. This was also present in the original game.

This was addressed in the official patch and in the CE mod.
The jump bug should be resolved in OpenTrespasser as well.

Some more details:
The cause of the issue is in Xob_bc.cpp, directly below the YIKES! comment: with the instruction OKtoJump = false;, which is executed basically every frame. A jump from an object often gets cancelled before it is performed.

A quick workaround is to execute OKtoJump = false; only in foot.cpp when a jump is performed. But this has a side effect: with good timing, the player can perform double jumps.
Neither the official patch nor the CE mod had this double jump bug.

The monorail track at the end of the first level seems to be a good spot for testing.

In-game commands are assigned to mouse and keyboard keys. Some of those assignments can be changed by the player in the Controls menu. Other commands like Show Hint (F1) are not reassignable.

The keybinding data is stored in a binary array. Adding more commands with assignable keys to while retaining compatibility with previous configuration is very difficult this way. Out-of-game editing of the keybindings by the player is also very difficult.
A new data structure for the keybindings should be devised.
One simple way would be a separate INI file with a very simple structure like this:

[OTPKeyBindings]
Jump=Q
Crouch=Z
Hand=LMB
...

Importing the binary key assignment format from previous INI files or the registry should remain possible.

Sometimes blue and green barrels are rendered wrong. There is a huge, single-colored, untextured, object sticking out from within the barrel. This object appears and disappears when you move around.
This happens in multiple levels, I have not checked all the levels though.

When a dino is attacking, sometimes a sound bug occurs. It sounds like the attacking sound effect is rapidly started over and over again before its playback is completed. It also sounds very weird.
This bug should be resolved.

In D3D mode, the sky is just a thick black line near the horizon.

The vanilla version also has this problem in D3D mode, but worse: the line contains semi-random material from the rest of the frame or previous frames and spazzes out during movement.

For reference: this is what the sky horizon line looks like in OTP software mode:

Frames per second (FPS) is a key performance indicator for a game. Looking at the FPS is a fast way to tell if a code change had a significant performance impact.
OpenTrespasser should have a built-in FPS indicator. It should at least show the current FPS.
Optionally it can show some advanced stats, like min/max FPS for the current session and average FPS for the current session and the past minute.

A closer look at the contents of the Source/Test directory reveals that it contains a lot of code files not used in any of the existing projects.
Those unbound source files should be puzzled together into new projects.

Trespasser has a mechanism for managing virtual memory that is used for textures. Unfortunately it is broken.
The symptom of the problem is that Trespasser would always crash with a memory access violation while creating mipmaps while loading a level. If generating mipmaps is disabled, the access violation would occur elsewhere.
PR #76 introduced a workaround for the problem: bypass virtual memory altogether by using regular memory allocation.
The root cause of the problem should be identified and resolved.

A new version of the GroffExp plugin for a contemporary release of 3DS Max should be created.
3DS Max SDKs can be found here: https://www.autodesk.com/developer-network/platform-technologies/3ds-max

The Field of View properties should be configurable by the player. The FOV settings from ATX can serve as a reference.

See also PR #118.

The trigger class CCreatureTrigger has an EvaluateAll attribute. The intent is to fire the trigger only when all the creatures listed in the trigger have died. (The more general CCreatureTrigger usage also supports other creature events.)

In the original levels, EvaluateAll is used in only one place: in Industrial Jungle right after the waterfall, at the entrance of the valley.

1. When the raptor from the fighting ground dies, it will cause another raptor to appear at spawn point 1.
2. When the raptor from the fighting ground, the raptor from spawn point 1 and the albertosaurus are all dead, it is supposed to cause two more raptors to appear around spawn point 2. This event is the CCreatureTrigger in question with EvaluateAll.

The type of death does not matter; the dinosaurs can kill each other.

The problem is that in the situation above, with the current engine code and the level script as-is, the EvaluateAll attribute is unused. The trigger fires when any one of the three dinosaurs dies.

The are-all-dinos-dead check happens in CCreatureTrigger::bEvaluateNow(), which is never called.

To use EvaluateAll, the trigger would require a FireExpression as an additional condition to be checked when attempting to trigger. In T-Script, the expression would have to be a "query expression", starting with @. This would cause CCreatureTrigger::bEvaluateNow() to be called. Such FireExpressions are also possible for other trigger types and definitely exist in the level script code.

One might propose to auto-insert the the FireExpression by adding these lines near the end of the CCreatureTrigger constructor:

if (bEvaluateAll && !peeFireExpression) {
    peeFireExpression = new CExpressionEvaluate();
    peeFireExpression->AddExpressionOperand(this, true);
}

But it does not work. The created expression is supposed to have the state EELEMENT_STATE_QUERY (4), but when the check happens in CExpressionEvaluate::EvaluateExpression(), it has EELEMENT_STATE_NOT_FIRED (0).

And finally, if one hacks around this problem as well and finally gets to the call of CCreatureTrigger::bEvaluateNow(), it will cause the game to crash. The function searches for the dino entities in the word data, but the query fails for all three dinos. Because there is no nullptr check for the search result, but merely an Assert, this will cause a crash.

Question to resolve:

• Are self-referential query FireExpressions, as required here, possible in T-Script?
• Why does the auto-insert snippet above not work? Why does the state get lost/changed?
• How can CCreatureTrigger::bEvaluateNow() be repaired? How are the dinosaurs and their state supposed to be queried here?
  • The solution must also be serializable (pcSave and pcLoad). That means if the player creates a savefile with one or two of the dinos already dead, then loads that save, the trigger must still work when the rest of the dinos die.
  • The above still holds when an alternative solution without FireExpressions is implemented.

The Smacker library bundled with the code is very old. In its current version, it is a blocker for the 64bit port. Furthermode, it does not let us use SAFESEH for trespass.exe, creating a safety and security issue. The library needs to be replaced with something:

• Use libsmacker (LGPL V2.1): a lightweight solution, but not an API-compatible drop-in replacement. Some work is required. http://libsmacker.sourceforge.net
• Use libavcodec (Part of FFmpeg, LGPL V3.0): perhaps a bit overkill, but it would support almost every video codec under the sun.

The physics code is lacking structure, often using functions with 1000 LOC or more. Those super-long functions should be divided into one or two layers of smaller functions.
Furthermore, the physics code could be shortened enormously be using loops. A lot of code is written in an "unrolled loop" style, but optimizers in compilers are perfectly capable of loop unrolling by themselves.

The code contains about 100 #ifdefs involving the __MWERKS__ preprocessor definition to discriminate for specifics of the Metrowerks development environment, often because of differences in the STL header file names or a lack of the std namespace.
All those #ifdefs should be removed.

The game and the tools should be available as both 32bit and 64bit applications.

The game and some tools, especially GUIApp, only work correctly when started with 16bit color depth.
Support for 32bit color depth should be added. Support for 16bit color depth can be dropped.

The OpenTrespasser.exe binary has a manifest containing the executable version. Its content can be viewed via the file properties in Windows Explorer. This information can also be extracted by tools and scripts.
Currently, the version indicated in the manifest is based on data hardcoded in gblinc\Version.hpp.

The following improvements are proposed:

• The version number is currently the same as vanilla Trespasser without the patch. It should instead be the OpenTrespasser version.
• The version string should include the shortened Git hash.
  • A CMake script should generate a header file that contains the shortened Git hash.
  • It remains to be decided if the script should run only during CMake generation or during every compilation.
• Ideally, all generated EXEs and DLLs should have a version manifest.