osu!taiko pooling [seems like incorrect way] #31294
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Is pooling faster than caching for sure?
KiaiHitExplosion:
```
[Cached(typeof(DrawableHitObject))]
public readonly DrawableHitObject JudgedObject;
```
TODO:
* `DefaultKiaiHitExplosion.cs`: strong hit looks better with greater X scaling. but I don't sure about perf & it's not about this PR actually
* `TaikoPlayfield.cs`: seems like Kiai explosion should be displayed only whin hit. Am I wrong? (`result.IsHit`)
DrawableHit & Hit types splitted into two TODO: * correct way (currently not working) for TaikoModSwap * correct way (currently exception ^~^) for editor * remove `ChangeType` (at least current realization) * fix Tests XD * run benchmarks -- is the changes useful at all?
. `EditorBeatmap:ChangeOnSelection`: seems like `EditorSelectionHandler` don't expected to create / remove HitObjects and therefore it don't called redraw. BUT object redrawing don't called only when we call this by keydown (and in that way savestates (redo/undo) works! and work correctly), but if we perform HitObject change by rightclick menu then it work correctly, and display correctly but there no undo/redo record :/ need to fix tests, maybe they say where the problem
Nikita-str
changed the title
|
I am so confused by all of this. Do you want help or not? Is this pull request "the correct way" or not? Should I "read the text" in the OP or not? Why is this open even? |
Right now I don't need help, except for question in the end of this comment. THIS WAY IS NOT CORRECT Meanwhile, can you say how I can get |
|
A better solution has been found therefore this PR closed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Don't read text below there is more simpler approach
Help Wanted:
<editor right-click-menu click action>and<editor key down action>for the same action (W/R key: set rim types for the selection) perform different behavior? Action is correct for right-click-menu but only do redo/undo record for W/R key down. There is changes inTaikoSelectionHandler:SetRimStatebut as I can understand it's called for both click & key down. Seems like it's not expected fromEditorSelectionHandlerto remove/create objects? And therefore some events don't called? Sorry for uncertainty.Samples.Volume(only forHitRimclass that is differ fromHitCentre)? Seems like the problem is aboutSampleschanging/setting, but I couldn't find where it, and maybe something like this already happened before and so it's simpler to ask.Please also see See.1 in the end of PR comment
Try to fix #21072
About The Issue Solution:
Added
HitCentre: Hit&HitRim: Hitclasses that corresponding to don & katsu circles. They cannot changeHit.Typeand now we can do:in
TaikoPlayfield.And with them there are problems mentioned in
Help Wantedsection.Also added pool for
KiaiHitExplosion(first commit), with it there no problem.Just for case, do I understand correctly that changing the
ContentinDrawableHitObjectis long performing operation that should be optimized? (because if not, and the long operation is creating newSkinnableDrawableand we can change them dynamically inContentthen this changes can be done more-more simpler)Should be solved before non-Draft:
Except for problems mentioned in
Help Wantedsection, there is some todo's that is just question about how to do better & simple bugs that do not require help, but should to be mentioned and closed before make PR non-Draft:DefaultKiaiHitExplosion.cs: strong hit looks better with greater X scaling. but I don't sure about perf & it's not about this PR actually (question).TaikoPlayfield.cs: seems like Kiai explosion should be displayed only whin hit(whenresult.IsHit). Am I wrong? (question)DrawableFlyingHit.cs: what should we draw on hit? I use swell circle but don't sure that it's correct. Before we drawDrawableHitwith dynamicSkinnableDrawable. In osu!stable draws yellow-to-red Hit Cirlce, so maybe there should be some additionalTaikoSkinComponents? (question)One more time, sorry for big uncertainty, real help needed only for
Help Wantedsection because it takes more time than I expected.See.1:
Maybe the solution approach is bad, maybe there is more good way to do all this changes without splitting
HitintoHitCentre&HitRim? but how else can we pool it? Maybe give a way to get your pool inPlayfield.cs:IPooledHitObjectProvider.GetPooledDrawableRepresentationif there noRegisterPool? and then we can give correct pool forHitwithout splitting it intoHitCentre&HitRim? Such approach seems more simpler.