Fix Steam achievements from always being disabled#700
Conversation
📝 WalkthroughWalkthroughThis PR adds an Changes
Estimated Code Review Effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
source/Clients/SteamAchievements.cs (1)
508-511: LGTM — correctly restores the Steam achievements gating.This override addresses the regression in
#695: without it,SteamAchievementsinherited the base defaultEnabledInSettings() => false, soSuccessStoryDatabase.VerifToAddOrShow()would short-circuit and log "found no action" for every Steam game (matching the symptom in the linked issue). The implementation also mirrors the established pattern inPSNAchievements.EnabledInSettings(), keeping the achievement providers consistent.📝 Optional: add an XML doc comment for consistency
Every other public override in this file (e.g.,
ValidateConfiguration,IsConnected,IsConfigured) carries an XML doc summary. Adding one here keeps the file uniform:+ /// <summary> + /// Indicates whether the Steam achievements client is enabled in the plugin settings. + /// </summary> + /// <returns>True if <see cref="SuccessStorySettings.EnableSteam"/> is set, otherwise false.</returns> public override bool EnabledInSettings() { return PluginDatabase.PluginSettings.Settings.EnableSteam; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/Clients/SteamAchievements.cs` around lines 508 - 511, Restore the Steam gating by keeping the EnabledInSettings override in SteamAchievements (public override bool EnabledInSettings()) and add a concise XML doc summary above it consistent with other overrides (e.g., ValidateConfiguration, IsConnected, IsConfigured) describing that it returns whether Steam integration is enabled in PluginSettings.Settings.EnableSteam; ensure the method body remains "return PluginDatabase.PluginSettings.Settings.EnableSteam;" and the XML comment uses the same style as the other public overrides.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@source/Clients/SteamAchievements.cs`:
- Around line 508-511: Restore the Steam gating by keeping the EnabledInSettings
override in SteamAchievements (public override bool EnabledInSettings()) and add
a concise XML doc summary above it consistent with other overrides (e.g.,
ValidateConfiguration, IsConnected, IsConfigured) describing that it returns
whether Steam integration is enabled in PluginSettings.Settings.EnableSteam;
ensure the method body remains "return
PluginDatabase.PluginSettings.Settings.EnableSteam;" and the XML comment uses
the same style as the other public overrides.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4936762a-0a60-4c7c-9c41-da9f01621add
📒 Files selected for processing (1)
source/Clients/SteamAchievements.cs
|
Works, thank you! |
|
playnite-successstory-plugin-fixsteam.zip Hi! Since the official update isn't out yet, I compiled a ready-to-use version incorporating this PR #700 fix to make Steam sync work again. In addition to this, I also applied a database fix in ItemCollection.cs (changing .Add to .TryAdd) to prevent the plugin from crashing when it tries to save data concurrently. You can download the .pext file in the zip attached below. Just drag and drop it into Playnite to install. Hope this helps! |
This is what I was looking for: An easy install! I consider myself a superuser, but everything above your post simply gave me a headache, so thank you uWaazy |
Fixes #695
Summary by CodeRabbit