Is DecodeManagedData supposed to return null?

[regalialong]

This actually tripped me up when trying to decode ETC_RGB4Crunched's cousin ETC2_RGBA8Crunched.

DecodeManagedData has the format switch to enter the code to actually decode the texture data for the format, but if your texture format isn't included, the size gets set to zero and then explicitly returned as null:

AssetsTools.NET/AssetsTools.NET.Texture/TextureFile.cs

Lines 569 to 583 in d7c25c7

	_ => 0
	};
	if (size == 0)
	return null;
	if (swizzler != null)
	output = swizzler.PostprocessDeswizzle(output);
	if (!useBgra)
	TextureOperations.SwapRBComponents(output);
	return output;
	}

But the signature doesn't declare nullability anywhere:

AssetsTools.NET/AssetsTools.NET.Texture/TextureFile.cs

Lines 455 to 458 in d7c25c7

	public static byte[] DecodeManagedData(
	byte[] data, TextureFormat format, int width, int height, bool useBgra = true, ISwizzler swizzler = null)
	{

I did spot that the csproj disables nullability (old .NET compat?), so that's probably why this doesn't generate a lint when writing code like that normally would.

The problem is that this throws off Rider which doesn't warn callers about the possibility of a null return value, so when I try to write out the resulting data with an unsupported format, I try to write a null and crash my app.

I did spot that your code checks for null, so I assume it returning null is wanted.

Maybe add CanBeNullAttribute to suggest users to check for null even if nullability is disabled for your project?

You do also have the empty output Array at the top that you could return instead, then callers would need to check size instead (like your code already does), which at least wouldn't crash on access.

It's fine if you don't think this matters, might want to put this in xmldocs instead then, but it footgunned me the first time

[nesrak1]

I did spot that the csproj disables nullability (old .NET compat?), so that's probably why this doesn't generate a lint when writing code like that normally would.

The problem is that this throws off Rider which doesn't warn callers about the possibility of a null return value, so when I try to write out the resulting data with an unsupported format, I try to write a null and crash my app.

I think I determined nullable annotations do not affect any version of .NET, so that's not the obstacle. AssetsTools.NET as a whole was never designed with nullability in mind. There was a version I was working on (and I still plan on working on) that gives everything nullable annotations, but it'll take a while to finish it. Some of the APIs had to change as a result.

Maybe add CanBeNullAttribute to suggest users to check for null even if nullability is disabled for your project?

I would rather not add an additional annotations dependency just to add nullability to this one method if I'm already going to use the normal .NET null annotations soon.

That all aside, as a user myself of the texture library, I still think the API has room for improvement. A lot of it is legacy code from all the way back in 2017 and hasn't gotten much of an upgrade. Like, I should not be required to get texture data manually if the TextureFile class is capable of reading texture data already.

[regalialong]

I would rather not add an additional annotations dependency just to add nullability to this one method if I'm already going to use the normal .NET null annotations soon.

Jetbrains.Annotations is useful for other things than nullability as well like MustDisposeResourceAttribute so it tends to make itself into my projects one way or another

But if you end up actually declaring this null, then yeah no worries, I just wasn't sure about the disabled nullability so that's the alternative to clue in analyzers

That all aside, as a user myself of the texture library, I still think the API has room for improvement. A lot of it is legacy code from all the way back in 2017 and hasn't gotten much of an upgrade. Like, I should not be required to get texture data manually if the TextureFile class is capable of reading texture data already.

It's pretty underdocumented but with Next's code side by side, it sure saves me a lot of sanity compared to writing the code myself :^)