pyfa-org / Pyfa

Python fitting assistant, cross-platform fitting tool for EVE Online
GNU General Public License v3.0
1.6k stars 406 forks source link

Ctrl-dragging (copying) module with 'Max Groups Allowed' results in error #1691

Closed Sadario closed 6 years ago

Sadario commented 6 years ago

Bug Report

If trying to copy a module that has attribute "Max Modules Of This Group Allowed = 1", by holding ctrl and dragging the module to a new slot, Pyfa encounters an error.

Expected behavior:

Expecting Pyfa to inform me that the module can't be copied, or that the module is not copied at all.

Actual behavior:

Pyfa prints out a stacktrace.

Detailed steps to reproduce:

Any fitting, new or existing, add a module with attribute 'Max Modules...' set to 1/true (Damage Control, Small Ancillary (Remote) Armor Repairer, etc), then hold ctrl, and drag/drop on to new slot.

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

[Velator, BugTest]

Damage Control II [Empty Low slot]

[Empty Med slot] [Empty Med slot]

Small Ancillary Remote Armor Repairer [Empty High slot]

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

622a3004c51a017d604265bab3ce27885069d530

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

Windows 10

Other relevant information:

OS version: Windows-10-10.0.14393-SP0
Python version: 3.7.0 (v3.7.0:1bf9cc5093, Jun 27 2018, 04:59:51) [MSC v.1914 64 bit (AMD64)]
wxPython version: 4.0.3 (wxWidgets 3.0.5)
SQLAlchemy version: 1.2.10
Logbook version: 1.4.0
Requests version: 2.19.1
Dateutil version: 2.7.3

####################

Traceback (most recent call last):
  File "C:\Users\XWOLNIK\Documents\Git\Pyfa\gui\builtinViews\fittingView.py", line 126, in OnData
    self.dropFn(x, y, data)
  File "C:\Users\XWOLNIK\Documents\Git\Pyfa\gui\builtinViews\fittingView.py", line 215, in handleListDrag
    self.swapItems(x, y, int(data[1]))
  File "C:\Users\XWOLNIK\Documents\Git\Pyfa\gui\builtinViews\fittingView.py", line 481, in swapItems
    sFit.cloneModule(self.mainFrame.getActiveFit(), srcIdx, mod2.modPosition)
  File "C:\Users\XWOLNIK\Documents\Git\Pyfa\service\fit.py", line 761, in cloneModule
    if new.fits(fit):
  File "C:\Users\XWOLNIK\Documents\Git\Pyfa\eos\saveddata\module.py", line 468, in fits
    fits = self.__fitRestrictions(fit, hardpointLimit)
  File "C:\Users\XWOLNIK\Documents\Git\Pyfa\eos\saveddata\module.py", line 530, in __fitRestrictions
    self.modPosition != mod.modPosition):
  File "C:\Users\XWOLNIK\Documents\Git\Pyfa\eos\saveddata\module.py", line 252, in modPosition
    return self.owner.modules.index(self)
ValueError: Module(ID=41476, name=Small Ancillary Remote Armor Repairer) at 0x17eaa99b320 is not in list
blitzmann commented 6 years ago

Seems to be a bug that was introduced somewhere between 2.1 and 2.3.1 (the two versions I have on my laptop).

Thanks for the bug report! I am currently looking into undo/redo functionality, which actually touches and refactors most of the related code, so I'm not 100% sure if I'm going to focus on this bug just yet. We'll see how far along the new features come before I take a closer look at this (though patches are always welcome if someone finds a fix) :)

feyyd commented 6 years ago

pull request #1701 should fix this, there was no check for maxGroupFitted. added the check and it swaps instead of cloning. I wasn't sure what other items can have maxGroupFitted, but i think the check I added covers all use cases (??) tested for 5-10 minutes and couldn't get it to break but I didn't know which modules even use the maxGroupFitted attribute so I'm sure I missed coverage.