Skip to content

Add Legends of the Jedi connection option#5306

Merged
vadi2 merged 1 commit intoMudlet:developmentfrom
rengawm:development
Jul 6, 2021
Merged

Add Legends of the Jedi connection option#5306
vadi2 merged 1 commit intoMudlet:developmentfrom
rengawm:development

Conversation

@rengawm
Copy link
Copy Markdown
Contributor

@rengawm rengawm commented Jul 4, 2021

Brief overview of PR changes/additions

Adds default connection info for Legends of the Jedi.

Motivation for adding to Mudlet

Add to the list of games offering a good experience in Mudlet and make it easier for players to find LotJ.

@rengawm rengawm requested a review from a team as a code owner July 4, 2021 15:45
@rengawm rengawm requested a review from a team July 4, 2021 15:45
@add-deployment-links
Copy link
Copy Markdown

add-deployment-links bot commented Jul 4, 2021

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

mudServer = QStringLiteral("Legends of the Jedi");
if (!deletedDefaultMuds.contains(mudServer)) {
pItem = new QListWidgetItem();
setupMudProfile(pItem, mudServer, getDescription(QStringLiteral("legendsofthejedi.com"), 0, mudServer), QStringLiteral(":/icons/legendsofthejedi_120x30.png"));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: Potential leak of memory pointed to by pItem [clang-analyzer-cplusplus.NewDeleteLeaks]

setupMudProfile(pItem, mudServer, getDescription(QStringLiteral("legendsofthejedi.com"), 0, mudServer), QStringLiteral(":/icons/legendsofthejedi_120x30.png"));
                                                         ^
../../Qt/5.14.1/gcc_64/include/QtCore/qstringliteral.h:66:6: note: expanded from macro 'QStringLiteral'
    ([]() noexcept -> QString { \
     ^
src/dlgConnectionProfiles.cpp:1705:9: note: Assuming 'success' is true
    if (!success) {
        ^
src/dlgConnectionProfiles.cpp:1705:5: note: Taking false branch
    if (!success) {
    ^
src/dlgConnectionProfiles.cpp:1710:5: note: Calling 'dlgConnectionProfiles::fillout_form'
    fillout_form();
    ^
src/dlgConnectionProfiles.cpp:1325:9: note: Assuming the condition is false
    if (mProfileList.isEmpty()) {
        ^
src/dlgConnectionProfiles.cpp:1325:5: note: Taking false branch
    if (mProfileList.isEmpty()) {
    ^
src/dlgConnectionProfiles.cpp:1355:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1355:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1361:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1361:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1367:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1367:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1373:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1373:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1379:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1379:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1385:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1385:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1391:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1391:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1397:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1397:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1403:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1403:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1409:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1409:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1415:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1415:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1421:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1421:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1427:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1427:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1433:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1433:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1439:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1439:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1445:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1445:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1451:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1451:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1457:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1457:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1463:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1463:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1469:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1469:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1476:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1476:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1482:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1482:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1488:9: note: Assuming the condition is false
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1488:5: note: Taking false branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1494:9: note: Assuming the condition is true
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1494:5: note: Taking true branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1495:17: note: Memory is allocated
        pItem = new QListWidgetItem();
                ^
src/dlgConnectionProfiles.cpp:1500:9: note: Assuming the condition is true
    if (!deletedDefaultMuds.contains(mudServer)) {
        ^
src/dlgConnectionProfiles.cpp:1500:5: note: Taking true branch
    if (!deletedDefaultMuds.contains(mudServer)) {
    ^
src/dlgConnectionProfiles.cpp:1502:58: note: Potential leak of memory pointed to by 'pItem'
        setupMudProfile(pItem, mudServer, getDescription(QStringLiteral("legendsofthejedi.com"), 0, mudServer), QStringLiteral(":/icons/legendsofthejedi_120x30.png"));
                                                         ^
../../Qt/5.14.1/gcc_64/include/QtCore/qstringliteral.h:66:6: note: expanded from macro 'QStringLiteral'
    ([]() noexcept -> QString { \
     ^

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This one seems to be a potential issue in existing code, nothing you added

@keneanung
Copy link
Copy Markdown
Member

The setup is pretty nice, congratulations 😄 I've got one complaint, but this is on us: Currently, the generic mapper is installed by default into the LOTJ profiles, which overlaps with the mapper in the UI. We should fix that before adding this.

if (!mudlet::self()->packagesToInstallList.contains(QStringLiteral(":/mudlet-mapper.xml"))) {
is the offending part, but I have no quick idea how to fix this...

@rengawm
Copy link
Copy Markdown
Contributor Author

rengawm commented Jul 4, 2021

Aha, I wondered about that. Would it make sense to just have our UI package call uninstallPackage on it since it's providing its own?

@keneanung
Copy link
Copy Markdown
Member

That might be a possible workaround, though messing with other installed packages on a profile irks me. I'd be happier if we found a solution for the actual problem. Thoughts @vadi2?

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Jul 4, 2021

Uninstalling the generic mapper is how it's done right now. Happy to rework the default packages code to support excluding the generic mapper.

@rengawm
Copy link
Copy Markdown
Contributor Author

rengawm commented Jul 4, 2021

For now, based on the docs referencing that practice for the generic mapper specifically, I just added that to our package.

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Jul 5, 2021

That's works well, but for future reference examples in specs are not normative!

Copy link
Copy Markdown
Member

@vadi2 vadi2 left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@vadi2 vadi2 merged commit 6506f0e into Mudlet:development Jul 6, 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

Development

Successfully merging this pull request may close these issues.

3 participants