Skip to content

Update Steamworks.NET#956

Merged
SnipUndercover merged 1 commit into
EverestAPI:devfrom
roblabla:update-steamworks
Aug 24, 2025
Merged

Update Steamworks.NET#956
SnipUndercover merged 1 commit into
EverestAPI:devfrom
roblabla:update-steamworks

Conversation

@roblabla

@roblabla roblabla commented Aug 3, 2025

Copy link
Copy Markdown
Contributor

This allows mods to make use of the newer features provided by Steamworks.NET. This is a requirement of my mod providing Steam Game Recording Integration.

Needs EverestAPI/Everest-libs#5 This PR is merged!

We're not updating to the very latest Steamworks.NET as it contains a breaking change that breaks Celeste (same as rlabrecque/Steamworks.NET#709). Fixing that would be relatively easy to do (just needs to nop out the call to SteamUserStats.RequestCurrentStats() in celeste's Stats.MakeRequest), but given my use-case doesn't need the newer functionality offered by the very latest Steamworks, I opted for the path of least resistance.

The largest part of the change is in the MiniInstaller. It does two things:

  1. It copies the actual implementation dll of Steamworks.NET from lib-ext to the correct location to be picked up by Celeste.
  2. It ignores Steamworks.NET in the ConvertToNETCore call. This is because otherwise, it would overwrite the previously copied Steamworks.NET with the version found in the orig folder, breaking everything.

I verified it works on Windows 64-bit and macOS (both Steam builds).

Fixes #860

@maddie480-bot maddie480-bot added the 0: draft This PR is not ready for review yet (bot-managed) label Aug 3, 2025
@roblabla roblabla force-pushed the update-steamworks branch from bd36aac to 3a7ac3a Compare August 3, 2025 09:27
@roblabla roblabla marked this pull request as ready for review August 3, 2025 09:27
@maddie480-bot maddie480-bot added 1: review needed This PR needs 2 approvals to be merged (bot-managed) and removed 0: draft This PR is not ready for review yet (bot-managed) labels Aug 3, 2025
@roblabla

roblabla commented Aug 3, 2025

Copy link
Copy Markdown
Contributor Author

Well, this is annoying. Because I added Steamworks.NET as a PackageReference, a reference assembly gets added to the output. This is not a huge problem for the everest build, as it gets overwritten with the correct assembly. What kinda sucks though is that this reference assembly will overwrite the original Steamworks.NET.dll, which means it will end up in the orig folder, breaking the unmodded game build.

I looked for a way to massage dotnet publish into not providing that reference assembly, but couldn't find one (to be clear, I may have overlooked something - I don't do a whole lot of C#).

I can see two ways to fix this:

  1. Don't use nuget for the Steamworks.NET dependency. I could instead add Steamworks.NET.dll in libs-ext (along with steam_api). This way, it'd get copied by the MiniInstaller as necessary. This would require updating lib-stripped as well.
  2. Provide the original Steamworks.NET in libs-ext/lib-vanilla. This way, the installer would overwrite the Reference Assembly with the original file.

For now, I think I'll try pursuing the first option.

@roblabla roblabla force-pushed the update-steamworks branch from 3a7ac3a to 4d83ef6 Compare August 3, 2025 14:53
@roblabla

roblabla commented Aug 3, 2025

Copy link
Copy Markdown
Contributor Author

All fixed. Steamworks.NET.dll is now copied from lib-ext, so there's no more shenanigans going on with the orig directory.

This allows mods to make use of the newer features provided by
Steamworks.NET.
@roblabla

Copy link
Copy Markdown
Contributor Author

The everest-libs PR got merged, this is read for review.

@SnipUndercover

Copy link
Copy Markdown
Member

I'll rerun the pipeline since it errored out.

@SnipUndercover

Copy link
Copy Markdown
Member

/azp run

@SnipUndercover

Copy link
Copy Markdown
Member

@maddie480 It seems like the tas checks don't want to run? The action times out on waiting for the build to complete, even though it finished in 6 min.

@Wartori54 Wartori54 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes are good, but not too happy since if Steamworks.NET.dll was platform and arch independent all of these would not be required at all.
Patching Steamworks.NET.dll to be like that is something not really in the scope of this PR though. (But it should be looked into in the future.)

@maddie480

Copy link
Copy Markdown
Member

@maddie480 It seems like the tas checks don't want to run? The action times out on waiting for the build to complete, even though it finished in 6 min.

it happens sometimes, Azure and GitHub Actions build "different merge commits", neither of which actually exist. GitHub builds ecc2f4d, Azure builds 11cb833, because ????

@maddie480

maddie480 commented Aug 17, 2025

Copy link
Copy Markdown
Member

(pushed an empty commit then force pushed to remove it, now both GitHub Actions and Azure Pipelines are working on commit a63dd5d so it should be fine)

@maddie480-bot

Copy link
Copy Markdown
Member

The pull request was approved and entered the 3-day last-call window.
If no further reviews happen, it will end on Aug 24, 2025, 12:00 AM UTC, after which the pull request will be able to be merged.

@maddie480-bot maddie480-bot added 3: last call window This PR was approved, and is in the 5-day last-call window before getting merged (bot-managed) and removed 1: review needed This PR needs 2 approvals to be merged (bot-managed) labels Aug 18, 2025
@maddie480-bot

Copy link
Copy Markdown
Member

The last-call window for this pull request ended. It can now be merged if no blockers were brought up.

@maddie480-bot maddie480-bot added 4: ready to merge This PR was approved and the last-call window is over (bot-managed) and removed 3: last call window This PR was approved, and is in the 5-day last-call window before getting merged (bot-managed) labels Aug 23, 2025
@SnipUndercover

Copy link
Copy Markdown
Member

(pushed an empty commit then force pushed to remove it, now both GitHub Actions and Azure Pipelines are working on commit a63dd5d so it should be fine)

Well for some reason the checks are still showing that they're canceled. Either GitHub is high or something's wrong, but I don't think this'll affect TASes by any means.
In any case, Let's Gamble, Try Merging.

@SnipUndercover SnipUndercover merged commit 9baf836 into EverestAPI:dev Aug 24, 2025
3 of 7 checks passed
@maddie480

Copy link
Copy Markdown
Member

The checks on e0d57e1 pass if you go check the commit directly rather than the "Checks" tab, so I vote for "GitHub is high"

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

Labels

4: ready to merge This PR was approved and the last-call window is over (bot-managed)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bundle an updated steamapi/Steamworks.NET

5 participants