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

Enable optional minimal SteamAPI integration for usage time tracking (editor only). #79126

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Jul 6, 2023

Disabled by default, can be enabled with (steamapi=yes build option), steam_appid.txt (it's only needed when running executable outside of Steam download) and steam_api library from the SDK should be added to distribution for this feature to work.

It initializes Steam API to inform Steam that Godot editor is running, but not using it in any way.

Fixes #18233

Reopened #56727, GitHub seems to completely ignore force pushes to the branches with "invalid characters", so it was impossible to update original PR.

@YuriSizov
Copy link
Contributor

I haven't followed the original discussion closely, but this would mean that for the official distribution we'd need to compile Linux, macOS, and Windows builds twice with and without this flag?

@bruvzg
Copy link
Member Author

bruvzg commented Jul 6, 2023

I haven't followed the original discussion closely, but this would mean that for the official distribution we'd need to compile Linux, macOS, and Windows builds twice with and without this flag?

The Steam library is loaded dynamically, so a single build with flag will work fine without Steam. But I guess we do not want official non-Steam builds to have the ability to automatically load proprietary code, so probably yes.

@akien-mga
Copy link
Member

akien-mga commented May 30, 2024

I finally took the time to test this (rebased on current master), and I'm having issues initializing the steam_api64.dll library on Windows, from latest Steamwork SDK 1.59:

 ERROR: Can't resolve symbol SteamAPI_Init, error: "Error 127: The specified procedure could not be found.".
   at: (platform\windows\os_windows.cpp:497)

That seems weird as it's indeed present in the public headers for the SDK version.

@bruvzg
Copy link
Member Author

bruvzg commented May 30, 2024

Seems like it's SteamAPI_InitFlat in the newer Steamworks SDKs (SteamAPI_Init is defined as a wrapper in the header only).

@akien-mga
Copy link
Member

Indeed, I tested with SteamAPI_InitEx as you mentioned first, but that also didn't work.
With SteamAPI_InitFlat (added in 1.59 which I'm using), it seems to work fine.

So I could test that when running it through Steam, this fixes the time tracking issue.

On the other hand, successfully init'ing the Steam API seems to prevent running the binary from the command line:

PS C:\Program Files (x86)\Steam\steamapps\common\Godot Engine> .\godot.windows.opt.tools.64.exe --verbose
PS C:\Program Files (x86)\Steam\steamapps\common\Godot Engine> Loaded SteamAPI library
ERROR: Parameter "mem" is null.
   at: Memory::realloc_static (core\os\memory.cpp:130)
ERROR: Parameter "mem" is null.
   at: Memory::realloc_static (core\os\memory.cpp:130)

(no window shows up)

Is that something we could address somehow so we get the benefit when in Steam, but not the drawbacks otherwise?

Maybe there's an env variable set by Steam which we can check to know whether to bother init'ing the Steam API in the first place?

@bruvzg
Copy link
Member Author

bruvzg commented May 30, 2024

Updated, but have not tested, will do it tomorrow.

@bruvzg bruvzg marked this pull request as draft May 30, 2024 13:11
@bruvzg bruvzg marked this pull request as ready for review May 31, 2024 08:27
@bruvzg
Copy link
Member Author

bruvzg commented May 31, 2024

Tested it (on macOS and Windows, both 32-bit and 64-bit), seems to work fine.

In the other hand, successfully init'ing the Steam API seems to prevent running the binary from the command line

I can't reproduce it. This might be memory corruption, since new function require fixed size string buffer, if you replaced only the function name it might write over some other data.

@akien-mga
Copy link
Member

I can't reproduce it. This might be memory corruption, since new function require fixed size string buffer, if you replaced only the function name it might write over some other data.

Yeah I had just replaced the function name so that's probably why.

I'll test the new version asap.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Tested on Windows and Linux, seems to work well.

On Linux, time tracking was already working fine and doesn't regress.
On Windows, this PR solves the time tracking issue.

@akien-mga akien-mga merged commit 0d11108 into godotengine:master Jun 3, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release enhancement feature proposal topic:editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Steam not logging Godot usage time correctly.
4 participants