Skip to content

Add support for other engine binaries in game configs (#1414).#1626

Merged
psychonic merged 4 commits intomasterfrom
issue-1414
Dec 28, 2022
Merged

Add support for other engine binaries in game configs (#1414).#1626
psychonic merged 4 commits intomasterfrom
issue-1414

Conversation

@psychonic
Copy link
Member

No description provided.

Copy link
Member

@peace-maker peace-maker left a comment

Choose a reason for hiding this comment

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

This is cool, it's easier to hook stuff in e.g. tier0 now. I'm unsure about the CRC refactoring though.

char name[64];
bridge->FormatSourceBinaryName(pszName, name, sizeof(name));

// This pattern doesn't work for every game. Instead, we need to use the GAMEBIN search path properly
Copy link
Member

Choose a reason for hiding this comment

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

Is this a TODO? Or does this magically open the $BASE/bin/engine.so as well as the $BASE/csgo/bin/server.so?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes and no... and good catch.

The TODO was in regard to games that don't use standard paths (/bin, //bin). However, you're absolutely right that it's not accounting for the far more common case of all of the files normally just in /bin. I'll have to revisit that. The easiest solution to both problems may be to just open/read the files via IFIleSystemBridge.

@psychonic psychonic marked this pull request as draft November 2, 2021 12:01
@psychonic
Copy link
Member Author

Some other issues with this current method of using Valve's filesystem layer is that if MM:S is loaded via the gameinfo method, it's loader will be picked up instead of the game's server.dll.

@psychonic
Copy link
Member Author

I updated with a different approach. The aforementioned issues appear to be solved now on Windows. It's currently completely broken on Linux, but it shouldn't be too bad to fix.

@peace-maker
Copy link
Member

What's missing for linux support here? We could have used this for the optional game update #1766

@psychonic
Copy link
Member Author

What's missing for linux support here?

I don't remember. I vaguely recall there being an issue around the dlopen usage. It would need to be tested again at minimum.

Before this, there was a bad assumption that, like on Windows, POSIX module
handle pointers were within the module's address space (and thus usable
with dladdr). That's not true!

Instead, to get a usable address on all platforms, we'll do a lookup of the
CreateInterface function that exists in all modules. This also has the
(arguable) benefit of further locking this implementation to modules owned
by the game.

To get a valid address inside the module now on both p
@psychonic psychonic marked this pull request as ready for review December 28, 2022 20:32
@psychonic
Copy link
Member Author

What's missing for linux support here? We could have used this for the optional game update #1766

I've now found and fixed the remaining issue on Linux with 67aea17

Copy link
Member

@peace-maker peace-maker left a comment

Choose a reason for hiding this comment

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

Could use the same link cast as in #576, but limiting the allowed binaries to engine ones seems reasonable given those are the libraries targeted.

@psychonic
Copy link
Member Author

Could use the same link cast as in #576, but limiting the allowed binaries to engine ones seems reasonable given those are the libraries targeted.

Cool. That's good info.

I had a small back-and-forth about this in my head. I'm not a fan of just casting the handle since that does not seem to be an API guarantee. However, it looks like dlinfo can be safely used to the same end. I then wondered if we shouldn't be relying on the Windows HMODULE to be a pointer in the library either. Rather than then looking up an equivalent method there, I landed on this being fine as-is for now. We can revisit if/when there is a use case.

@psychonic psychonic merged commit ecb707e into master Dec 28, 2022
@psychonic psychonic deleted the issue-1414 branch December 28, 2022 22:58
@Alienmario
Copy link
Contributor

Alienmario commented Oct 30, 2023

Is this supposed to automatically recognize the _srv filename suffix in some linux binaries?
e.g. "library" "MaterialSystem" should work?

Edit: turns out it will, but only if you use lowercase!

@psychonic
Copy link
Member Author

Yes. To clarify, any platform-specific prefixes and/or suffixes are handled automatically. File system case sensitivity can still apply.

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.

3 participants