-
Notifications
You must be signed in to change notification settings - Fork 25
CHLCC: Menus Polish #365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
CHLCC: Menus Polish #365
Conversation
58069c9 to
7702594
Compare
7702594 to
ebd724c
Compare
ebd724c to
619487d
Compare
619487d to
62a0164
Compare
62a0164 to
fd5dda1
Compare
Sound: - Add scrollbar support - Add click support for switching playback mode - Fix misaligned elements and animations Tips: - Start with right page - Fix sprites bounds System Messages: - Fix 1px sprite gap for a button Sound, Movie, CG: - Add "Select ***" animation based on the Tip's menu "Select Word" animation Sound, Movie, CG, Save/Load: - Simplify sprite loading for "Select ***" in .lua Tips, Save/Load, Title, Sound, Movie, CG: - Use one implementation for all "Select ***" animation (via composition) Backlog, Clear, Tips, Save/Load, Title, Sound, Movie, CG, Options, Trophy: - Use one implementation for "Show Menu" animation taken from Options menu (via composition) Bonus: - Shave off 16 bytes of every Animation (Widget has Animation field) - Shave off 8 bytes of every Menu
fd5dda1 to
6a771bf
Compare
Enorovan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Select Animations sure are better and the missing inputs in Sound Menu work
KKhanhH
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the behavioral changes for the music menu scrolling? Do cross input interactions work well, like scrollbar partial scroll then switch to kb/gamepad, backing out and in from a partial scroll, scroll hold, etc
src/ui/widgets/scrollbar.h
Outdated
|
|
||
| class Scrollbar : public Widget { | ||
| public: | ||
| Scrollbar() {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do = default
| inline uint8_t ButterflyFrameCount; | ||
| inline float ButterflyFlapFrameDuration; | ||
| inline float ButterflyFadeDuration; | ||
| inline float MenuSelectPromptDuration; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These would be much better suited in a common menu profile file but thats not in yet so I'll let it slide
| class SelectPromptAnimation : public Animation { | ||
| public: | ||
| SelectPromptAnimation(); | ||
| void Draw(const std::span<const Sprite> sprites, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dont make function arguments value const in header files
src/profile/games/chlcc/tipsmenu.cpp
Outdated
| SelectWordPos = GetMemberVector<glm::vec2>("SelectWordPos"); | ||
| SelectWordDuration = EnsureGetMember<float>("SelectWordDuration"); | ||
| SelectWordInterval = EnsureGetMember<float>("SelectWordInterval"); | ||
| assert(SelectWordSprites.size() == SelectWordPos.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we throw an exception or something if this asserts false, its in the init stages anyway so might as well catch it rather than bug out in releases
| SelectMenuHeader = GetMemberVector<Sprite>("SelectMenuSprites"); | ||
| SelectMenuHeaderPositions = | ||
| GetMemberVector<glm::vec2>("SelectMenuTextPositions"); | ||
| assert(SelectMenuHeader.size() == SelectMenuHeaderPositions.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
| GetMemberArray<Sprite>(SelectMovie, 11, "SelectMovie"); | ||
| GetMemberArray<glm::vec2>(SelectMoviePos, 11, "SelectMoviePos"); | ||
| SelectMovie = GetMemberVector<Sprite>("SelectMovie"); | ||
| SelectMoviePos = GetMemberVector<glm::vec2>("SelectMoviePos"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one doesn't have a check
| GetMemberArray<Sprite>(SelectData, 10, "SelectData"); | ||
| GetMemberArray<glm::vec2>(SelectDataPos, 10, "SelectDataPos"); | ||
| SelectDataSprites = GetMemberVector<Sprite>("SelectDataSprites"); | ||
| SelectDataPos = GetMemberVector<glm::vec2>("SelectDataPos"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one also has no check
src/games/chlcc/musicmenu.cpp
Outdated
| using namespace Impacto::Vm::Interface; | ||
|
|
||
| constexpr bool isRepeatOne(MusicPlaybackMode playbackMode) { | ||
| return to_underlying(playbackMode) & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could overload operators for & and ^, then wouldn't need a specialty function or spam to_underlying everywhere
src/games/chlcc/musicmenu.cpp
Outdated
| MenuTransition.SetDuration(MenuTransitionDuration); | ||
| void MusicMenu::ToggleRepeatMode() { | ||
| PlaybackMode = | ||
| (MusicPlaybackMode)(to_underlying(PlaybackMode) ^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does ^= work?
|
|
||
| // adjusts sprite height to prevent visual bug tied to mouse support (1px of | ||
| // out of bounds highlight sprite can be visible) | ||
| TrackHighlight.Bounds = TrackHighlight.Bounds.ScaleAroundCenter( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This modifies a profile sprite, can you instead modify the trackbutton code?
5c7c532 to
4846e24
Compare
CHLCC: Menus Polish
Sound:
Tips:
Sound, Movie, CG:
Sound, Movie, CG, Save/Load:
Tips, Save/Load, Title, Sound, Movie, CG:
Backlog, Clear, Tips, Save/Load, Title, Sound, Movie, CG, Options, Trophy:
Bonus: