Fix tvOS Storage#1876
Conversation
|
Hey, not to sidetrack you but it might be worth looking at some of the changes here: Primarily just the
Just so we're not migrating these over. These are settings saved on the |
|
Thanks for the consideration, but this work is separate from adding new settings. |
There was a problem hiding this comment.
One of the items that I am testing is migrating from Main/1.5(1) to this branch. When I do this, I lose my servers with the following errors:
[💥 Critical] Field.Stored#228:subscript(_enclosingInstance:wrapped:storage:) Attempted to access 'StoredUser''s value outside it's designated queue.
CoreData: warning: dynamic accessors failed to find @property implementation for 'ownerID' for entity 'AnyData' while resolving selector 'ownerID' on class '_CoreStore.CoreStoreManagedObject__V3___TtCOO12Swiftfin_iOS13SwiftfinStore2V37AnyData__AnyData'. Did you remember to declare it @dynamic or @synthesized in the @implementation ?
CoreData: warning: dynamic accessors failed to find @property implementation for 'ownerID' for entity 'AnyData' while resolving selector 'ownerID' on class '_CoreStore.CoreStoreManagedObject__V3___TtCOO12Swiftfin_iOS13SwiftfinStore2V37AnyData__AnyData'. Did you remember to declare it @dynamic or @synthesized in the @implementation ?
CoreData: warning: dynamic accessors failed to find @property implementation for 'field' for entity 'AnyData' while resolving selector 'field' on class '_CoreStore.CoreStoreManagedObject__V3___TtCOO12Swiftfin_iOS13SwiftfinStore2V37AnyData__AnyData'. Did you remember to declare it @dynamic or @synthesized in the @implementation ?
CoreData: warning: dynamic accessors failed to find @property implementation for 'field' for entity 'AnyData' while resolving selector 'field' on class '_CoreStore.CoreStoreManagedObject__V3___TtCOO12Swiftfin_iOS13SwiftfinStore2V37AnyData__AnyData'. Did you remember to declare it @dynamic or @synthesized in the @implementation ?
CoreData: warning: dynamic accessors failed to find @property implementation for 'key' for entity 'AnyData' while resolving selector 'key' on class '_CoreStore.CoreStoreManagedObject__V3___TtCOO12Swiftfin_iOS13SwiftfinStore2V37AnyData__AnyData'. Did you remember to declare it @dynamic or @synthesized in the @implementation ?
CoreData: warning: dynamic accessors failed to find @property implementation for 'key' for entity 'AnyData' while resolving selector 'key' on class '_CoreStore.CoreStoreManagedObject__V3___TtCOO12Swiftfin_iOS13SwiftfinStore2V37AnyData__AnyData'. Did you remember to declare it @dynamic or @synthesized in the @implementation ?
Called -[UIContextMenuInteraction updateVisibleMenuWithBlock:] while no context menu is visible. This won't do anything.This does not exist on a clean install nor on future app usage after the initial wipe. As a note, none of the user settings are lost. Only the server. Once I log back into the server all my settings are back where they were before.
I don't think this is egregious, especially for tvOS, if we document this in the release notes that settings will be lost. If it's possible to migrate that would be ideal though.
/Users/runner/work/Swiftfin/Swiftfin/Shared/Components/Marquee.swift:86:1: error: (redundantEmptyView) Remove redundant else { EmptyView() } branches in SwiftUI result builders.
|
Also added the the fix for the linting error. It's unrelated to this PR so idk why |
JPKribs
left a comment
There was a problem hiding this comment.
Settings are now saved at the device level instead of at the user level. So, changing a setting in Jellyfin User A then switching to Jellyfin User B will give me Jellyfin User A's settings. If I change something in Jellyfin User B, the setting will update for Jellyfin User A.
On tvOS, this is the same but switching Apple TV users I have different settings. That tradeoff seems correct.
Again, this doesn't seem like a dealbreaker but this changes all settings to be device settings opposed to Jellyfin User settings. Checking some other apps, this is how most other apps do this as well so this could be ideal.
This would also prevent us from the tvOS issue where StoredValues were N*size for usage since we have a smaller size limit there.
Mostly just FYI / is the above intentional
|
Alright sorry about that! I've wrapped up testing and this all looks and works as expected with our 2 caveats:
Other than these two, this looks and works as expected! I think even 2 seems to reflect what a lot of other apps I have do as well so this is likely just 1). |
|
Thanks for the thorough testing!
This was due to a mis-casing of keys between
This is the same issue as I've noticed earlier, where it's not "per device" but that the settings would reflect the "first signed in user during the current app session". This was caused by having views being made once now with |
JPKribs
left a comment
There was a problem hiding this comment.
This is the same issue as #1935 (comment), where it's not "per device" but that the settings would reflect the "first signed in user during the current app session". This was caused by having views being made once now with NavigationRoute. Additionally, the defaults keys should have been computed variables as well anyways.
Ah! This makes sense and your changes now resolve this as well!
Migration is working as expected now as well! Looks great!
|
Wow. Congrats!! |
This does a bit of work around data storage, performing the following migrations:
To help with tvOS memory pressure and overall data maintenance, we will need to implement periodic cleanup of redundant stored values and proper deletion/reset of user settings.