Skip to content

Factor out drawing map information label into a method#2857

Merged
vadi2 merged 1 commit intoMudlet:developmentfrom
vadi2:factor-out-map-info
Jul 25, 2019
Merged

Factor out drawing map information label into a method#2857
vadi2 merged 1 commit intoMudlet:developmentfrom
vadi2:factor-out-map-info

Conversation

@vadi2
Copy link
Copy Markdown
Member

@vadi2 vadi2 commented Jul 25, 2019

Brief overview of PR changes/additions

No functional changes.

Just create a separate T2DMap::paintMapInfo method so paintEvent isn't one giant mess and really hard to work with.

This makes it much easier with profiling because we can now see how much painting the map info took in the overall picture:

perf data - Hotspot_270

And we can see how within the method we spend the time:

perf data - Hotspot_271

The overhead of calling the function is negligible when Qt's basic primitive paint functions take most of the time spent.

(credit to hotspot for profling)

Motivation for adding to Mudlet

Mapper code less scary, so more people are inclined to work with it! :)

Other info (issues closed, discussion etc)

@vadi2 vadi2 added this to the 4.0.0 milestone Jul 25, 2019
@vadi2 vadi2 requested a review from a team as a code owner July 25, 2019 07:23
@vadi2 vadi2 requested a review from a team July 25, 2019 07:23
@add-deployment-links
Copy link
Copy Markdown

add-deployment-links bot commented Jul 25, 2019

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.

@vadi2
Copy link
Copy Markdown
Member Author

vadi2 commented Jul 25, 2019

Yep it's still a pretty big function, I agree with CodeFactor, but I don't want to do too many changes at once.

Copy link
Copy Markdown
Contributor

@Kebap Kebap left a comment

Choose a reason for hiding this comment

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

Refactor I approve

@vadi2 vadi2 merged commit d7e0a84 into Mudlet:development Jul 25, 2019
@vadi2 vadi2 deleted the factor-out-map-info branch July 25, 2019 08:33
} else {
areaExit = false;
}
areaExit = pE->getArea() != mAreaID ? true : false;
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.

💡 No need for that ternary operator - the following is sufficient:

areaExit = (pE->getArea() != mAreaID);

Copy link
Copy Markdown
Member

@SlySven SlySven left a comment

Choose a reason for hiding this comment

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

I guess this is not as big a change as I at first worried it might be - and as it factors out a chunk of the massive T2DMap::paintEvent(...) it is worthwhile. 👍

@vadi2
Copy link
Copy Markdown
Member Author

vadi2 commented Jul 25, 2019

Glad to hear you like it! I'd like to do a few more chunks like this out of it too - so it is easier to work with.

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