osu!taiko Hit & KiaiHitExplosion pooling

[Nikita-str]

Fixes #21072

Need To Disucss:

• Which skin should be used here? Previously was drawn dynamic Hit. In osu!stable there drawn a circle with color that changed from yellow to red. Maybe there should be additional TaikoSkinComponents?
• seems like Kiai explosion should be displayed only when hit happens. Am I wrong?
• Is there a benches that check performance related to pooling? How to run it?

Solution Notes:
The Hit part of solution use different behavior for editor and non-editor mode. When it's EditorMode we still recreate Hit pieces each Apply (also see) but in game we don't recreate it. It's done in that way because SetRimState should to change SkinnableDrawable for the same HitObject and seems like DrawableHitObject is buffered for it and simple Remove + Insert does not work.
Instead of doing this + this + this you can split taiko Hit into few types and pooling by them but such solution is more complicated.

[bdach]

Not even sure where to begin with this one. What is anything supposed to be here? AdditionalPrepareDrawablePool()? OnLoadSkinnableCreate()? OnLoadCreateMainPiece()? RestorePieceState()? Zero xmldoc on any of these?

It just all looks immediately wrong on entry. Dunno if I'm willing to put myself through review of this.

[Nikita-str]

Some comments added

---

Firstly additionally say about about OnLoadCreateMainPiece & RestorePieceState: the problem was here (at least, maybe there is some low-perf actions besides it). SkinnableDrawable was recreated each OnApply. Now its fixed and it's create only in load function. Except for editor mode for DrawableHit.

As you can see there was function CreateMainPiece that called each OnApply. Now it's called only on load so it's renamed to OnLoadCreateMainPiece. I don't know which comments I should add here :\

About RestorePieceState: this function already was before and it was named RecreatePieces because it call CreateMainPiece and in previous my changes it's NOT call it so it's not a recreate so I rename it, and yeah you can delete it and move all actions (that now do NOT create SkinnableDrawable) in OnApply function, but for me it's not good to delete important TODO before it will be twice checked. but ok — I remove it.

But please check OnApplys because I assume that low-perf-action is call new SkinnableDrawable and change Content but maybe there is some others actions that should be optimized, and I just don't know which one. For example is it ok to change Height, Size on OnApply? Both of this now can be moved in LoadComplete funciton (if animation don't change them), but I want to make as few changes as it possible so I don't move it. And is it ok to call updateColour on OnApply?

---

Next about KiaiHitExplosion pooling, because there almost no changes actually but github says 150 lines

Firstly compare prev HitExplosion with new HitExplosionBase as you see HitExplosionBase contains only code from HitExplosion, except for in load function this:

InternalChild = skinnable = new SkinnableDrawable(new TaikoSkinComponentLookup(getComponentName(result)), _ => new DefaultHitExplosion(result));

changed to this:

InternalChild = Skinnable = OnLoadSkinnableCreate();

(So again I really don't know which comments you except from me about OnLoadSkinnableCreate... it's create SkinnableDrawable on load, it does what it's name says).
Previously logic for KiaiHitExplosion was differ from HitExplosion, but mostly they the same, yes? So now they both based on HitExplosionBase.
Also you can see that new version of HitExplosion file adds nothing new. All logic just moved to HitExplosionBase and reused by KiaiHitExplosion too.

So now we can create pool for KiaiHitExplosion exactly the same as for HitExplosion.
Compare:

• this(not changed code) and this(added code)
• this(not changed code) and this(added code)
• this(not changed code) and this(added code)
• this(not changed code) and this(added code)

These are all changes that happened around KiaiHitExplosion.

[Nikita-str]

Oh yeah, AdditionalPrepareDrawablePool was renamed into PropertyBasedDrawableHitObjectPool and it used because PoolableDrawable for taiko Hit is determined not only by its type therefore we cannot use RegisterPool for taiko Hit. Also add some comments.