Make ISpriteLoader aware of the source file name.

[IceReaper]

Sometimes games have have multiple different sprite formats, where the only possible filetype detection algorythm would be wether the file could be successfully parsed till fileend is reached... This PR basicaly makes the ISpriteLoader aware of the filename, which allows in these cases to avoid unnecesary complex code based checks and simply rely on the file extension.

[penev92]

That seems messier than it should be. You have methods that take a Stream and are supposed to produce frames out of the data in it, but at the same time take a filename they may or may not need (but very likely shouldn't need).
I would prefer to see a CanHandle() method on the Parsers/Loaders/relevant classes instead (and not just for sprites, ofc) to have a cleaner and more standardized solution.

[pchote]

That job is already performed by TryParseSprite. Adding a new method to the interface that duplicates this role would be far messier than just giving it the info that it needs.

[penev92]

A potential CanHandle doesn't necessarily need to do all the heavy lifting that a TryParse is required to do.

[pchote]

TryParse isn't required to (and really shouldn't be!) do any heavy lifting if there is a quick way to determine that the file isn't valid - it should be exiting after as little work as possible.

Adding a new method, on the other hand, now either requires you to do the same checks twice, or to say that CanHandle is allowed to return true in cases where TryParse returns false. These are both poor design outcomes.

[teinarss]

Wouldn't it be better to let ISpiritLoader to expose a property on what file exts it can handle? Or do we have cases when there is no file exts to check?

[pchote]

IMO that would be needlessly restrictive, by preventing implementations that might want to filter on more than just the extension (e.g. the directory that holds the sprites) and would also signal that we encourage implementations to use extensions as their primary filtering mechanism which they really shouldn't be!

[reaperrr]

https://github.com/OpenRA/OpenRA/wiki/Changelog-(bleed)/_compare/e6ed27ddef788582d0f98bbe37abd2cc20369ac9...fc271cd9bb1713afa264680484c5cb10b462f62a