Skip to content

Conversation

United600
Copy link
Collaborator

Works perfectly fine on both Debug and Release builds, but I still have some concerns about the Store build. @rocksdanister Can you take a look and confirm that there are no regressions or hidden edge cases?

Followed the steps in this guide.

@United600 United600 requested a review from Copilot September 14, 2025 16:21
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the codebase from Newtonsoft.Json to System.Text.Json following Microsoft's migration guide. The change aims to improve performance and reduce dependencies by using the built-in JSON serialization library.

Key changes:

  • Replaced Newtonsoft.Json with System.Text.Json imports across affected files
  • Updated JSON parsing logic to use JsonDocument and JsonElement APIs
  • Modified serialization calls to use JsonSerializer instead of JsonConvert

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
LivelyWallpaperPlayerViewModel.cs Updated JSON parsing logic for wallpaper properties and replaced serialization calls
LivelyWallpaperService.cs Replaced deserialization calls for LivelyInfoModel
WebView2Util.cs Updated parameter serialization in script execution helper

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@rocksdanister
Copy link
Contributor

rocksdanister commented Sep 14, 2025

In Lively I am using JsonConverter now.

Here that maybe overkill, don't have experience with System.Text.Json but LivelyProperties.json is designed with backward compatibility in mind - will not have required properties missing (unless the wallpaper itself is bad) and existing properties are not changed in future update.. so the checks are probably not necessary.

LivelyInfo.json can have missing fields depending on how and when its created but here we are deserializing so should be fine, its specification won't change in the future besides adding new properties.

@United600
Copy link
Collaborator Author

Thanks for the info. It should work fine, but I’m still mildly concerned about .Net Native support for System.Text.Json.

@rocksdanister
Copy link
Contributor

You can verify by running with .NET Native in debug mode with this wallpaper (which I use for testing myself):
Property API Test.zip

@United600
Copy link
Collaborator Author

You can verify by running with .NET Native in debug mode with this wallpaper (which I use for testing myself): Property API Test.zip

The info looks correct but can't verify the properties. Debug and Release mode aren't really my concern.

LivelyWallpaper item info

@rocksdanister
Copy link
Contributor

rocksdanister commented Sep 15, 2025

The info looks correct but can't verify the properties.

Just remove the music wallpaper check and run the wallpaper as visualizer.

if (!model.IsLocalWebWallpaper() || !model.IsMusicWallpaper())

@United600
Copy link
Collaborator Author

Correct again, but the app hangs after clicking Continue on the breakpoint (which I assume is expected).

Gravacao.2025-09-15.204929.mp4

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.

2 participants