Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade tool projects #138

Closed
5 tasks done
SamVanheer opened this issue Mar 15, 2022 · 5 comments
Closed
5 tasks done

Upgrade tool projects #138

SamVanheer opened this issue Mar 15, 2022 · 5 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@SamVanheer
Copy link
Collaborator

SamVanheer commented Mar 15, 2022

The tool projects are Visual Studio 2010 projects and do not compile due to both missing and outdated dependencies and changes made to the game codebase.

Upgrade the projects:

  • Use Visual Studio 2019 projects and build toolset
  • Compile successfully (with the exception of the 3DS Max plugin smdlexp because it requires the Max SDK)
  • Fix as many compiler warnings as possible
  • Convert all projects to C++
  • As many projects as possible should also work as intended

Issue #136 should be closed by the work required for this issue.

@SamVanheer SamVanheer added bug Something isn't working enhancement New feature or request labels Mar 15, 2022
@SamVanheer SamVanheer self-assigned this Mar 15, 2022
@fel1x-developer
Copy link
Contributor

Is there any reason for converting all projects to C++?

@SamVanheer
Copy link
Collaborator Author

It eliminates the need to do #define Vector vec3_t, allows local variables to be declared after the start of a function which helps to eliminate unreferenced local variable warnings, enforces some stricter rules compared to C (some casts need to be explicit for example), it turns some warnings into errors (like this one), and in the future will allow for things like smart pointers to manage memory more easily.

It also allows the use of things like std::thread.

Having everything compiled as C++ code is more consistent and produces expected results for all files, whereas now a file could have a #ifdef __cplusplus check in there or implicitly depend on C linkage.

It'll also make it possible to use the headers from the game in the tools, currently some headers (e.g. mathlib.h) are duplicated.

SamVanheer added a commit that referenced this issue Mar 15, 2022
…om build/rebuild/clean (project missing 3DS Max SDK)

#138
SamVanheer added a commit that referenced this issue Mar 15, 2022
SamVanheer added a commit that referenced this issue Mar 15, 2022
…pr & strlwr defined on platforms other than Windows only

#138
SamVanheer added a commit that referenced this issue Mar 15, 2022
SamVanheer added a commit that referenced this issue Mar 15, 2022
SamVanheer added a commit that referenced this issue Mar 15, 2022
SamVanheer added a commit that referenced this issue Mar 15, 2022
SamVanheer added a commit that referenced this issue Mar 15, 2022
SamVanheer added a commit that referenced this issue Mar 15, 2022
SamVanheer added a commit that referenced this issue Mar 15, 2022
@SamVanheer
Copy link
Collaborator Author

I've done most of the work required for this issue, i just need to fix any remaining warnings that can be fixed, and test the projects to make sure they work as intended.

There are 231 warnings and 21 messages when compiling with Visual Studio 2019 concerning local variables shadowing other variables, unused variables, uninitialized variables, signed/unsigned comparison mismatches, pointers that may be null due to malloc returning null when out of memory as well as buffer overflows.

Some of these can be fixed (e.g. removing unused locals and not naming unused parameters), others would require changing behavior (e.g. checking for null return from malloc should either be a fatal error or be replaced with new, which throws on allocation failure).

I'd say memory allocations should use throwing new everywhere since that simplifies error handling but that should have proper error logging to inform the user of why it failed (at minimum reporting out-of-memory error).

If everything works as intended then it should be fine to merge.

SamVanheer added a commit that referenced this issue Mar 16, 2022
SamVanheer added a commit that referenced this issue Mar 16, 2022
SamVanheer added a commit that referenced this issue Mar 16, 2022
…rnings, comment out parameters that aren't used but useful or needed, remove parameters that aren't used at all

#138
SamVanheer added a commit that referenced this issue Mar 16, 2022
SamVanheer added a commit that referenced this issue Mar 16, 2022
SamVanheer added a commit that referenced this issue Mar 16, 2022
SamVanheer added a commit that referenced this issue Mar 16, 2022
… load smd files during studiomodel compilation (fixes "input could be null" warnings)

#138
SamVanheer added a commit that referenced this issue Mar 16, 2022
@SamVanheer
Copy link
Collaborator Author

I replaced GLaux and GLUT with SDL2. The purpose of both was to make a window and draw to it using OpenGL, and process user input in mdlviewer's case. That said, GLaux was used by CSG and BSP to draw what the tool in question was doing. VHLT doesn't have this functionality anymore, probably because it was never used and having a GUI-only option on a tool used on the command line adds a dependency that's best left out.

I fully re-implemented this feature using SDL2 and OpenGL. qcsg doesn't seem to draw anything, while qbsp2 draws the geometry it's working on. Enabling this feature causes qbsp2 to take about a minute to process c1a0, whereas before it took less than a second. The window created to draw this doesn't update properly and so Windows will treat it as frozen which prevents it from updating its contents, so you'd have to avoid doing anything that can change window focus or otherwise trigger Windows to treat it as frozen.

I removed some of the functions in that code because they were never called and they were causing "unreferenced formal parameter" warnings. It would probably be better to remove this stuff altogether from those two tools.

mdlviewer is incredibly bare bones compared to even the oldest community-made model viewers, so other than curiosity i doubt anybody will want to use it. If they do, they'll have to grab the SDL2 dll from the game installation and put it next to mdlviewer.exe to run it.

The SDL2 changes came from the HLEnhanced repository where i'd already done most of that work.

I've fixed as many warnings as i could, most of the remaining warnings are caused by code not checking for null returns by malloc/realloc/kalloc/calloc and passing potential null pointers to other functions like memset.

Other warnings involve string operations that aren't secure like using strcpy or scanf, and unsafe array access caused by C-style dynamic array allocations that make arrays with fixed size have a dynamic size at runtime.

Most of these warnings can be fixed by using C++-style design: using std::vector to allocate arrays and reallocate instead of using malloc/calloc + realloc, and using std::vector to allocate windings and returning those windings by value instead of allocating the entire object dynamically.

Return value optimization and guaranteed copy elision combined with the use of constructors and move operations should eliminate any overhead that would be associated with that kind of refactoring work.

Several functions allocate a lot of stack space, many of those are threaded functions so it might be a good idea to use static threadlocal storage for this.

Replacing the thread code with std::thread should eliminate several unused parameters that are currently commented out, it should also get rid of warnings related to null thread handles.

Those changes should fix all remaining warnings, but that's a bit out of scope.

I tested the projects:

  • bspinfo: Works as expected.
  • light: Obsolete, basically an older version of qrad. Not tested.
  • makefont: Produces a fonts.wad file identical in size, differing slightly in contents but that might be due to different command line arguments used to create the original wad. Since this tool relies on the Win32 GDI API to generate fonts it's possible that API is returning slightly different data, or perhaps smoothing some pixels out. The CreateConsoleFont function is corrupting the stack, reported by Visual Studio in a debug build.
  • makels: Works as expected, produces a text file containing instructions for qlumpy.
  • mdlviewer: Works fine, although there isn't much functionality there to begin with.
  • mkmovie: Obsolete, the engine outputs bmp files directly when using the startmovie/endmovie commands. Not tested due to lack of input files.
  • procinfo: This is a static library and checks for the existence of old CPU extensions that anyone running Windows 7 or newer either has or doesn't need. Nobody will use something this old to test for extension support. A newer but still old version of this code is in the Source SDK: https://github.com/ValveSoftware/source-sdk-2013/blob/0d8dceea4310fde5706b3ce1c70609d72a38efdf/mp/src/tier1/processor_detect.cpp
  • qlumpy: Parses the script produced by makels correctly. Since the bmp files listed in the file are not 8 bit qlumpy exits with an error, as expected.
  • serverctrl: This is hardcoded to run hlds from "d:\quiver\hlds.exe", so for the purposes of the test i changed the path to my HLDS installation. Launching HLDS through this program launches the HLDS VGUI1 interface which handles the actual starting of the server. serverctrl cannot send and receive anything but this is probably because the HLDS GUI already does this work. Otherwise the program seems to work as intended.
  • smdlexp: this is a plugin for 3DS Max 4.2 so testing it isn't feasible. Plugins exist for modern modeling tools which renders this plugin obsolete.
  • sprgen: Tested with the sample provided with the Half-Life SDK on Steam. Produces output identical to sprgen provided with that SDK.
  • studiomdl: Compiled the player model provided in the Half-Life SDK. The output is virtually identical to the output created by the studiomdl provided with that SDK, but there are a few differences that appear to be caused by slight changes in floating point calculations. These changes are too minor to be visible.
  • xwad: Tested with halflife.wad: all files extracted successfully. Note that this tool extracts mipmaps as well, which makes the output a bit difficult to use. Tools like Wally and GCFScape can extract the original image directly.

The map compile tools (qcsg, qbsp2, vis, qrad) are tied together so i tested them as one. I compiled c1a0 provided with the Half-Life SDK. Geometry-wise the output is the same, but because we don't have the lights.rad file used by Valve the lighting is different. The lighting that is there matches what's in the original version.

I also tested the compile tools built as C and C++. Like studiomdl there are slight changes in floating point calculations that change the output. These kind of changes are minor and considering that ZHLT and VHLT are both compiled as C++ i doubt this matters.

SamVanheer added a commit that referenced this issue Mar 16, 2022
@SamVanheer
Copy link
Collaborator Author

I've formatted all files using the .clang-format configuration file.

This wraps up work on the tools code, everything aside from smdlexp compiles without requiring additional dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants