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

Re-introduce Command Fit State #1074

Merged
merged 2 commits into from
Apr 2, 2017

Conversation

blitzmann
Copy link
Collaborator

Simple QoL improvement / a fix for a regression dating back to November.

Bit of background: I disabled the state column for the Command / Fleet view when we made the change over to the new command burst mechanics. The reason behind this is because the State column, which controls which image is show to represent state of item, only received stuff. Normally we check what stuff is, and figure out what to do from there (eg: if stuff is a module, check the modules state and display an image that works).

The problem originally with command fits is that it shares the same stuff type as projected fits. So there wasn't a way to differentiate a projected fit from a command fit in the State class.

But holy shit, this was super easy after taking a look at it again tonight. We can't differentiate on stuff, but what we do have is access to the calling view. So to re-enable the State column for command view, we check to see if we're calling from command or projected, then get it's information, then display based on active flag.

@codecov-io
Copy link

codecov-io commented Mar 27, 2017

Codecov Report

Merging #1074 into development will decrease coverage by <.01%.
The diff coverage is 0%.

@@               Coverage Diff               @@
##           development    #1074      +/-   ##
===============================================
- Coverage        10.19%   10.19%   -0.01%     
===============================================
  Files             2139     2139              
  Lines            31402    31404       +2     
===============================================
  Hits              3203     3203              
- Misses           28199    28201       +2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 878d9d7...ce9615b. Read the comment docs.

if projectionInfo is None:
# can't use isinstance here due to being prevented from importing CommandView.
# So we do the next best thing and compare Name of class.
if type(self.fittingView).__name__ == "CommandView":
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use:
self.fittingView.__class__.__name__

Not sure it's any faster though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

True, I'll change is as it's not a function call. Thanks!

return -1
if projectionInfo.active:
if info.active:
Copy link
Contributor

Choose a reason for hiding this comment

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

info is used all over the place (281 occurrences). I liked projectionInfo better. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well it's no longer projection info. It can be command info now, so it needs to be more generic. It's a variable scoped to this function, so there shouldn't be any issue in determining the meaning of it.

@blitzmann blitzmann merged commit f08a669 into pyfa-org:development Apr 2, 2017
@blitzmann blitzmann deleted the command-fit-state branch April 14, 2017 04:12
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