Adjust catch playfield and catcher layout to further match osu!(stable) #24895
Conversation
|
@frenzibyte This needs revisiting, not sure if due to #24706 being merged or due to the catcher not being limited by that PR, but:
|
|
To elaborate on the above: maybe this is "correct" behaviour based on the classic skin's sprite offsets, but needs to be double-checked. |
|
From a quick glance at what's happening, that looks to be indeed "correct" behaviour. In osu!(stable), the origin position of the catcher is set to exactly 16 pixels from the top of the sprite (in 1x version, where catcher is 306x320), regardless of its height or anything else. Also confirmed visually by scaling sprite to 8x its size and checking in gameplay on stable: 
|
| // Sets the origin roughly to the centre of the catcher's plate to allow for correct scaling. | ||
| OriginPosition = new Vector2(0.5f, 0.06f) * Catcher.BASE_SIZE; | ||
| // Sets the origin to the top of the catcher's plate to allow for correct scaling. | ||
| OriginPosition = new Vector2(0.5f, 0.0525f) * Catcher.BASE_SIZE; |
There was a problem hiding this comment.
The derivation of this 0.0525f magic constant should be spelled out in comments. I refuse to spend time on reconstructing the math. The OP only links random xmldoc:
see this xmldoc and stable's definition of
catcherWidthfor more information.
I don't know how you get from any of these above to 0.0525f.
If you have this stuff calculated and cross-referenced already, the least you can do is to provide it for double-checking.
There was a problem hiding this comment.
This part is ballparked and can be reverted I believe, the comment already states it's a close value, but I guess that's not quite clear.
There was a problem hiding this comment.
@frenzibyte Are you going to redo visual comparisons after attempting the revert?
| // stable applies a 0.5x factor to all gamefield sprites (assuming game field is 512x384), | ||
| // and also a constant 0.7x factor for catcher sprites specifically. | ||
| Scale = new Vector2(0.5f * 0.7f); |
There was a problem hiding this comment.
Where are these factors in stable code?
There was a problem hiding this comment.
I have opened this PR before we seemed to have allowed referencing osu!(stable) code over the codebase (in a recently merged PR that I have forgot its name), so I did not reference by line.
| // there is no explanation to this offset, but applying it brings lazer nearly 1:1 with stable. | ||
| const float stable_offset = 6f; |
There was a problem hiding this comment.
May have something to do with #9763. That said I can't find the original offset in stable source so may be a red herring.
There was a problem hiding this comment.
Question is, if this is the same thing then which out of 6 and 8 are more correct. I highly doubt it differs per ruleset.
There was a problem hiding this comment.
Hmm, you may be onto something. I'll look into this further more, there is something I want to check.
There was a problem hiding this comment.
"6" was apparently ballparked. So I'm betting on the fact that one (or both?) values were ballparked.
There was a problem hiding this comment.
Turns out the vertical offset that is applied in osu! ruleset is specific to it, stable does not apply any similar adjustment to catch (see this).
| @@ -81,5 +83,11 @@ private void load() | |||
| }, | |||
| }; | |||
| } | |||
|
|
|||
| protected override void Update() | |||
There was a problem hiding this comment.
Surely this can be avoided by adjusting the visual elements to use centre origin...
There was a problem hiding this comment.
I didn't look too much, you might have better clues on how this one can be adjusted than I do.
|
I am going to close this PR as I have figured out the source of the discrepancy and want to split the changes such as the playfield layout change becomes a follow-up of all the other changes, in order to help with review. |
This PR is not quite final, but I've spent so much time on this hitting many dead-ends to the point I thought opening a PR and leaving it for someone else to provide a better idea would be wiser (it hurts to give up, but I've never been good at this kind of stuff in the first place).
Some of the changes in here may not make any sense, but that's as far as I could go with them. I'll briefly explain each commit in here.
Adjust catcher origin position to match 1:1 with stable
In stable, the catcher's origin position is always set to Y = 16, as per the following code:
We cannot immediately use that number in
SkinnableCatcher, because we use dimensions that are technically different from stable (they're same as stable but with stable scale numbers factored in). The factor to convert from stable dimensions to lazer dimensions is basically 0.35x, see this xmldoc and stable's definition ofcatcherWidthfor more information.We can simply convert stable's origin position using that factor, resulting in Y = 5.6, dividing by
BASE_SIZEresults in Y = ~0.525Fix legacy catcher sprites getting shrunk
This is a no-brainer, stable doesn't resize any of its sprites, meanwhile lazer is shrinking them down to fit the catcher's catching range boundaries (i.e.
BASE_SIZE).Adjust catch playfield layout to further match stable
This is where things get quite quirky. In stable, the playfield is positioned vertically at essentially 18.75% of the window's height (see this note and stable's code).
This must be done in osu!(lazer) in order for the catcher's positioning to match stable.
But it's actually not enough...
...the above is also required for the catcher's position to get as close to stable as possible, and I cannot say anything beyond that I only got that value from eyeballing the catcher between stable and lazer.
That offset sounds redundant, but it shows that this whole change is not enough, or that it's misguided for a certain reason.
I could go ahead and revert all changes other than the one that actually fixes catcher size being different on legacy skins, but I'm not sure how gameplay would sit with catch players once lazer goes ranked, so I thought it's better to focus on matching playfield layout with stable as well.