Factor in reload time when calculating capacitor usage, damage and tank appears inaccurate for tank.

[Neugeniko]

Bug Report

Expected behavior:

When factoring reload time of the saar in this fit the sustained armor repair is
(34.7 HP/sec peak x 36sec (time to run out of paste)/(36 + 60(reload time))= 13.0125 HP/sec

Actual behavior:

When factoring reload time of the saar in this fit the sustained armor repair is 27.4 HP/S

Detailed steps to reproduce:

Fits involved in EFT format (Edit > To Clipboard > EFT):

[Rifter, AC SP WEB MA AB ALT 8 - 1295 OMNI + 88 + 147*]

200mm Steel Plates II
Energized Adaptive Nano Membrane II
Small Ancillary Armor Repairer, Nanite Repair Paste
IFFA Compact Damage Control

1MN Afterburner II
Fleeting Compact Stasis Webifier
J5b Enduring Warp Scrambler

200mm AutoCannon II, Fusion S
200mm AutoCannon II, Fusion S
200mm AutoCannon II, Fusion S

Small Projectile Burst Aerator II
Small Anti-Explosive Pump I
Small Anti-Kinetic Pump I

Republic Fleet Phased Plasma S x360
Nanite Repair Paste x8

Release or development git branch? Please note the release version or commit hash:

py3

Operating system and version (eg: Windows 10, OS X 10.9, OS X 10.11, Ubuntu 16.10):

Win 10

Other relevant information:

[blitzmann]

Duplicate #1401, this was a recently discovered problem and is a regression due to some refactoring that was done a while back. It's my intention to address in the next week or so :)

[Neugeniko]

The cap calculation takes into account the reload of the saar in this case but doesn't calculate the proper sustained hp/sec due to reload. With a asb the cap doesnt matter and the sustained hp/sec is just determined by 'duty cycle' of asb, currently the asbs are bugged and show reinforced=sustained hp/sec.

[blitzmann]

Reopening until this can be processed a bit better, might be a different issue

[Neugeniko]

Could I get a pointer to where armor rep is calculated in pyfa?

[Neugeniko]

To start I don't know if this relevant but may be beginning. In fit.py -

1147 cycleTime = mod.getModifiedItemAttr("duration")

should that be cycleTime = mod.cycleTime like in the line 1163 since they both operate on sustainable and factor in reload makes a difference between these two. Also reinforced=sustained armor rep for the above fit with these changes, the expected starting point before duty cycle calculation.

[Ebag333]

Cap is actually wrong across the board. Current formula used is fairly close, but it isn't very accurate in a number of situations.

2 years ago a project was started to fix the problem, it was completed over a year ago but never merged.

It fixes this issue, incorrect passive shield regen, and a bunch of other incorrectly calculated stuff.

#812

[blitzmann]

should that be cycleTime = mod.cycleTime like in the line 1163 since they both operate on sustainable and factor in reload makes a difference between these two.

That very well may be the case, I don't think I'll have the time today to look into it fully, but let me know if you find anything interesting.

... but never merged. It fixes this issue, incorrect passive shield regen, and a bunch of other incorrectly calculated stuff.

IIRC, you're the one that closed the PR with some open questions still surrounding it. 😛 Regardless, as I mentioned in another issue, it is on my radar to re-address this in the coming months as I think it could be really useful and provide some cool features. However my priority focus right now is to get the kinks worked out of our current codebase. While the cap code may not be 100% accurate right now, it's still accurate for the most part, and if we can find some easy wins with the current system, that would be preferable right now rather than to completely usurp the existing code and releasing that of which I still don't fully understand. Not to mention there's still a laundry list of things todo right now 😃

[Ebag333]

I refactored the code 3 different times so you could push button merge it. You never did. I even had a branch with it working (more than one, actually).

So yes, after sitting open with nothing happening on it for a year, I closed the PR.

[blitzmann]

I refactored the code 3 different times so you could push button merge it. You never did. I even had a branch with it working (more than one, actually).

You full well know it wasn't that it couldn't be merged easily. Not only was it a large change with a lot of commentary, as previously stated elsewhere, there were issues with including the code with the mac bundle I believe, which is why it had the PyInstaller milestone on it. I'm not going hash through it again, just know that it's on my radar and will be addressed when it comes time, hopefully in the next few months :)

Thanks

[Ebag333]

No, that was the pyinstaller stuff. The capsim could have been merged on its own.

Last time we spoke on it, you said you wouldn't merge it until you understood it.

Now almost a year and a half later :

I still don't fully understand.

[blitzmann]

No, that was the pyinstaller stuff. The capsim could have been merged on its own.

Apologies, I misremembered, there wasn't a PyInstaller milestone on it, but was labeled as blocked because builds had to be modified, and IIRC there was a problem with including it into the mac bundle - the monkey patching didn't work for some reason. I never delved down into the full understanding of it, no, but that was because I couldn't even get it built properly, so was kind of moot. I was holding off on it until we changed build processes, which has finally been prompted. I was also waiting on you for a few things, at least according to this comment: #812 (comment)

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. :)

And then nothing. ¯_(ツ)_/¯ We can sit here and argue all day as to why shit happened or didn't happen over a year ago, but I don't really feel like further explaining my decisions. I have told you these facts:

1. It didn't get merged because I had difficulties with getting it incorporated with the mac build as well as I thought I was waiting on something from you.
2. I am still interested in evaluating it further and including it at a later date when some other higher priority things have finished, because I really, honestly believe it includes some cool features and fixes some other jankiness with current capsim.

I'm not going hash through it again, just know that it's on my radar and will be addressed when it comes time, hopefully in the next few months :)

Don't know what else you want me to say other than if this conversation continues to devolve in it's relatedness from the topic, I will lock the thread for a while. :)