Projected Fit infinite recursion

[dachucky]

If you project fit A onto fit B, then project fit B onto fit A, this will cause the fits to be recursively projected onto each other infinitely eventually hitting Python's recursion limit. This will break both fits until one is deleted, as well as any fit that either fit is projected onto.

Best way to try this is to create two new blank fittings and project each one onto the other.

[blitzmann]

Confirmed, thanks! I've also confirmed that any recursion chain regardless of depth is affected (ie, not just the reported A -> B -> A but also A -> B -> C -> D -> A

I'm amazed this hasn't been brought up before.

[blitzmann]

I haven't looked at the code yet, but I do have an idea as to how we may be able to solve this. These are my ramblings so that I remember them and discussion may start.

We might be able to simply break the recursion by introducing the concept of copying a projected fit internally to a new object, and using that object as the first-degree projection. This way, instead of a bidirectional projection A <-> B, we have a uni direction projection with A <- B'.

So if you have A <-> B, then we take a copy of B and assign it as B'. B' will have the modified attributes of it's projected fits (A, which is NOT prime), which will project onto A.

However, this might not answer the question of what happens if you then take A and project it onto another fit. Since we take the prime of only the first degree (in this case A), we have pretty much the same problem where A <-> B.

Maybe we should take the prime of the entire projection chain, but that is something I haven't even considered.

Again, this is all theory crafting, and I'm not sure how well it will play out in the end.

I fucking hate recursions.

[blitzmann]

Actually, is recursion even necessary?

General question: are there any modules that can be projected onto a fit that modify the attribute of another projection-enabled module?

Certainly, tracking links help with the Tracking of the targets guns, but does nothing for the targets own tracking links. Hence, recursion isn't needed...

I'll have to look into this. =)

EDIT: after speaking with a buddy, these indeed seems to be the case. I will poke @DarkFenX for his opinion. Also have to determine if breaking projection recursion also breaks boosters. If you have A boosting B, and B projected onto C, we want to make sure that B still gets his boosts before applying modified attributes to C. IIRC, the code isn't related, just need to double check

[dachucky]

Boosters should be coming in via fleet, not projected fits anyway. Haven't looked at whether having someone as a fleet booster in the chain breaks anything, so that might not be an issue anyway.

[blitzmann]

I was able to fix this with this commit: blitzmann@73795d3

However, I was just fiddling with the code without any real understanding of it. Not too familiar with chain and the code behind this bit. It fixes the problem of recursion, but introduces another problem: projected fits give different stats. Without this commit, I have a Scythe projected onto an Oracle (both fits from #63) which gives 606 HP/s. With this commit, that changes to 984.8 HP/s.

Will continue looking into this...

[blitzmann]

Just pushed another commit that seems to work as expected: blitzmann@3afdae1

So, the reason this issue happens in the first place is (because I love being verbose and documenty):

When a fit is recalculated, Pyfa clears the fit of previous values and basically starts a new slate. In te process of clearing everything, it also clears data for it's modules, implants, etc. And projected fits. Now, wgen projected fits get's the .clear() command, it uses the same clear function as the original fit, and it clears it's projected fits, which can cause the loop. So Pyfa tries to continue clearing them over and over.

Latest commit simply adds a flag to turn on/off clearing projected fits. If its the main fit, we run through the projected fits and clear them, sending the flag to turn off clearing their projected fits. I've talked with a few other people, and it does seem that projected fits have no effect at all on other projections, so this should be a safe operation.

It also shows the correct projected stats on my previous test fits. Will create a pull request for @DarkFenX to review.

[StinGer-ShoGuN]

I mentioned a similar problem on the former PyFa website (evefit.org): you could project fit onto itself and that would cause Python to crash. Solution applied was to forbid self projection.
So now when I want to create a self projection, I just copy the fit and project the copy onto the original fit.

However, if it happens that I delete the copy fit without removing it from the projected items in the original fit, Pyfa randomly crashes. So is this case, if you do something like A -> B -> C -> D -> A and you remove a fit in the chain without properly "cleaning" the chain, I'd expect Pyfa to randomly crash as well.

[blitzmann]

Self projection is still forbidden. However, you bring up a good point: deleting a fit that is projected will not remove it from the victim ships. This happens on the current master branch, so this fix doesn't seem to fix or make better this particular issue. This doesn't seem to crash pyfa immediately, but might result in a crash since pyfa isn't expecting this (I have had it crash while trying to duplicate a fit with projected fits, with the error stating non-unique primary key). I will look into scrubbing the projected fits table when a fit is removed.

I do believe the fix for this issue (#78) should still move forward as it doesn't seem to apply to this new issue, and continues to prevent fit recursion which always leads to a crash.

As stated in my other comment on #84, I'll look into getting a build together with these changes.

[blitzmann]

After discussing proposed fixed with Kadesh, it seems that it doesn't work. While #78 prevents Pyfa from crashing from run away recursion, it allows us to see that Pyfa does not calculate projected fits correctly when there is a loop. Weather or not it's because of proposed fix, or if proposed fix simply unmasks issue due to fixing crash, is unknown.

Will test out a few fits over the next few days and record my findings and post here for documentation.

[blitzmann]

I'm going to make another serious effort on this in addition to #83

The main points to keep in mind are that there should only be 1 degree of projection. If we have A > B > C, we should only calculate B along with it's ganglinks and apply them to A. `C does not need to be calculated, although it currently is. I believe this will also make it possible to project a fit onto itself, but that is not a priority.

I think the previous fix might help out. I think the reason it gave weird stats was because we did not limit the recursion to 1 degree and thus it kept applying attributes to weird locations. Again, will have to look more into this.

[blitzmann]

Should be fixed with the merge e74e261