Disable/Enable fitting restrictions

[SubjectXXX]

Bug Report

Modules get unfit on re-enabling fitting restrictions

Expected behavior:

Nothing happens

Actual behavior:

First Rocket launcher and Assault Damage Control II gets removed

Detailed steps to reproduce:

Load this fit. Disable fitting restrictions, enable them right back.

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

[Jaguar, SB]

Navy Micro Auxiliary Power Core
Assault Damage Control II
Caldari Navy Power Diagnostic System

Pithum B-Type Medium Shield Booster
Thukker Medium Cap Battery
Initiated Compact Warp Scrambler
1MN Y-S8 Compact Afterburner
[Empty Med slot]

Rocket Launcher II, Nova Rage Rocket
Rocket Launcher II, Nova Rage Rocket
Rocket Launcher II, Nova Rage Rocket
[Empty High slot]

Small Anti-Kinetic Screen Reinforcer II
Small Ancillary Current Router II

Warrior II x2

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

1.36.0

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

win10

Other relevant information:

[blitzmann]

Thanks for the bug report! Indeed, this is no expected behavior. I'll take a look into why this might be happening!

[WatchMeCalculate]

Initial investigation of at least the rocket launcher disappearance. This occurs for all shipTypes as well as modules being Turret or Missile. Basically, when you toggle the function it triggers sevice\fit.py to this code block:

 if not fit.ignoreRestrictions:
        for m in fit.modules:
            if not m.isEmpty and not m.fits(fit):
                self.removeModule(fit.ID, m.modPosition)

In eos\savedata\module.py as it's validating each module on the first Hardpoint module it finds the hardpoint check fails

 if fit.ship.getModifiedItemAttr('launcherSlotsLeft') - fit.getHardpointsUsed(Hardpoint.MISSILE) < 1:

The values for for this example would be launcherSlotsLeft=3 and HardpointsUsed=3, which evaluates to 0 < 1 which is true. Causing that module to be deleted, and the rest of the hardpoint modules to pass the check with launcherSlotsLeft being 3 and HardpointsUsed being 2.

[blitzmann]

Thanks for looking into this @WatchMeCalculate! That gives me a bit more information on where it might be breaking, I'll try to look deeper into it later (unless you can figure out a fix for it :)) Seems to me like it's a logic bug

[WatchMeCalculate]

the fits function returns an answer to if a module can be fitted to an existing fit. So it treats a hardpoint module as if it is about to fit. Without changing what module.fits is doing, a potential solution would be to put hardpoint limit logic up into toggleRestrictionIgnore, after ensuring every module is a valid type, ensure that the hardpoint limits are valid.

if not fit.ignoreRestrictions:
    hardpointModules = []
    for m in fit.modules:
        if not m.isEmpty and not m.fits(fit, False):
            self.removeModule(fit.ID, m.modPosition)
        elif m.hardpoint:
            hardpointModules.append(m)
        for m in hardpointModules:
            if fit.getHardpointsFree(m.hardpoint) < 0:
                self.removeModule(fit.ID, m.modPosition)

[WatchMeCalculate]

More formalized:
Hardpoints Used refactor
Change to ignoreRestriction Logic

[WatchMeCalculate]

The other option would be to just set hardpoint to false and not remove them if there are more launchers than allowed as the UI gives adequate feedback to show youre violating the restriction. Same feedback when you have too many active drones

[WatchMeCalculate]

I think the second option would be cleaner solution. User would have the feedback to remove the launcher, also with removing hardpoint without some logic rework it would always delete whatever module is in the highest position regardless of which modules were added in what order.

if not fit.ignoreRestrictions:
     for m in fit.modules:
-       if not m.isEmpty and not m.fits(fit):
+       if not m.isEmpty and not m.fits(fit, False):
            self.removeModule(fit.ID, m.modPosition)

[WatchMeCalculate]

Continuing looking into this issue. For the Assault Damage control. The reason why this is being deleted is in the same vein as the hardpoint cause. since module.fits is a function that determines if a module can be fit to the ship, the logic doesn't quite handle the validity of an already fit ship. The Assault Damage control is a special case as it has a maxGroupFitted value (so all items that have this value are affected) and the logic in savedata\module.pyL#468 will evaluate itself and will fail

max = self.getModifiedItemAttr("maxGroupFitted", None)
 if max is not None:
    current = 0  # if self.owner != fit else -1  # Disabled, see #1278
    for mod in fit.modules:
        if mod.item and mod.item.groupID == self.item.groupID:
            current += 1
    if current >= max:
        return False

If there was an extra check to ensure that mod.modPosition != self.modPosition, it would effectively handle this case.

[WatchMeCalculate]

Found another issue while working on this where modules never get recolored after they are deemed invalid. #1626

This illustrates what is happening before my changes

If you attempt to remove hardpoints down to 4/4 but leave the red colored hardpoint it won't get recolored.

Now:

This example doesn't show it, but if the first valid assault damage control gets deleted and the second one remains that was colored red, when restrictions are re-enabled, it correctly will recolor

[blitzmann]

Thanks so much to @WatchMeCalculate for pushing the fix for this! :D

Summary of the problem: the method we use to determine if a module fits or not is usually used before we fit the modules, so various counts are proper. We kind of hijack this function for when the fit restriction application turns back on, but at that time everything is already fit so various counts don't work.

The solution implemented is good for now, though I don't doubt there's probably some edge cases that might creep up.