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

Remove __downloadPreviewFromWeb from maps.preview #507

Merged
merged 2 commits into from
Nov 27, 2016

Conversation

duk3luk3
Copy link
Member

@duk3luk3 duk3luk3 commented Nov 20, 2016

All map preview calls already use the downloadManager class if
maps.preview doesn't find a preview icon. The __downloadPreviewFromWeb
call is entirely unnecessary and slows down the client because it is
synchronous.

Fixes #506

  • PR branch should be named issuenum-fix/feature/cleanup-description
  • Code is split into logical commits
  • Code has tests
  • Final commit includes "Fixes #issue" in commit message

When all builds pass and a maintainer is happy with your PR, the "ready" label will be applied. Please complete these tasks then:

  • Rebase onto develop
  • Add changelog entry
  • Remove this entire template section

@duk3luk3 duk3luk3 added this to the 0.12 milestone Nov 20, 2016
@duk3luk3 duk3luk3 self-assigned this Nov 20, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 21.754% when pulling 43c086b on duk3luk3:506-fix-webpreview-async into 11c5a61 on FAForever:develop.

@duk3luk3
Copy link
Member Author

This needs review and testing to make sure I haven't missed anything.

@duk3luk3 duk3luk3 mentioned this pull request Nov 20, 2016
@duk3luk3
Copy link
Member Author

@Brutus5000 @muellni can one of you have a look over this please?

@muellni
Copy link
Contributor

muellni commented Nov 24, 2016

I'm not very familiar with the client, but I ran the client on this branch and spotted no problems in map previews.

All map preview calls already use the downloadManager class if
maps.preview doesn't find a preview icon. The `__downloadPreviewFromWeb`
call is entirely unnecessary and slows down the client because it is
synchronous.

Fixes FAForever#506
@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 21.805% when pulling dd852c7 on duk3luk3:506-fix-webpreview-async into 6c716e3 on FAForever:develop.

@duk3luk3 duk3luk3 merged commit dd852c7 into FAForever:develop Nov 27, 2016
@duk3luk3 duk3luk3 deleted the 506-fix-webpreview-async branch November 27, 2016 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Client loads web previews on startup
4 participants