-
Notifications
You must be signed in to change notification settings - Fork 16
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
History develop #168
base: develop
Are you sure you want to change the base?
History develop #168
Conversation
Moved the Load last game and favorites with menu into the branch
Both history and last game works for roms and 64dd games
Giving a bit more space between the lines of history
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Removing the empty entry that snuck in
Fixing up ordering
Moved add to favorite to the options context menu
refactored most of the history / favorite code to now be "bookkeeping". this now uses a common struct to hold the required paths and the record type (to open up future work for emus).
Users can remove favorites via the menu
Updated the tabs so they fill the top of the screen.
src/menu/views/load_rom.c
Outdated
@@ -199,6 +205,7 @@ static component_context_menu_t options_context_menu = { .list = { | |||
{ .text = "Set Save Type", .submenu = &set_save_type_context_menu }, | |||
{ .text = "Set TV Type", .submenu = &set_tv_type_context_menu }, | |||
{ .text = "Set ROM to autoload", .action = set_autoload_type }, | |||
{ .text = "Add To Favorite", .action = add_favorite }, |
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.
{ .text = "Add To Favorite", .action = add_favorite }, | |
{ .text = "Add to favorites", .action = add_favorite }, |
@@ -23,6 +23,7 @@ | |||
#define MENU_DIRECTORY "/menu" | |||
#define MENU_SETTINGS_FILE "config.ini" | |||
#define MENU_CUSTOM_FONT_FILE "custom.font64" | |||
#define MENU_HISTORY_FILE "history.ini" |
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.
given this is menu related, and particular reason why you should not just use the current config.ini
with a seperate header?
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.
At first I felt it was better to have them in their own file out of the way of the config.ini
that way I could load and save as needed.
also makes it easier to wipe in a single go.
src/menu/ui_components/tabs.c
Outdated
|
||
static const char* tabs[3] = | ||
{ | ||
"Browser", |
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.
"Browser", | |
"Files", |
Or something like "File Explorer"
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.
Perhaps also consider which should be the default Tab... since favorite might be perefereable (also consider as a config option).
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.
a config option for what tab it brings up first could be an idea.
but I'd leave it with the file browser as the default, it feels what people would expect.
src/menu/views/browser.c
Outdated
static void draw (menu_t *menu, surface_t *d) { | ||
rdpq_attach(d, NULL); | ||
|
||
ui_components_background_draw(); | ||
|
||
ui_components_layout_draw(); | ||
ui_components_layout_draw(); |
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.
ui_components_layout_draw(); | |
ui_components_layout_draw(); |
src/menu/views/load_rom.c
Outdated
@@ -272,7 +279,7 @@ static void draw (menu_t *menu, surface_t *d) { | |||
ALIGN_RIGHT, VALIGN_TOP, | |||
"L|Z: Extra Info\n" | |||
"R: Options" | |||
); | |||
); |
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.
); | |
); |
@@ -22,6 +22,8 @@ static void actions_clear (menu_t *menu) { | |||
menu->actions.options = false; | |||
menu->actions.settings = false; | |||
menu->actions.lz_context = false; | |||
menu->actions.previous_tab = false; |
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.
Why not just use the dpad / joystick e.g.
menu->actions.go_left = false;
menu->actions.go_right = false;
as already defined?
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.
Left, Right on the D-Pad feels more accident prone, I'd personally want something more specific for the user to press like the C buttons.
I still think that left and right on the dpad should be the fast movement.
name = path_last_get(menu->load.rom_path); | ||
} | ||
|
||
menu->load.load_favorite = -1; |
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.
something here in my loins says for readability/maintainability it should be something like menu->bookkeeping.get_favorite = -1;
and probably the whole function above...
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 did have these in the bookkeeping struct before, but figured it might be better in the load with the other variables.
but fine with where ever.
All comments are for discussion... not a review... |
Contains the code required for logging the last game and the start of a favourites system
Description
When you launch a game, either rom or disk.
the file path for those will be written out to an ini file.
so the user can hit the "last game" button (current C-Left) while browsing and it will take them to either the rom or disk info screen.
with the game already selected.
Also there is a favorite system where the user can press the favorite button (C-Right) on the rom / disk info screen.
and it will be saved to a favorite list.
the user can hit the favorite button while on the browser and be taken to a screen to select their favorites.
Motivation and Context
Issue request for last game and favorites.
How Has This Been Tested?
Run on Supercart 64,
Tested last loading just a rom, just a disk and a disk and rom
Tested History, loading rom, disk and rom and disk.
Screenshots
Types of changes
Checklist:
Signed-off-by: GITHUB_USER <GITHUB_USER_EMAIL>