Source2Gen is an executable + Linux support

[Cre3per]

See issue #48 for details and status

Thanks for .clang-format, cmake, and for using assertions! It made working on this project much easier than some other projects.

[es3n1n]

Oh wow, good job. I took a quick look at the changes, and here are some notes.
Not really a big fan of macros since we are coding in C++ and not C. We were trying to remove our macros a while ago, and now we are getting some new ones :)
This definitely needs lots of refactoring, but good job nonetheless

[es3n1n]

imo we should rather use an approach like this https://github.com/es3n1n/obfuscator/blob/dev/src/lib/util/platform.hpp but it's up to you to decide

[Cre3per]

I got inspired by Rust's #[cfg(target_os = "linux")]. I've never used this approach in C++ before and don't know how well it plays out.
I have no preference for one or the other. I can change it to PLATFORM_IS_* if you like it better.

[es3n1n]

Yeah, I am not too sure either, #if PLATFORM == WINDOWS looks more verbose than mine, although it's pretty much the same thing. I guess you can keep it like this, if you like this one.

Perhaps it would make our thing 🚀 blazing fast, since It was inspired by rust 🙃

[Cre3per]

Oh wow, good job. I took a quick look at the changes, and here are some notes. Not really a big fan of macros since we are coding in C++ and not C. We were trying to remove our macros a while ago, and now we are getting some new ones :) This definitely needs lots of refactoring, but good job nonetheless

Thanks for the quick review and your attention to C++! I'm with you on avoiding macros as much as we can.
We'll use a sort of offset class to avoid uses of IF_LINUX/WINDOWS. There'll be some macros for pads that exist on a single platform, unless we use [[no_unique_address]] and zero-sized pads, ever tried that? EDIT: eww, no to [[no_unique_address]]

I had to un-constexpr the requiredModules array in startup.cpp because I didn't want to use LOADER_GET_MODULE_FILE_NAME(). Do you have an idea if/how we can build module file names at compile-time without macros?

[es3n1n]

There'll be some macros for pads that exist on a single platform, unless we use [[no_unique_address]] and zero-sized pads, ever tried that? EDIT: eww, no to [[no_unique_address]]

Never even heard of it, lol

I had to un-constexpr the requiredModules array in startup.cpp because I didn't want to use LOADER_GET_MODULE_FILE_NAME(). Do you have an idea if/how we can build module file names at compile-time without macros?

There should definitely be a way to do that. I will try to come up with an implementation, but I am a bit busy with some other projects, so this probably could take some time

[cpz]

otherwise looks like a good pr

[cpz]

Also,

As I know, all these functions where the second parameter is the return value only apply to windows, so you probably need to modify these functions to work on unix

Edit:
Just saw your comment in code

Isnt why we have different kSchemaSystemVersion. We need different schema system version because of support for older games than cs2 or dota2 (e.g. Dota Underlords)

[cpz]

Providing a path for the dumper to the game is convenient if you only use the dumper on the server, but if you use it yourself, you'll get a little tired of it after a while. So on windows we'll most likely have to use Registry and from there take SteamLibrary path and hardcode the folder name for each game. Which sounds a little bit bad, but I don't know any other way. If path was provided then we'll work with it otherwise using registry + hardcoded names of folders for each game.

[es3n1n]

Providing a path for the dumper to the game is convenient if you only use the dumper on the server, but if you use it yourself, you'll get a little tired of it after a while. So on windows we'll most likely have to use Registry and from there take SteamLibrary path and hardcode the folder name for each game. Which sounds a little bit bad, but I don't know any other way. If path was provided then we'll work with it otherwise using registry + hardcoded names of folders for each game.

What's wrong with just placing generated SDKs in the same folder as the binary itself tho? We'll provide an optional path where to place the generated SDKs, and if it wasn't provided, we'll just drop it to the current directory

[cpz]

Providing a path for the dumper to the game is convenient if you only use the dumper on the server, but if you use it yourself, you'll get a little tired of it after a while. So on windows we'll most likely have to use Registry and from there take SteamLibrary path and hardcode the folder name for each game. Which sounds a little bit bad, but I don't know any other way. If path was provided then we'll work with it otherwise using registry + hardcoded names of folders for each game.

What's wrong with just placing generated SDKs in the same folder as the binary itself tho? We'll provide an optional path where to place the generated SDKs, and if it wasn't provided, we'll just drop it to the current directory

We are talkin about game DLL's. Path is being used to get them and load them in to memory. I'm aint against dumping in current executable folder sdk, I thinkin that if I will pass path to dota 2 each time when I want to dump -- I'll break my monitor in week.

[Cre3per]

Also,

As I know, all these functions where the second parameter is the return value only apply to windows, so you probably need to modify these functions to work on unix

Edit: Just saw your comment in code

Isnt why we have different kSchemaSystemVersion. We need different schema system version because of support for older games than cs2 or dota2 (e.g. Dota Underlords)

Without knowing the history of source 2, I think "support older games" and "return value optimization" aren't conflicting each other. Could it be that older games or different platforms simply didn't use return value optimization? If that's the case, what we're seeing with these functions might not be caused by versioning, but changed compiler settings on Valve's side.
If this were caused by a change in APIs, I think it'd be weird that it differs between platforms, as Valve is very likely compiling all platforms from the same source tree.
But again, I have no experience with source 2. If you think there are different versions of this function, I'll update the comment.

[Cre3per]

Providing a path for the dumper to the game is convenient if you only use the dumper on the server, but if you use it yourself, you'll get a little tired of it after a while. So on windows we'll most likely have to use Registry and from there take SteamLibrary path and hardcode the folder name for each game. Which sounds a little bit bad, but I don't know any other way. If path was provided then we'll work with it otherwise using registry + hardcoded names of folders for each game.

I've had thoughts in a similar direction. PATH or LD_LIBRARY_PATH needs to contain
cs2/game/bin/linuxsteamrt64/ and
cs2/game/csgo/bin/linuxsteamrt64/

Source2Gen could generate the two paths itself, given a path to the game installation directory. I've added a script (scripts/run.sh)* that does this on Linux. We could add a similar script for Windows.
Linux users, in particular developers, who are the users of source2gen, will know where their game is installed and they'll know how to use their shell's history. An automatic lookup of the game directory would be of little benefit for them.

You have suggested to use the registry on Windows, should we note that idea in the linked issue so whoever restores Windows compatibility has an easier time implementing it?

* source2gen can't set PATH or LD_PRELOAD_PATH itself because the dynamic linker reads those variables once at program launch. Changing them in the program has no effect. That's how it works on Linux at least.

[es3n1n]

Sorry for the delays in my response(and sorry for my previous response; I was sleep-deprived, T_T). I was a bit busy with some other projects, but now I am more or less free from them, so you can expect faster replies from me.

You have suggested to use the registry on Windows, should we note that idea in the linked issue so whoever restores Windows compatibility has an easier time implementing it?

Yes, please. Most likely this person would be me anyway.

Without knowing the history of source 2, I think "support older games" and "return value optimization" aren't conflicting each other. Could it be that older games or different platforms simply didn't use return value optimization? If that's the case, what we're seeing with these functions might not be caused by versioning, but changed compiler settings on Valve's side. If this were caused by a change in APIs, I think it'd be weird that it differs between platforms, as Valve is very likely compiling all platforms from the same source tree. But again, I have no experience with source 2. If you think there are different versions of this function, I'll update the comment.

Yes, indeed. they might've changed compiler options, but since we are no valve employees we cannot know this for sure, this is why this is called a version, this variable would be used in the future if source 2 developers introduce any more changes to the compiler options/functions/anything else. This is just more convenient.

I am OK with having this comment; just add there at least for now and we're good :)

What are the current merge blockers for this PR? Apart from the windows support.

[Cre3per]

Sorry for the delays in my response(and sorry for my previous response; I was sleep-deprived, T_T). I was a bit busy with some other projects, but now I am more or less free from them, so you can expect faster replies from me.

You have suggested to use the registry on Windows, should we note that idea in the linked issue so whoever restores Windows compatibility has an easier time implementing it?

Yes, please. Most likely this person would be me anyway.

Without knowing the history of source 2, I think "support older games" and "return value optimization" aren't conflicting each other. Could it be that older games or different platforms simply didn't use return value optimization? If that's the case, what we're seeing with these functions might not be caused by versioning, but changed compiler settings on Valve's side. If this were caused by a change in APIs, I think it'd be weird that it differs between platforms, as Valve is very likely compiling all platforms from the same source tree. But again, I have no experience with source 2. If you think there are different versions of this function, I'll update the comment.

Yes, indeed. they might've changed compiler options, but since we are no valve employees we cannot know this for sure, this is why this is called a version, this variable would be used in the future if source 2 developers introduce any more changes to the compiler options/functions/anything else. This is just more convenient.

I am OK with having this comment; just add there at least for now and we're good :)

• Added a hint about the registry to the linked issue and README.md
• Updated the documentation of kSchemaSystemVersion

What are the current merge blockers for this PR? Apart from the windows support.

• @cpz's comments. Waiting for a re-review.
• Use of macros and missing constexpr. This is done from my side. We've had some good ideas in previous threads. If you've got more ideas, I'd be happy to improve the code.
• An "ok" from you guys that we can merge the Linux part untested, with a note in README.md stating that it is in an alpha state.
  • For CS2, source2gen generates an SDK that looks right. For now, I can't run CS2 and don't have a reference SDK. I'd come back with fixes once I am a user of the SDK.
  • I haven't tested any games apart from CS2, source2gen might well crash for other games. Assuming the person who re-adds Windows support already has all the other games downloaded, I suggest they test those on Windows only. It's very possible that the get_required_modules() array differs between games. We could keep other Linux games "potentially broken" until CS2 is confirmed working.

It might be worth to consider adding automatic tests (Separate issue) now that source2gen is faster and doesn't need the games to be running. Even if the tests can only run on a developer machine, they'll make it safer to work on source2gen and could reduce the effort required to review pull requests.

[es3n1n]

* @cpz's [comments](https://github.com/neverlosecc/source2gen/pull/47#pullrequestreview-2015741031). Waiting for a re-review.

Okie, I will let him know.

* Use of macros and missing `constexpr`. This is done from my side. We've had some good ideas in previous threads. If you've got more ideas, I'd be happy to improve the code.

Yeah, I will try to check it out today.

  * For CS2, source2gen generates an SDK that _looks_ right. For now, I can't run CS2 and don't have a reference SDK. I'd come back with fixes once I am a user of the SDK.

Sure I guess, users will complain if something won't work anyway

  * I haven't tested any games apart from CS2, source2gen might well crash for other games. Assuming the person who re-adds Windows support already has all the other games downloaded, I suggest they test those on Windows only. It's very possible that the `get_required_modules()` array differs between games. We could keep other Linux games "potentially broken" until CS2 is confirmed working.

Yeah, we will test it only on Windows. As for the required modules, we previously had an array that worked for all the source 2 games. I am too lazy to check, but if you didn't change the checked modules themselves, then we should probably be good.

It might be worth to consider adding automatic tests (Separate issue) now that source2gen is faster and doesn't need the games to be running. Even if the tests can only run on a developer machine, they'll make it safer to work on source2gen and could reduce the effort required to review pull requests.

Ah yes, I've been dreaming about it for a long time, adding something like GTest for tests would make the further work much easier. But that is a different story and will be implemented later.

-- Once cpz approves all the changes, I guess I will start on adding Windows support back and will most likely refactor other parts as well. This would probably need your help at some point, but we are too far from that as of now.

[Cre3per]

  * I haven't tested any games apart from CS2, source2gen might well crash for other games. Assuming the person who re-adds Windows support already has all the other games downloaded, I suggest they test those on Windows only. It's very possible that the `get_required_modules()` array differs between games. We could keep other Linux games "potentially broken" until CS2 is confirmed working.

Yeah, we will test it only on Windows. As for the required modules, we previously had an array that worked for all the source 2 games. I am too lazy to check, but if you didn't change the checked modules themselves, then we should probably be good.

The target game doesn't need to run anymore for source2gen to work. If the game doesn't load its libraries, someone else has to. That someone is source2gen. The get_required_modules() array now lists all libraries that register a schema.
For CS2, I looked at the example SDK linked on main to figure out what libraries can be dumped and listed those. If the set of libraries differs between games, we could maintain a list per-game, or load all libraries and ignore those that don't expose a InstallSchemaBindings() symbol.

[cpz]

I've did some changes for compiling on Windows and dumping, there was problems with static_asserts and find_module_symbol.

find_module_symbol in loader can be changed, so it wont be based on defines, I've made like this because didnt have enough time to think, sadly atm I have limited time to do anything.

Otherwise, seems like it works.
We should somehow make it work based on registry on Windows cos atm for testing I was just placing needed dlls near source2gen executable.