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

Cleanup visibility #85

Merged
merged 4 commits into from
Jun 15, 2016
Merged

Cleanup visibility #85

merged 4 commits into from
Jun 15, 2016

Conversation

peppsac
Copy link
Contributor

@peppsac peppsac commented Jun 15, 2016

The point of this PR is to improve the readability of the code handling tile visibility.
It contains some of the ideas from @Jeremy-Gaillard 's PR #71 and should help fixing bug #82

@gchoqueux could you review this PR?

Introduces setVisible/setDisplayed to remove the ambiguity between
using node.visible (is the node visible?) and node.material.visible (is
this node displayed).
Also limits the number of places modifying visibility/displayed properties
to help understand where/why these are changed.
Simplify BrowseTree logic to improve readability:
  - use 'action' parameter instead of several booleans
  - introduce variable with descriptive names (disposableChildrenCount,
    isVisible, wasVisible, etc)
@gchoqueux gchoqueux merged commit f38e28c into master Jun 15, 2016
gchoqueux added a commit that referenced this pull request Jun 15, 2016
gchoqueux added a commit that referenced this pull request Jun 15, 2016
@gchoqueux
Copy link
Contributor

Thx, good job

@gchoqueux gchoqueux deleted the cleanup_visibility branch June 15, 2016 09:00
NikoSaul pushed a commit to NikoSaul/itowns2 that referenced this pull request Jul 4, 2017
NikoSaul pushed a commit to NikoSaul/itowns2 that referenced this pull request Jul 4, 2017
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.

None yet

2 participants