osu!taiko drum sprites are misaligned on legacy skins

[MTGPROD]

Describe the bug:

Screenshots or videos showing encountered issue:

osu!lazer version:

2020.419.0

Logs:

[peppy]

Please describe your issues better.

[MTGPROD]

I'm sorry, I wanted to let you know quickly.

[peppy]

@MTGPROD can you reproduce this on the most recent release?

[enoslayd]

Isnt this #8681?

[peppy]

Yes, quite possibly. If it's still reproducible I need some more details on how to do so.

[Fvern]

I'm experiencing this, in fact the same problem in issue #8844, on the latest version (2020.429.0) where it was supposed to be fixed.

Steps to Reproduce:
Use the skin I linked in the previous issue and play any map.

[enoslayd]

weird doesnt happen to me



i even tried both skin on #8844 and it doesnt happen
2020.429.0

did you try to delete and reimport the skins?

[Fvern]

Edit: Actually, no. I reimported before updating. Lemme try that.

[Fvern]

Tried it, did not fix the issue.
Edit: Extra info: Pulled the latest code and built osu.Desktop in VS, still having the same issue in there.

[Joehuu]

Seems to happen when Beatmap skins is enabled.

[MTGPROD]

@peppy Same thing (I precise that i'm on Android)

[MTGPROD]

@Joehuu Yes, you're right I deactivated beatmap skin and..

[peppy]

I'll try and get to this, but if someone does happen to look into this before me, this is due to the skin version being read using the full fallback logic when we actually want to know the skin version-at-point-of-usage (in this case, the texture source).

Or maybe a better solution being that a beatmap should not be allowed to provide a skin version (needs regression testing if done this way).

[peppy]

Paper trail points to https://github.com/ppy/osu/pull/8643, which adds 1.0 fallback for the purpose of skins primarily (see linked issue). I'm wondering if we should just drop the version looking from LegacyBeatmapSkin for simplicity. @smoogipoo opinions? If we keep it, we're going to have to change the config lookup to be more like GetSkinFromTextureLookup("texture").Configuration.LegacyVersion..

[smoogipoo]

So the version would only come from the user skin? I think that's ok. Doesn't stable do that anyway?

[peppy]

I'd say stable also does this (the complex fallback logic doesn't look to be present at all in stable, probably with good reason).