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

Add shiny lock option to Event Manaphy research #414

Merged
merged 9 commits into from
Sep 15, 2024

Conversation

Real96
Copy link
Collaborator

@Real96 Real96 commented Aug 9, 2024

Add what asked in #413

@Real96 Real96 changed the title Add shiny lock option to Event Manapy research Add shiny lock option to Event Manaphy research Aug 9, 2024
@SteveCookTU
Copy link
Collaborator

Seeing as this is a static entry, I believe we should be following the same format as before and have it all contained within the templates rather than a check box just for a single Pokemon. If this means there are two static templates for locked vs non-locked then that should still be better.

@Real96
Copy link
Collaborator Author

Real96 commented Aug 14, 2024

Seeing as this is a static entry, I believe we should be following the same format as before and have it all contained within the templates rather than a check box just for a single Pokemon. If this means there are two static templates for locked vs non-locked then that should still be better.

Added the second template!

Copy link
Owner

Choose a reason for hiding this comment

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

I don't believe this file needs to have any changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes if we add the shiny combobox, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added combobox instead of checkbox

Comment on lines 362 to 370

if (staticTemplate->getShiny() == Shiny::Never)
{
ui->checkBoxSearcherShinyLock->setChecked(true);
}
else
{
ui->checkBoxSearcherShinyLock->setChecked(false);
}
Copy link
Owner

Choose a reason for hiding this comment

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

This should be able to be removed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Comment on lines 223 to 229

if (staticTemplate->getShiny() == Shiny::Never) {
ui->checkBoxGeneratorShinyLock->setChecked(true);
}
else {
ui->checkBoxGeneratorShinyLock->setChecked(false);
}
Copy link
Owner

Choose a reason for hiding this comment

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

This should be able to be removed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Comment on lines 186 to 199
ui->comboBoxGeneratorPokemon->addItem(
QString::fromStdString(Translator::getSpecie(templates[i].getSpecie(), templates[i].getForm())),
QVariant::fromValue(i));

if (templates[i].getShiny() == Shiny::Never)
{
ui->comboBoxGeneratorPokemon->addItem(
QString::fromStdString(Translator::getSpecie(templates[i].getSpecie(), templates[i].getForm()))
+ " (" + tr("Shiny Locked") + ")",
QVariant::fromValue(i));
}
else
{
ui->comboBoxGeneratorPokemon->addItem(
QString::fromStdString(Translator::getSpecie(templates[i].getSpecie(), templates[i].getForm())),
QVariant::fromValue(i));
}
Copy link
Owner

Choose a reason for hiding this comment

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

Do we want to do this or just add a shiny combobox that gets filled out by the template like we have in the other windows?

Copy link
Collaborator Author

@Real96 Real96 Sep 1, 2024

Choose a reason for hiding this comment

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

Can't we have both? Or you prefer having only one of the two?

Imo, i would keep both. But if you want only one of the two, the shiny combobox would be better because "Shiny Locked" text is being cut.

You mean this as combobox, right?
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Owner

Choose a reason for hiding this comment

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

I don't believe this file needs to have any changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Owner

Choose a reason for hiding this comment

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

I don't believe this file needs to have any changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Owner

Choose a reason for hiding this comment

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

I don't believe this file needs to have any changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Owner

Choose a reason for hiding this comment

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

I don't believe this file needs to have any changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Owner

Choose a reason for hiding this comment

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

I don't believe this file needs to have any changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Owner

Choose a reason for hiding this comment

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

I don't believe this file needs to have any changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@Admiral-Fish Admiral-Fish merged commit 183adbf into master Sep 15, 2024
3 checks passed
@Admiral-Fish Admiral-Fish deleted the manaphy_shiny_lock branch September 15, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants