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

Cap/shield Simulation #812

Closed
wants to merge 26 commits into from
Closed

Conversation

Ebag333
Copy link
Contributor

@Ebag333 Ebag333 commented Nov 14, 2016

Use new module for cap/shield simulation.

Things left to be done.

  • Handle projected items on cap graph
  • Graph pane doesn't refresh when switching views. Need to add that. No plans to fix as a larger issue.
  • Switch cap stability numbers to new formula.

You can get the cap graph to show properly by opening the graph window, switching to capacitor, then dragging and dropping your fit in.

@Ebag333
Copy link
Contributor Author

Ebag333 commented Nov 14, 2016

So here's what the graph looks like:
https://puu.sh/shVqn/c2aa3d8b00.png

You can see that according to the graph (new formula) we cap ourselves out right around 120s. The old capsim says 55s.

Why the discrepancy? Why that big huge spike?

Well, that's the reload of the AAR. After 36s it goes into reload (60s), and then it kicks back on and drives us out of cap.

The current cap sim hardcodes everything to a reload of 10s. I'm pretty sure it also doesn't handle cooldowns properly (like MJD).

55+50 = 105s, which is pretty close to the 120s the new sim gives us.

@blitzmann
Copy link
Collaborator

55+50 = 105s, which is pretty close to the 120s the new sim gives us.

wat

120s is the new code, 55s is the old code, where does 105s come from?

Also, really liking this so far, and you even have a graph! <3

@Ebag333
Copy link
Contributor Author

Ebag333 commented Nov 14, 2016

120s is the new code, 55s is the old code, where does 105s come from?

Reload and passive regen.

Why the discrepancy? Why that big huge spike?

Well, that's the reload of the AAR. After 36s it goes into reload (60s), and then it kicks back on and drives us out of cap.

The current cap sim hardcodes everything to a reload of 10s. I'm pretty sure it also doesn't handle cooldowns properly (like MJD).

55+50 = 105s, which is pretty close to the 120s the new sim gives us.

To explain further.

The new sim has a time of 120s. The old sim had a time of 55s.

50 seconds of that can be explained by the reload time (since the new sim properly sets it at 60s instead of 10s).

Okay, so now we're down to 120-55-50 or 15s difference.

Where does that extra 15s come from?

The passive regen that occurs when the AAR is in reload. Basically while you're in reload you are net cap positive, and it's enough to build up buffer to last you an extra 15s.

@Ebag333
Copy link
Contributor Author

Ebag333 commented Nov 14, 2016

Also, really liking this so far, and you even have a graph! <3

Ugh, unfortunately the graph stuff doesn't do what we need it to. TL;DR: it's a bit garbage.

Whoever built it got it working for DPS, and stopped there. It's super hard coded for the values, and it's not self contained at all.

And of course doesn't refresh when you switch graphs....

There's a LOT of work to be done on graphs.

@Ebag333 Ebag333 mentioned this pull request Nov 15, 2016
5 tasks
@blitzmann
Copy link
Collaborator

yeah, graphs have always been a thing that need to be re-written. It should be a plugin system like the stats panel and whatnot - one that provides the UI portion as well as the data portion.

if you're gonna be fucking with graphs, though, check out https://github.com/mwaskom/seaborn been wanting to try this out to get prettier graphs (matplotlib vanilla is fucking ugly)

@Ebag333
Copy link
Contributor Author

Ebag333 commented Nov 15, 2016

All right, updated Gnosis to actually handle modules with a reload but no charges (think MJD, with reactivation delay playing the part of reload).

It also now handles passing a fire at percent value, or if my shield/cap is below this percentage, fire off this module. Super useful for armor/shield/cap boosters. Though of course until (if) we get a unified sim, can't exactly fire off the shield booster when your shield drops, so there is that. But at least now we can handle cap boosters better, and in a much more realistic fashion.

So now the sim handles a crapton of stuff that the GUI does not.

OOH Idea! 💡

For now we can just pass in the peak cap percent as the fire off below point (since, you know, we actually know what that percentage is now). Later when the GUI catches up, we can default to that, and let the user change it.

@Ebag333
Copy link
Contributor Author

Ebag333 commented Nov 17, 2016

All right, so at this point it's basically done.

This does change some things that folks may have base assumptions about. (Yeah yeah, I know you hate this part @blitzmann )

So after spending a good 30 minutes trying to explain what the capacitor + and - meant on Slack, I'm 100% convinced that folks don't fully understand it.

To briefly explain to anyone stumbling across this.
+ is your peak regen plus any modules which affect you positively.
- is any modules that affect you negatively.

It seems simple enough, but there's a number of problems with this including the fact that your peak regen changes as time goes by, while your module use doesn't (assuming you don't turn on/off any modules).

What I've done is rename the to fields, and remap them slightly.

+ is now Peak: and shows your peak regen (calculated off the capacitor size and regen).
- is now Used: and shows you the delta of modules (both local and projected).

So if you have a NOS at +4 GJ/s, and a Neut at -5 GJ/s, then you will see Used: -1 GJ/s. This also accounts for charges, reload time, and reactivation delays properly.

Oh, added bonus. Now has tooltips including showing you at what percentage your capacitor regen peaks. Used has a nice tooltip with an explanation of what it is. Much more useful than just showing the same number like it did before.

The look and feel of the cap stability % is essentially the same, but it's using the new sim.
(Which means that stability numbers can be drastically different.)

.

Okay, so now for the bad.

I broke the graph code migrating this over and adding extra functionality (probably from exposing what % you hit peak regen). The graph window is kinda garbage, I'll get the graph working correctly but I have zero plans at the moment on fixing the window. It needs a whole rewrite. Mostly what this means is if you switch to the capacitor graph, you need to trigger a refresh of the fit, so it's not a huge deal.

The other problem is I broke sustained shield/armor/hull boosting.

The main problem here is that the way in which it's calculated is pretty much a big hack. There isn't any way to really fix this cleanly, short of rewriting that (which is basically just writing a whole simulation).

What I would like to do is to get rid of the sustained numbers entirely, as they're fairly misleading to begin with. I would like to write a full fledged combat simulation which would be similar to the one I wrote for cap, and we would show sustained tank via a graph, similar to what we're doing for cap. Basically it's going to be impossible to show a "sustained tank" with a single number.

So. That's the state of this.

Oh, one other fairly major change.

addDrains() now stuffs the module (or drone, or whatever src is) that is triggering the effect into self.__extraDrains. This was far easier than extending self.__extraDrains with all the various things that we might need. It also meant that I didn't need to change the various effect files. I kept the existing amount/time/etc fields as it gives us a way to override those numbers from the module itself (like, for example, neut sig radius reduction and cap battery resist). If those numbers aren't passed in, we use the numbers from the source module/drone/fighter/etc.

Basically we handle getting all our info in gnosis.py. I wanted to try and keep one single place where we do all our mapping from the various fields that we store things in to the field that's required for Gnosis (since Gnosis doesn't know or care about Pyfa's data structure). I could have done it under saveddata.fit but...that file is already messy enough.

Let me know what you want to do with the sustained tank stuff. I believe @IndictionEve was wanting to expose the selected profile, this would be a great place to put that. :D

@Ebag333
Copy link
Contributor Author

Ebag333 commented Nov 17, 2016

Well, fixed the cap graph. That was easy.

Gotta track down a bug though. For some reason when heating modules it uses less cap, not more. Not sure why...

@Ebag333
Copy link
Contributor Author

Ebag333 commented Nov 18, 2016

Gotta track down a bug though. For some reason when heating modules it uses less cap, not more. Not sure why...

Issue fixed.

Turns out, actually putting them into the dict helps (we were only handling online modules previously).

@blitzmann this is basically done now, ball is in your court.

@Ebag333
Copy link
Contributor Author

Ebag333 commented Nov 18, 2016

Spoke too soon. Uncovered another issue.

This has to do with the way we're handling reactivation delays, and that to Python, False = 0. It'll take a fix in Gnosis itself, but I'll have it done shortly....

@Ebag333
Copy link
Contributor Author

Ebag333 commented Nov 18, 2016

Gnosis is updated with the fix (and now handles ReactivationDelay separately from Reload).

Various bugfixes in the cap code.

Think I squashed most of the bugs, but would be nice to get a second set of eyes on this.

@blitzmann blitzmann mentioned this pull request Nov 20, 2016
@Ebag333
Copy link
Contributor Author

Ebag333 commented Nov 28, 2016

Okay, I figured out how to do the whole "run the repper this much while staying cap stable".

It'll take more upgrades to the Gnosis module itself, as we have to capture when reppers fire (and by how much). Ugh. This capsim is quickly turning into a full fledged tick sim.

It'll take some assumptions on our part, but since there's already quite a few in play I don't think they're un-reasonable.

  1. We'll only fire off cap charges if we're below the cap stable %. While this isn't optimal in all scenarios, it's not a bad strategy and is how players tend to use the module (since most people fit the largest cap charges they can. It also IS optimal if you're using very large cap charges.
  2. For regular reppers, we'll add a reactivation delay to slow them down until we're cap stable.
  3. For ancil reppers, we'll treat them as regular reppers if they have no charges. If they have charges, we'll assume that the player wants to go through the reload cycle, so won't slow them down at all.

All these can be handled on the Pyfa side, those require no changes to Gnosis.

So all we need to do is add optional fields to pass in our dict for shield, armor, and hull.

@Ebag333
Copy link
Contributor Author

Ebag333 commented Nov 28, 2016

Gnosis bit is done. That was easy.

Figuring out how to get the remote modules passed into Gnosis will be a bit trickier.

@Ebag333
Copy link
Contributor Author

Ebag333 commented Nov 28, 2016

Well well well. Interesting what you come across when you start poking around.

From saveddata.fit:

    @property
    def tank(self):
        hps = {"passiveShield" : self.calculateShieldRecharge()}
        for type in ("shield", "armor", "hull"):
            hps["%sRepair" % type] = self.extraAttributes["%sRepair" % type]

        return hps

Turns out that this is using the base amounts. So for example, a small AAR on an unbonused ship with V skills and Improved Exile will give 78 HP for armorDamageAmount. If, however, you look at self.extraAttributes, you get 52.

Digging into it, I can see why.

self.extraAttributes is set when we run effects. For example

# shieldBoosting
#
# Used by:
# Modules from group: Shield Booster (93 of 93)
runTime = "late"
type = "active"
def handler(fit, module, context):
    amount = module.getModifiedItemAttr("shieldBonus")
    speed = module.getModifiedItemAttr("duration") / 1000.0
    fit.extraAttributes.increase("shieldRepair", amount / speed)

While this one is late, not all of them are. Plus, if we set something else to run late (like maybe something that modifies boost amount) there's nothing that forces them to run before this effect runs.

TL;DR: can't trust the extraAttributes. Awesome. This is probably a bigger issue with many things we set in extraAttributes and not just repairs.

@Ebag333
Copy link
Contributor Author

Ebag333 commented Nov 28, 2016

Sustainable tank is....mostly done.

Ended up just slowing down all modules that use cap evenly (that don't already have a reactivation delay), so this effects Ancils both with and without cap equally.

For a future version, it'd be nice to have a popup menu where you can select what to exclude from the slow down, but that's a lot more complicated.

Need to better handle situations where you'll never be cap stable no matter how much you slow down your modules. Right now it just bails after 1000 runs.

So, here's what it looks like. New sim on the left, old sim on the right. Ignore the slightly different cap used.

https://puu.sh/sx6Hx/5eea2fbd2f.png

As far as I can maths out, the numbers on the left are correct. I haven't walked through the HP to EHP conversion yet, so I can't guarantee that is correct, but the raw HP numbers are good.

I think the primary problem here is that the old formula didn't really handle reload and reactivation very well (at all?). Also, the theoretical maximum rep numbers are also wrong, since those don't handle the reload properly.

@blitzmann at this point it's basically ready to merge for the sim stuff. Everything works, and everything I've tested seems to be correct under the new sim.

There's some serious issues with the numbers in other areas (like the theoretical maximum, extraAttributes, etc). I'm not sure if you want me to tackle those in this PR, or just merge this one (once I do some house keeping on it) and handle those in a separate one.

To be clear, this PR touches (so far) :

Capacitor Pane:
Duration/Stability
+ (renamed to Peak)
- (renamed to Used)

Recharge Rates:
Sustained tank (bottom row) for peak shield regen, shield/armor/hull reps.

@Ebag333
Copy link
Contributor Author

Ebag333 commented Nov 28, 2016

For the peak shield regen, something weird is going on. The numbers for raw are correct, the numbers for effective are very wrong.

self.damagePattern.calculateEffectiveTank(self, self.sustainableTank)

self.sustainableTank is the correct (raw) number. The function returns a very wrong number. Need to figure out what's going on here...

And fixed my derp. I was pointing at the wrong attribute. :/

@Ebag333
Copy link
Contributor Author

Ebag333 commented Dec 2, 2016

@blitzmann might have a ringer that's looking into this (his idea, not mine). Would still like your feedback as to why it's setup this way, but don't go digging too deep into code yet... :)

@Ebag333
Copy link
Contributor Author

Ebag333 commented Dec 2, 2016

#869 fixes this! Yay!

Also, note to self. Add fitting engine preference to use fueled capacitor boosters only when below peak capacitor regen. (Translation, use boosters when below ~25%. This is the optimal strategy for boosters with max sized charges.)

@Ebag333
Copy link
Contributor Author

Ebag333 commented Dec 26, 2016

@blitzmann I moved Gnosis.py under Eos as requested.

Since it's been a few months now, and we've had exactly 2 comments on this thread (other than me) can we revisit merging this? :)

@blitzmann
Copy link
Collaborator

I plan to take a look at this in the near future. Currently bust with holidays and stuff (ie, steam winter sale lol)

This one will take a while obviously as its changing core features of the engine.

@Ebag333
Copy link
Contributor Author

Ebag333 commented Dec 27, 2016

ie, steam winter sale lol

Steam is the devil.

This one will take a while obviously as its changing core features of the engine.

I get it. Just realized that after the comment about looking into the import mess, never came back to this. So polished up what was there and wanted to leave a note that this is ready to merge.

I think at this point I'd like to get this merged in, and once the Preference Pane PR is merged we can extend the engine with more options (in a new PR).

Because they are special snowflakes.  Thanks CCP.
@ftsartek
Copy link
Contributor

ftsartek commented Feb 7, 2017

Again, very good idea, would be a great implementation. While it's not a vital addition it's a great feature that I'm sure would be very well received.

@Ebag333
Copy link
Contributor Author

Ebag333 commented Feb 8, 2017

@blitzmann let me know when you're ready to merge this. I can resubmit an up to date version of this since the code base has changed massively since this was done.

It's not the priority (finishing cleanup is), just wanted to make a note of this.

@Ebag333
Copy link
Contributor Author

Ebag333 commented Feb 9, 2017

So apparently Pyfa already supports cap stability ranges. It's just not used.
https://puu.sh/tUW3Y/c6786dd30e.png

Must. Implement. This.

@blitzmann
Copy link
Collaborator

@blitzmann let me know when you're ready to merge this. I can resubmit an up to date version of this since the code base has changed massively since this was done.

@Ebag333 gonna want to review this (and the book of comments that goes along with it) this week if you can get it up to date. Do you have a quick synopsis of what has been changed with the calcs / what is different / what we need to be aware of / quirks / whatever?

@Ebag333
Copy link
Contributor Author

Ebag333 commented Mar 12, 2017

Do you have a quick synopsis of what has been changed with the calcs / what is different / what we need to be aware of / quirks / whatever?

TL;DR: everything with cap changes. Neuts also change a lot.

Sustainable tank is completely rewritten.

Shield passive regen uses the new formula (since it's the same as cap).

I really need to come back and update this. Not only has the entire code base changed, but there's a lot of little things that have been cleaned up, like adding burst projectors cycle time to the rawCycle property. So that needs to be cleaned up.

Honestly the base code changed so much that it'll be waaaaay easier to create a new PR. This is pretty stand alone so it wouldn't be hard to do.

I think that we should figure out the build process though, so we can have a clean framework without hacking in the modules.

@blitzmann
Copy link
Collaborator

Honestly the base code changed so much

Sure the branch has changed a lot, but as you said this doesn't touch on a lot of things apart from cap sim.

I think that we should figure out the build process though, so we can have a clean framework without hacking in the modules.

Fair enough. I'll leave it up to you if you want to update this PR or open a new one. I don't mind 'hacking' in modules right now if this gets us some useful features (like graphs!) in case we can't get update builds in a timely fashion.

Just FYI, I had an issue with my new 10.9 VM that sort of trashed the image, so I need to start over with that.

I aslo assume, with the rewrite, you tested the new values with old values and they look more or less the same / more accurate? Any odd outliers?

@blitzmann blitzmann added the Blocked This issue is blocked by another issue, which must be solved first label Mar 13, 2017
@blitzmann
Copy link
Collaborator

@Ebag333 I labeled this as blocked since we're waiting on resolution of updated builds and general cleanup. Remove the label when needed, it's just a reminder to myself that it's not quite ready (and I think it's a good convention to start using)

@Ebag333
Copy link
Contributor Author

Ebag333 commented Mar 13, 2017

I aslo assume, with the rewrite, you tested the new values with old values and they look more or less the same / more accurate? Any odd outliers?

Well the numbers are quite a bit different, but not in ways that can't be explained.

No odd outliers that I saw. If anything, it removes outliers as we handle some things better than we did before (ancil primarily).

It is a significant change to the way cap and sustained tank is handled, but it's MUCH more auditable (when debugging) because we are getting a map back of the actual simulation, so you can see what exactly is happening.

Once we get it merged, I'd like to get a prerelease build out and let some beta testers use it. I think that it's a big enough change to warrant that.

I would also like to hear your thoughts on the GUI change, as that is quite large.

I labeled this as blocked since we're waiting on resolution of updated builds and general cleanup. Remove the label when needed, it's just a reminder to myself that it's not quite ready (and I think it's a good convention to start using)

Good idea.

@Ebag333
Copy link
Contributor Author

Ebag333 commented Mar 28, 2017

I labeled this as blocked since we're waiting on resolution of updated builds and general cleanup.

Since we keep touching things that deal with this (like your recent PR #1071), what do you think about going ahead with the merge on this?

My thought is, since doing a Windows build is NBD (it's Mac/Linux that's holding us up), we could easily do builds for Windows if we want things to be super clean, and just handle Mac as we always have.

If you're highly confident in being able to monkey patch Gnosis in, then might as well, eh?

@blitzmann
Copy link
Collaborator

blitzmann commented Mar 29, 2017

Sorry, must have missed the previous comments

Since we keep touching things that deal with this (like your recent PR #1071), what do you think about going ahead with the merge on this?

It really depends on how well I can get the os x things to work without a rebuild. It's just a python package, so I don't expect it to be very difficult (logbook was easy), unless your package uses some odd packages itself, or more recent ones that the os x build doesn't have. It would all depend.

Once we get it merged, I'd like to get a prerelease build out and let some beta testers use it. I think that it's a big enough change to warrant that.

I absolutely agree 100%. I think I even want to branch it to another branch outside of dev just in case something is truly wrong with the calcs that can't be fixed by the time we roll out another release. I doubt that would be the case, but I think this scope of this change warrants it.

I would also like to hear your thoughts on the GUI change, as that is quite large.

I was under the assumption these were simulation changes and just tweaking the GUI to interact with it. I wasn't aware of actual GUI changes. Will have to look.

You said you had some clean up to do, is that still the case? This is a large PR (in the sense of impact rather than lines changed) - when I start review I don't want to hit a moving target. :)

@Ebag333 Ebag333 closed this May 3, 2017
@Ebag333 Ebag333 deleted the Cap/Shield_PeakRegen branch May 3, 2017 14:19
@Ebag333 Ebag333 restored the Cap/Shield_PeakRegen branch May 3, 2017 14:22
@Ebag333 Ebag333 reopened this May 3, 2017
@Ebag333
Copy link
Contributor Author

Ebag333 commented Jan 8, 2018

A year come and gone. RIP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked This issue is blocked by another issue, which must be solved first
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants