-
Notifications
You must be signed in to change notification settings - Fork 421
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix TextureLoaderStore
not always appending extensions to lookups
#5287
Conversation
Tests need attention here. |
`getFunc` calls `baseStore`, which has nothing to do with textures, therefore it has to explicitly specifiy the extension. Alternative would be carrying the extension provided from `name` in the `getWithBlocking` method, but I'm not sure that's necessary.
Turns out in master, osu-framework/osu.Framework/Game.cs Line 181 in 6047241
osu-framework/osu.Framework/Game.cs Lines 234 to 235 in 6047241
(notice That is also the reason why we see lookups like This is relevant in the test failure because the test was never supposed to work without specifying the extension, but it was actually working because of the png/jpg extensions being added to osu-framework/osu.Framework.Tests/Visual/Sprites/TestSceneTextures.cs Lines 264 to 266 in 4fef626
(notice |
I'll run tests game-side and confirm this doesn't regress skins and storyboards just to be sure. |
Can't seem to find any issues so far. This is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems correct
Tournament texture storage uses a
StorageBackedResourceStore
, which is not of typeResourceStore
, thereforeTextureLoaderStore
can't append the image extensions and fail lookups:osu-framework/osu.Framework/Graphics/Textures/TextureLoaderStore.cs
Lines 24 to 29 in e918d33
This is a regression from #5240, since prior to that PR,
TextureStore
appends extensions as well (this is also the reason why previously texture store sometimes looks up textures with double extensions, i.e.texture.jpg.png
).Instead, I've changed
TextureLoaderStore
to explicitly construct a resource store and add the given store along with the right extensions there, rather than safely cast and silently fail.Note that
SampleStore
andTrackStore
suffer from the same issue, which would fail similarly if constructed with a non-ResourceStore<T>
store (e.g.StorageBackedResourceStore
).