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

GBA: Add "Scan e-Reader Card(s)" context menu item #9938

Merged
merged 1 commit into from Jul 23, 2021

Conversation

Pokechu22
Copy link
Contributor

See issue 12588. This doesn't introduce any more complicated functionality; it only exposes the functionality that was already available via drag and drop in a more discoverable way.

I haven't tested this at all.

@JMC47
Copy link
Contributor

JMC47 commented Jul 22, 2021

I guess I'll have to test this lol

Copy link
Contributor

@JMC47 JMC47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was able to load e-Reader cards via the context menu and unlock the content.

Source/Core/DolphinQt/GBAWidget.cpp Outdated Show resolved Hide resolved
Source/Core/DolphinQt/GBAWidget.cpp Outdated Show resolved Hide resolved
@@ -281,6 +298,10 @@ void GBAWidget::contextMenuEvent(QContextMenuEvent* event)
unload_action->setEnabled(CanControlCore() && !m_game_title.empty());
connect(unload_action, &QAction::triggered, this, &GBAWidget::UnloadROM);

auto* card_action = new QAction(tr("&Load e-Reader Card"), menu);
card_action->setEnabled(CanControlCore() && !m_game_title.empty());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
card_action->setEnabled(CanControlCore() && !m_game_title.empty());
card_action->setEnabled(CanControlCore());

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you need to load the e-Reader ROM first? Or does that happen automatically?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e-Reader itself has to be loaded in order to scan cards. It's like a normal game in how it is loaded in mGBA.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, then I think it makes sense to still require a non-empty game title if you can't load a card when the e-Reader itself is loaded. (It'd be better to check if it's the e-Reader itself that's loaded, but I'm not sure how to do that.)

@@ -281,6 +298,10 @@ void GBAWidget::contextMenuEvent(QContextMenuEvent* event)
unload_action->setEnabled(CanControlCore() && !m_game_title.empty());
connect(unload_action, &QAction::triggered, this, &GBAWidget::UnloadROM);

auto* card_action = new QAction(tr("&Load e-Reader Card"), menu);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on changing this to &Scan e-Reader Card?

Copy link
Contributor

@JMC47 JMC47 Jul 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard agree. Because most e-Reader cards are in pairs, you have to scan two at once. So you scan one, hit a button, scan another.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've implemented loading multiple cards at once. Would it make sense to change it to &Scan e-Reader Card(s), or is just &Scan e-Reader Card fine?

@JMC47
Copy link
Contributor

JMC47 commented Jul 22, 2021

Should the Pull Request's name be changed then?

@Pokechu22 Pokechu22 changed the title GBA: Add "Load e-Reader Card" context menu item GBA: Add "Scan e-Reader Card(s)" context menu item Jul 22, 2021
@lioncash lioncash merged commit 4f87821 into dolphin-emu:master Jul 23, 2021
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants