search

readyplayerme / rpm-unity-sdk-avatar-loader

Avatar Loader is responsible for downloading, importing and preparing RPM avatars for use in editor and runtime.
MIT License
12 stars 5 forks source link

Avatar Cache always discarding up-to-date avatar because of LastModified DateTime mismatch #54

Closed aherbig closed 1 year ago

aherbig commented 1 year ago

Describe the bug Avatar Caching does not work properly.

Seems that the LastModified date from the MetadataDownloader is parsed from the Response-Header in a UTC Timezone via DateTime.Parse. The LastModified date from the local json is parsed via newtonsoft.json and puts out a different timezone.

Even though both times are "equal", the check inside the MetadataDownloader.IsUpdated will always return false because "metadata.LastModified == previousMetadata.LastModified" are always different due to the TimeZones of the DateTime objects being different.

private bool IsUpdated(AvatarMetadata metadata, AvatarUri uri, bool avatarCachingEnabled) { AvatarMetadata previousMetadata = LoadFromFile(uri.LocalMetadataPath); if (avatarCachingEnabled && metadata.LastModified == previousMetadata.LastModified) return false; return true; }

To Reproduce Steps to reproduce the behavior:

  1. Enable Avatar Cache
  2. Load an Avatar via the AvatarObjectLoader
  3. Wait until its loaded
  4. Load the same avatar via the AvatarObjectLoader
  5. Previously loaded avatar gets deleted and redownloaded, even though it was not modified

Expected behavior Already downloaded Avatar should be reused

Additional context My computers local timezone is GMT+1. I can imagine the bug not presenting itself when testing on a device that uses GMT timezone

srcnalt commented 1 year ago

Hi @aherbig can you load the avatar-loader package directly from the branch to see if the issue resolved? https://github.com/readyplayerme/rpm-unity-sdk-avatar-loader.git#feature/caching-update

aherbig commented 1 year ago

Hey @srcnalt works perfectly. Thank you.

I also noticed why it caused trouble in our project.

We are using this code here to set up Newtonsoft.Json to parse everything in UTC timezone:

JsonConvert.DefaultSettings = () => new JsonSerializerSettings { DateTimeZoneHandling = DateTimeZoneHandling.Utc, DateParseHandling = DateParseHandling.DateTime };

Maybe you want to include that in a new unit test to prevent any future issues.

Thank you so much for following up on that issue

srcnalt commented 1 year ago

Thank you for the suggestion, I will check this out!