Set hasWidth to true when assigned value to Width

cool-pants commented 4 years ago

Description

Changed the hasWidth parameter value allocation to set it to true after it is assigned a value.

Motivation and Context

Before if hasWidth was set false and hasRight had value true and hasLeft be true then the value returned by the GetDimension function would be false which should be actually true since a value has been indeed assigned to width.

How Has This Been Tested?

Tested by comparing data obtained from GetDimensions() method before and after method in log. The data was tested over four separate skins, namely:

Estuary
Estouchy
Amber
Confluence

Screenshots (if appropriate):

Types of change

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] Clean up (non-breaking change which removes non-working, unmaintained functionality)
[x] Improvement (non-breaking change which improves existing functionality)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that will cause existing functionality to change)
[ ] Cosmetic change (non-breaking change that doesn't touch code)
[ ] None of the above (please explain below)

Checklist:

[X] My code follows the Code Guidelines of this project
[ ] My change requires a change to the documentation, either Doxygen or wiki
[ ] I have updated the documentation accordingly
[X] I have read the Contributing document
[ ] I have added tests to cover my change
[ ] All new and existing tests passed

garbear commented 4 years ago

The explanation makes sense. I'd be interested to see what problems this causes in the GUI, by demonstrating a problem that is fixed by this change.

Two code comments:

Assignment on separate lines, for git history
Indentation (watch those tabs), for readability

eigendude commented 4 years ago

Every student's right of passage is learning how to make badass PRs. Always proofread your work. You can see the effect of all commits squashed in the Files tab: https://github.com/garbear/xbmc/pull/116/files. In this case there's still a minor readability infraction.

The "How Has This Been Tested?" is blank. This is what I was getting at earlier. How can we test your change?

eigendude commented 4 years ago

Sorry work account from my phone

cool-pants commented 4 years ago

Can you please clarify what the infraction is, I still can't get it. Should I have aligned the = for both assignments?

Also I am still in process of building the code so I will update the description today.

garbear commented 4 years ago

awkward indentation

cool-pants commented 4 years ago

Huh it shows perfectly fine on my device.

garbear commented 4 years ago

That's because you haven't configured your IDE to show leading and trailing whitespace. See the tab?

Leading whitespace

cool-pants commented 4 years ago

Thanks for the help. I fixed it now hopefully.

garbear commented 4 years ago

Looks good now. With some test data we can upstream.

garbear commented 4 years ago

This is a valid code path optimization. Merging to include in test build

garbear / xbmc