fix: Update Texture reload to prevent crash due to wrong type.

[Doprez]

PR Details

when reloading a Texture it would try to pass in a value of type Image to the content manager for an asset that of type Texture.

Description

This adds a new method to the Content loader to check the type of an asset from the URL.

This also adds that check to the Texture class to determine how it should be treated when reloading.

Related Issue

#2008
#1493

Motivation and Context

Its the start of a fix for a seemingly larger problem with disposing of the graphics device when the game is in fullscreen.

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My change requires a change to the documentation.
• I have added tests to cover my changes.
• All new and existing tests passed.
• I have built and run the editor to try this change out.

[Doprez]

I was also debating if there should be the possibility to prevent crashing on failed Asset loading by adding a try catch but I know its best to avoid if possible.

        public T Load<T>(string url, ContentManagerLoaderSettings settings = null) where T : class
        {
            try
            {
                return (T)Load(typeof(T), url, settings);
            }
            catch(Exception ex)
            {
                Log.Error($"Error loading asset '{url}': {ex.Message}");
                return null;
            }
        }

instead of

        public T Load<T>(string url, ContentManagerLoaderSettings settings = null) where T : class
        {
            return (T)Load(typeof(T), url, settings);
        }

[Jklawreszuk]

Looks legit 😄

[Eideren]

This is far more efficient:

var textureDataReloaded = assetManager.Load<object>(url);
if (textureDataReloaded is Texture tex)
   // ...
else if (textureDataReloaded is Image img)
   // ...

As for the method signature you introduced in ContentManager - if we were to have a method for this purpose, I think it's best to have a bool TryGetLoadedAsset(string myUrl, out object myAsset) and then the user would test the asset's type if that's what they're looking for, but they could also just use the asset itself as an added feature. But this would effectively be the same as calling IsLoaded(url) followed by Load<object>(url) and testing the type like shown above. So all in all I think it's best to remove public Type GetType(string url) and we could instead document the fact that Load<T> works with a derived type.

[Doprez]

Updated based on the suggestion. I agree with the documentation especially, I had no clue you could just use the base type object even though it makes perfect sense thining about it now.

[Eideren]

Thanks !