osu
osu copied to clipboard
ppy
New footer design implementation and rewrite of FooterButton class
- Addresses https://github.com/ppy/osu/discussions/20135#
- Resolves https://github.com/ppy/osu/issues/5923
Tried to stick as best as i could to the designs
matched the animations and such with ShearedButton.cs for consistency and rewrote the code to more closely track with that.
Sample Vid:
https://user-images.githubusercontent.com/74463310/188483167-e0475761-9ec4-40c3-8cf4-0e85faf0e8a2.mov
will do the back button and Logo replacement seperately since they are more finnicky and lots of screens depend on them
The metrics seem really off from the design. Is there a reason for that? (ie. font size, icon size, bottom colour bar size).
The metrics seem really off from the design. Is there a reason for that?
some i changed purposefully because they didn't match the sized you would expect from the design if i took the values at face value, especially the text , the base button width, etc..
i tested at 1x scale on a 1080 p screen and both are in pixels i assume so im not sure why that could be. if you want i can match them exactly
@mk56-spn have you read #17940?
nope, this explains so much, ill adjust accordingly
some i changed purposefully because they didn't match the sized you would expect from the design if i took the values at face value, especially the text , the base button width, etc..
I'd expect them to match visually if that's the case, but they don't.
some i changed purposefully because they didn't match the sized you would expect from the design if i took the values at face value, especially the text , the base button width, etc..
I'd expect them to match visually if that's the case, but they don't.
i apparently had the shear set up incorrectly which was causing a lot of the issues i saw when working on it
one thing, ive resized the bar because its 60 px tall in this iteration of design are you ok with this looking as such:
shall i resize back to 50 for now or just make sure to get the back button pr done before this needs merging?
Due to the refactor to FooterButton, the Rewind action (right click on the FooterButtonRandom button) behaves a little strange in relation to animations. For example, right clicking on the button will not call the base OnMouseUp() function and won't scale back to the original size because of this early return: https://github.com/ppy/osu/blob/97e85ef0d844a2de23715b547dde2bf89713af44/osu.Game/Screens/Select/FooterButtonRandom.cs#L121-L131
Also of note, it seems that in the FooterButtonRandom class, the OnPressed() and OnReleased() don't call their base implementations, which means that using F2 to trigger the random button does not play the scaling animations.
The tweening and animation style is completely wrong with this change, but I can fix that once the other issues are fixed. Maybe. For now, please remove the scaling of the buttons during mouse down completely. And change the buttons to be anchored to the bottom of the parent, not the top.
@arflyte please advise how you imagined mods showing in the footer, if at all. this ad-hoc implementation looks very bad:
https://user-images.githubusercontent.com/191335/188546146-f0956449-3d53-47d9-a2b5-ffb64e51bbee.mp4
The tweening and animation style is completely wrong with this change, but I can fix that once the other issues are fixed. Maybe. For now, please remove the scaling of the buttons during mouse down completely. And change the buttons to be anchored to the bottom of the parent, not the top.
@arflyte please advise how you imagined mods showing in the footer, if at all. this ad-hoc implementation looks very bad:
https://user-images.githubusercontent.com/191335/188546146-f0956449-3d53-47d9-a2b5-ffb64e51bbee.mp4
Waiting on the answer to this since I've restructured the nesting hierarchy for drawables ( it was awfully bloated I've come to realize) and I need to know if I can do away with the mods box before I continue
That is definitely not the way to display selected mods. There's supposed to be a floating indicator on top of the button, but I haven't designed that yet.
Place the selected mod floating on top of the button for now, until I work on its fix.
That is definitely not the way to display selected mods. There's supposed to be a floating indicator on top of the button, but I haven't designed that yet.
Place the selected mod floating on top of the button for now, until I work on its fix.
addressed
Centering looks wrong.
Also please restore the height of the footer back to what it was. Even though this is a temporary change, it looks worse with it being taller in only this location:
Centering looks wrong.
Also please restore the height of the footer back to what it was. Even though this is a temporary change, it looks worse with it being taller in only this location:
Yeah im aware i didnt req a pr review yet because im trying to figure out how to use the overlaycolourprovider to feed the colours yet, sorry i shouldve perhaps mentioned that
and yep i was gonna handle the centering today
Lol
I don't even know what to do with this PR. I think it's probably blocked until @arflyte provides a proper design.
Lol
I don't even know what to do with this PR. I think it's probably blocked until @arflyte provides a proper design.
well i was told to put it up there in my defense i agree that that overlap sucks,
if possible I'd love a quick review of my latest two commits, think the structuring of the drawable is much better now and I removed all the margin jankiness i'd done due to lack of knowledge. Not pressing at all since this is blocked for now regardless, just would be neat.
shear centering should also be better
@mk56-spn there's been some design work on the mod display it would seem (https://www.figma.com/file/DXKwqZhD5yyb1igc3mKo1P/Song-Select-Draft-2?node-id=0%3A1&t=bRPeY78djO0JkLzS-0, bottom left)
That said, it's probably going to require a bit of implementation detail so I'd argue that the display should just be removed from this PR completely and added as a separate effort.
That said, it's probably going to require a bit of implementation detail so I'd argue that the display should just be removed from this PR completely and added as a separate effort.
Sounds good. Fwiw I'm willing to do this part as well. But yeah I'd rather get practice mode and the other bits I have in my pipeline out so I'll put this secondary effort on the backburner If thats ok
@arflyte still missing design
what do
i can make a semi fitting placeholder change for them if you want
in other words replace their current colours with B6/ B5 background colour, move the colour itself to a thin bar on one of the sides , add some corner radius rounding, a bit of spacing between them , make them less tall and shift them up so they dont collide with the new buttons.
and probably match the shear
I honestly think you should implement your new buttons in complete isolation. In a test scene. So you don't have to remove the multiplier display code and wait for all other design elements to be completed.
Trying to immediately slot this in amongst many other surrounding elements is, as we have seen from this PR, not a good idea.
You're welcome to suffix your component with a V2 as has been done elsewhere. Opening in a new PR is also fine if it cleans up the history.
yep ive upped my commit practices since making this and i totally agree this PR looks rather filthy, gonna close this in a bit in favour of a clean branch