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

Default database profiles #639

Merged
merged 16 commits into from
Jun 19, 2016
Merged

Conversation

Ebag333
Copy link
Contributor

@Ebag333 Ebag333 commented Jun 6, 2016

This pull is to specifically add database profiles, but it's setup so that we can add other things as we see fit.

Removed the hacky way of importing the default uniform resist profile (currently checks every time Pyfa is launched).

One change to consider would be merging the deadspace and asteroid NPCs, as they are fairly similar. As they are not exactly the same (and some differ quite a bit), I've kept them separate.

Data is scraped from Chukker's database, which is from 2015.

Target resist profile number is calculated off HP*resist, and doesn't account for shield VS armor tank, just total raw buffer. This gives NPCs slightly better numbers than if we accounted for just their "real" tank, but is probably more accurate to actual game play. Since NPC repping is spotty at best, this is probably as good as it gets.

Damage profiles are calculated off the damage done by that group, and does not account for weapon cycle times. Cycle times are not exposed on the main page (only on the detailed drill down), and scraping would be painful. Checking the numbers against a handful of NPCs, it seems to work out okay as CCP generally does high damage = high cycle time, low damage = low cycle time, so a NPC with multiple damage sources will end up with a fairly close representation of what the actual numbers are. Room for improvement, if we can figure out a better way to scrape Chukkers database.

Ebag333 added 8 commits May 16, 2016 07:55
Removed the kludgey method of using _init_ to check to see if the Uniform 25/25/25/25 damage pattern exists, and if missing create it.

Moved creation of it to prefetch.py.
Loads defaults for target resists and damage profiles....for now.

Can be extended to add anything we want on DB creation.  Can be extended to add a "load default value to database" button somewhere down the line, to support people who have existing databases and don't want to recreate them.
@blitzmann
Copy link
Collaborator

Two things (as we discussed on Slack):

  • Reimplement the default Uniform damage profile stuff
  • Fix the target resist values

Also, defaultDatabaseValues.importDefaults() errors out as defaultDatabaseValues isn't defined. ;)

Ebag333 added 3 commits June 12, 2016 17:24
Only ints are handled currently, if you throw a float at it, you'll get
a stacktrace.  This is just a little safety to convert it to an int, so
if we somehow get a float (like via database injection/modification), we
quietly convert it to the expected format
Redid all the values, especially updated target resist profiles.
Re-implemented the _init_ on service\damagePattern.py to check for
default values.
Allows for existing databases to be updated with the current default
profiles.  Can also be used to inject other defaults (maybe 0 and V
chars?)
@Ebag333
Copy link
Contributor Author

Ebag333 commented Jun 13, 2016

Changes you requested made.

Tested against:

  • No database. Creates new database an injects the default values.
  • Existing database with Uniform profile missing. injects the Uniform profile, doesn't do anything else.
  • Existing database with several custom profiles, and Uniform profile. Doesn't do anything.
  • Existing database with several custom profiles, and Uniform profile. Import Database Defaults is selected. Existing profiles aren't changed, default profiles are imported.

I think this is pretty good to go, unless anything jumps out at you.....

@Ebag333
Copy link
Contributor Author

Ebag333 commented Jun 13, 2016

Screenshot of the import defaults option. Maybe should be moved to the Help menu, since it's more of an override/migration/repair/etc?

http://puu.sh/prEJ3/a1f9bab6c4.png

@blitzmann
Copy link
Collaborator

Preliminary testing looks pretty good. I like that I can delete these and then reload them if needed, and that it doesn't mess with current databases unless requested by the user. I also like moving the Uniform DP addition to this new class - keeps all defaults contained, and I believe it's a great compromise with what we've discussed before.

Few notes, most of which are just pythonic changes:

for damageProfileRow in damageProfileList:
            damageProfile = eos.db.getDamagePattern(damageProfileRow[0])
            if damageProfile is None:
                damageProfile = eos.types.DamagePattern(damageProfileRow[1], damageProfileRow[2], damageProfileRow[3], damageProfileRow[4])
                damageProfile.name = damageProfileRow[0]
                eos.db.save(damageProfile)

Can be reduced to something like this:

for damageProfileRow in damageProfileList:
            name, em, therm, kin, exp = damageProfileRow
            damageProfile = eos.db.getDamagePattern(name)
            if damageProfile is None:
                damageProfile = eos.types.DamagePattern(em, therm, kin, exp)
                damageProfile.name = name
                eos.db.save(damageProfile)

This unpacks the list (which, semantically, should probably be a tuple instead of a list, but now I'm just nitpicking =P) into variable, reducing the amount of text on the screen while also being explicit in what they are (the fuck is damageProfileRow[2]? Unpack to the proper variable, and you instantly know it's the therm value).

  1. You should apply @classmethod to these functions. None of them require an instantiated class. So, when you import it, you can import with from eos.db.saveddata.loadDefaultDatabaseValues import defaultDatabaseValues, remove the class instantiation importDBDefaults = loadDefaultDatabaseValues.defaultDatabaseValues(), and simply call the methods directly with defaultDatabaseValues.importRequiredDefaults()

  2. Please rename defaultDatabaseValues to DefaultDatabaseValues. There's no denying we don't use pep8 convention in pyfa, but at least what we do use is more or less consistent. =)

  3. As for menu placement... I was envisioning something in the preferences screen, but that is a lot of work for little gain... I think I would like it better in the Help menu for now. It's not something that will be invoke often. If you could, stick it between About and Forums, and separate it with a list separator between both - I have been thinking about creating a batch of scripts that will help with database maintenance / clean up, and that will be a good place to stick them whenever I get around to it. =)

Thanks for your hard work! =D

@Ebag333
Copy link
Contributor Author

Ebag333 commented Jun 16, 2016

@blitzmann updated as per your feedback. I think I got all the changes mentioned, let me know if I missed something or if you have other suggestions.

I reran some quick tests (new DB, existing DB, reimporting multiple times, etc), haven't seen any issues.

I did merge in a bunch of stuff from Master, so a bunch of the "modified" files aren't really modified. Also, that cleared up the conflict (it conflicted with the change to move the turning overrides on).

class ImportError(Exception):
pass

class defaultDatabaseValues():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change case to DefaultDatabaseValues

@blitzmann
Copy link
Collaborator

blitzmann commented Jun 16, 2016

A few more comments. Also, not sure why the merge is causing the files to be changed - anything merged from master should be ignored in the merge compare. v0v

=)

@Ebag333
Copy link
Contributor Author

Ebag333 commented Jun 16, 2016

@blitzmann check now please. :)

The funny thing is that I even manually copied and pasted the entire file on a couple, and for some reason github still thinks it's different. Maybe a case of Linux/Unix whitespace vs Windows?

Anyway, let me know if you catch anything else....

@blitzmann blitzmann merged commit 0456973 into pyfa-org:master Jun 19, 2016
@Ebag333 Ebag333 deleted the default-database-profiles branch October 17, 2016 05:53
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.

2 participants