Skip to content

Prepare main branch for building using the exports#41

Open
gynt wants to merge 8 commits intomainfrom
feature/builds-polyfill-and-constants
Open

Prepare main branch for building using the exports#41
gynt wants to merge 8 commits intomainfrom
feature/builds-polyfill-and-constants

Conversation

@gynt
Copy link
Contributor

@gynt gynt commented Feb 25, 2026

  • adds polyfill for C++03 style assertion checks on struct sizes
  • adds addresses and constants for shc steam version
  • fix issue with undefined3 in ghidra typedefs
  • improve reccmp setup for dual exe and dll checks

@gynt gynt requested a review from TheRedDaemon February 25, 2026 23:50
Comment on lines +128 to +131
# We could reinstate this later:
## set(OPEN_SHC_DLL_DEST "${CRUSADER_DIR}/ucp/modules/${OPEN_SHC_NAME}-${OPEN_SHC_VERSION}" CACHE PATH "Path for OpenSHC.dll and OpenSHC.pdb")
# For now it is easier to put it in a dll subfolder
set(OPEN_SHC_DLL_DEST "${CMAKE_BINARY_DIR}/dll" CACHE PATH "Path for OpenSHC.dll and OpenSHC.pdb")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the issue that is solved here? Does reccmp not follow Symlinks?

I would contest that a bit, since it breaks the "debugging with game" flow.
The main reason for the DLL is the partial replacement and being able to run the game with it.
The flow "reimplement and check reimplementation state" could be done exe only.

Copy link
Contributor Author

@gynt gynt Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no big issue. What it solved was that I couldn't find the dll file, as the path to the _original/ucp/modules/OpenSHC-1.41.0 folder makes the location of the dll file in an unexpected place for reccmp. I can also adapt reccmp of course. It is kind of the question of what should work "out of the box" and what is an advanced usage users need to configure. I had the thought that the reccmp flow on the dll should work out of the box (as building the exe doesn't work without all reimplementations for known functions). But I can see clearly the argument for "debugging with game" flow to be optimal.

What about adding a cmake statement to also copy the dll and pdb to the build-RelWithDebInfo/dll/ subdirectory that reccmp expects? Then the "debugging with game" is default but the other situation also works.

README.md Outdated

2. Create a softlink to the original game executable with the name `_original` by running [softlink.bat](softlink.bat). It will request the full path to the game folder.
3. Setup the needed Python environment to run `reccmp` for binary comparison by running [setup.bat](reccmp/setup.bat) in the reccmp folder.
3. Setup the needed Python environment to run `reccmp` for binary comparison by running [setup.bat](reccmp/dll/setup.bat) in the reccmp folder.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that a version for the exe also exists could be mentioned here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Setup the needed Python environment to run reccmp for binary comparison by running setup.bat in the reccmp/dll folder (or the setup.bat in the reccmp/exe folder if binary comparison on the reimplemented .exe file is preferred)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the hash of the game in the file name and every enum?

Also, I remember the discussion from here: #10
Did you conclude that a difference in naming between struct and func would not be helpful?

Copy link
Contributor Author

@gynt gynt Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is the file version independent way of getting a source of truth. Currently the steam exe is the source of truth, so this hash is in the enum. Future versions we support (if we ever go there, currently we decided against it), get their own file with their hash in the file name. The enum will consist of mostly the same enum names as the steam exe, but of course different enum values as addresess are different.

Regarding data versus code, the first .data section of the .exe starts at 0x59e000, so anything lower is func, and everything higher is struct. The usage context of the enum names is obvious enough to know the distinction, so I think it is fine without F and D

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment regarding the exe-reccmp files:
Are these just copies from their DLL using versions, mostly unchanged?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes is the same files as in the dll/ subfolder

Comment on lines +1 to +2
#define static_assert_cpp98(condition) typedef int _static_assert_cpp98_obj[(condition) ? 1 : -1];
#define static_assert_cpp98_obj(condition, obj) typedef int _static_assert_cpp98##_##obj[(condition) ? 1 : -1];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this properly complain in case of issues? Do you have a (shortened) example for the compiler error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the error will be error C2118: negative subscript with a stack trace pointing to the location of the static_assert_cpp98_obj statement that caused the issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you manage to make it fully compatible with the full Windows.h? (see #14)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't run into that issue yet, but we can do the solution I suggested just now on that issue page.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this enum for?
Are these really completely general, or should they rather be part of or be themselves a normal game enum?
I also feel the precomp is not be good place, since unlike the addresses or the Ghidra data types, these are not really "unchangeable" meta structures.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are they not "unchangeable"? Because we could expand the list in the future triggering a rebuild of all files? Or because we need to change it when we go 800x800 on maps?
I considered this file for constant integer values that are systematically used across the code but never used as an enum (e.g. in a switch-case statement). Constants we deduce from what they must have used as a macro or constant at some point in the source code but which has been completely inlined. What could be added is the amount of preallocated space for the unit array, etcetera.

Comment on lines 9 to 13
tmp/
/*.7z
/*.sarif
/*.bin
/reccmp/**/*.json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For what are these? (Maybe we need to comment the project specific more anyway.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documented!

.gitignore Outdated
/reccmp/**/*.json

# Users should use git add --force for these:
/cmake/openshc-sources.txt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if we can create a flow that does not involve such git tricks.
What is the issue and intention behind this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is laziness. I usually do git add . and commit. However, the cmake/openshc-sources.txt that is upstream should reflect the required state for the latest buildable binary. Locally, it can have fewer, or additional lines, depending on what is being worked on (to reduce rebuild times). Auto adding the local developing state of that file when doing git add . would be annoying.

I can remove this feature on the upstream .gitignore and only use this trick locally. Fine by me!

Tuple of (success, diff, stdout, stderr)
"""
cmd = [str(Path("reccmp") / "run.bat"), "reccmp-reccmp", "--target", "STRONGHOLDCRUSADER", "--json", "diff.json"]
cmd = [str(Path("reccmp") / "dll" / "run.bat"), "reccmp-reccmp", "--target", "STRONGHOLDCRUSADER", "--json", "diff.json"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the MCP be actually changed to also receive if it should work with the "exe" or the "dll"?
Although I assume the "exe"-Version can not build unless every known function is implemented, so maybe this would not be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, as the exe can not build unless every known function is implemented, the mcp server only uses the dll. This does mean the mcp server doesn't have access to some optimizations that only apply to exe files, but I find that acceptable (unless it becomes a problem in the future).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants