Skip to content
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

GUI: Add an icon grid to the launcher display #3135

Draft
wants to merge 194 commits into
base: master
Choose a base branch
from

Conversation

@av-dx
Copy link
Contributor

@av-dx av-dx commented Jul 7, 2021

This is part of the GSoC '21 project : "GUI for Game Library Management"

Add a grid of game icons to the launcher as an alternative display to the existing list display of games. A toggle-able grid view, using game icons from the https://github.com/scummvm/scummvm-icons repo and flag icons from https://flagpedia.net. Clicking on a game brings up a tray of the common game-related tasks.

A screenshot of the grid:
grid_screenshot

av-dx added 30 commits May 22, 2021
@somaen somaen self-assigned this Jul 28, 2021
gui/ThemeEngine.cpp Outdated Show resolved Hide resolved
gui/launcher.cpp Outdated Show resolved Hide resolved

#ifndef DISABLE_LAUNCHERDISPLAY_GRID
ButtonWidget *_listButton;
ButtonWidget *_gridButton;

/**
* Create two buttons to switch between grid display and list display

This comment has been minimized.

@somaen

somaen Jul 28, 2021
Member

This is currently confusing, as this talks about "switch", but the function name is "chooser", and the function AFTER that is "createSwitchButton", so which does what here?

This comment has been minimized.

@av-dx

av-dx Jul 29, 2021
Author Contributor

createSwitchButton() makes one pic button which does the specified cmd on being clicked, while the addChooserButtons() calls createSwitchButton() twice to make two buttons, one for list and one for grid.
So shall I rename addChooserButtons() to addSwitchButtons() ? but then createSwitchButton and addSwitchButtons look very similar.

void GroupedListWidget::setAttributeValues(const U32StringArray &attrValues) {
_attributeValues = attrValues;
if (!attrValues.empty())
assert(_attributeValues.size() == _dataList.size());

This comment has been minimized.

@somaen

somaen Jul 28, 2021
Member

This needs a comment explaining the requirement.

if (_dataList.size() == _listColors.size()) {
// If the color list has the size of the data list, we append the color.
_listColors.push_back(color);
} else if (!_listColors.size() && color != ThemeEngine::kFontColorNormal) {

This comment has been minimized.

@somaen

somaen Jul 28, 2021
Member

"_listColors.size() == 0" is clearer IMHO, as we aren't checking a boolean, but an actual value. May be a matter of taste though. This could perhaps be written in terms of empty() instead?

gui/widgets/groupedlist.cpp Outdated Show resolved Hide resolved
void GroupedListWidget::toggleGroup(int groupID) {
// Shrink group if it is expanded
if (_groupExpanded[groupID]) {
_groupExpanded[groupID] = false;

This comment has been minimized.

@somaen

somaen Jul 28, 2021
Member

This can also be written as simply

_groupExpanded[groupId] = !_groupExpanded[groupId]
sortGroups()

Which is perhaps slightly less readable, so a matter of taste.

This comment has been minimized.

@av-dx

av-dx Jul 29, 2021
Author Contributor

I think I was earlier performing checks to see if the selection went out of bounds on shrinking a group so that I could move the selection to something else, so I made the two branches. However, it didn't work out too well, so now I just set the selection to -1, whenever a group header is interacted with. I may try to figure out the earlier mentioned problem later though, so I will keep the branches as is.

gui/widgets/groupedlist.cpp Outdated Show resolved Hide resolved
} else {
buffer = _list[pos];
}
if (isGroupHeader(_listIndex[pos]))

This comment has been minimized.

@somaen

somaen Jul 28, 2021
Member

Consider assigning the style-choice to a variable, so that we can use a single call to drawText.

gui/widgets/groupedlist.h Outdated Show resolved Hide resolved
@av-dx av-dx force-pushed the av-dx:grid-launcher-test branch from 0d31aa8 to 915d669 Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants