citymania-org / grf-py

A Python framework for making NewGRFs for OpenTTD
GNU General Public License v2.0
15 stars 5 forks source link

`ImageSprite` breaks the cache mechanism #27

Closed ahyangyi closed 7 months ago

ahyangyi commented 7 months ago

Hi,

I have noticed that ImageSprite does not work when I use at least two instances. It turns out that its fingerprint is None and it seems None is used as the cache key, causing any two instances of ImageSprite to conflict.

At this moment I will work around the issue by subclassing ImageSprite (which I was going to do anyways), but I think it's still a bug worth fixing someday.

ldpl commented 7 months ago

None was supposed to mean uncacheable but things might have gone wrong, I'll take a look at some point.

ahyangyi commented 7 months ago

None was supposed to mean uncacheable

Thanks for that explanation! I think I understand what's going on, then.

I used constructs in the form of WithMask(ImageSprite(), ImageSprite()). Hence, while those ImageSprites themselves are uncacheable, perhaps the cache manager attempted to cache those WithMask objects, and saw the same fingerprint?

ldpl commented 7 months ago

Oh, yeah, that explains it, WithMask fingerpring is just wrong, it doesn't treat those Nonevalues correctly. Either way, I don't like this None approach and plan to switch to using exceptions for non-cacheable sprites so this may be a good occasion to actually do that.