pyfa-org / Pyfa

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

Abyssal module custom attrs support via import/export #1707

Closed DarkFenX closed 5 years ago

DarkFenX commented 6 years ago

This one is VERY inconvenient because you have to pass abyssal stats to your friend along with the fit. I am mostly thinking about EFT-alike export-import because it's the one i see used the most often. Before actually implementing anything, wanted to share the idea and get feedback from interested parties.

For example, following format:

<abyssal module name>
<abyssal extended data prefix><base module name>
<abyssal extended data prefix><mutaplasmid name>
<abyssal extended data prefix><comma-separated list of attributes and values (alphabet-sorted?)>

abyssal extended data prefix could be 2-4 spaces or some other way to visually separate/indent it, which is easy to parse and which doesn't produce visual clutter.

Abyssal 1MN Afterburner
  Gistii A-Type 1MN Afterburner
  Gravid 1MN Afterburner MutaPlasmid
  capacitorNeed 21.3, cpu 16.2, speedFactor 179, power 13.5

On UI export side, we could have single EFT radiobutton with two additional ticks below it (which become active when you select EFT format): export abyssal module stats and export implants (which should memorize their last used values, otherwise they will be very inconvenient).

Possible adjustments could be considered to compact it: 1) merge 1st and 2nd extended row (via comma or some other separator) 2) remove everything but mutaplasmid grade from mutaplasmid name

So with both applied, result would be:

Abyssal 1MN Afterburner
  Unstable Gistii A-Type 1MN Afterburner
  capacitorNeed 21.3, cpu 16.2, speedFactor 179, power 13.5

But it would need smarter parser, as you will have to find appropriate mutaplasmid for specified typename based on partial mutaplasmid name.

DarkFenX commented 6 years ago

I was thinking about what kind of information does mutated module name carry compared to base module name, and it's just the fact that module has been modified (while losing some info like exact base module type). So, i think that the best condensed format would be:

<base module name>
<abyssal extended data prefix><mutaplasmid grade name>: <comma-separated list of attributes and values (alphabet-sorted?)>

Or, as example:

Gistii A-Type 1MN Afterburner
  Unstable: capacitorNeed 21.3, cpu 16.2, speedFactor 179, power 13.5

Biggest downside, as with any condensed form presented here, we have to find mutaplasmid using just base item name and mutaplasmid grade, but on the bright side it's very compact and clean. If we decide to stick to this format, two questions arise:

1) i have no idea how hard is it to get mutaplasmid from grade and base item name. Worst case we will have to create map {(base metagroup item, grade name): mutaplasmid} and maintain it manually. While we can do it ourselves as it's low-maintenance, if we expect other tools to support this format (eve client, if ccp ever gets to it), can we expect other developers to do it as well? 2) When exporting fits w/o mutated stats, do we export mutated item name (which points to item's metagroup and shows that it is modified) or base item name (exactly specifies type but omits that it has been mutated)? I feel that neither option is good, but we have to stick to one of these. Considering that mutated item name cannot be imported w/o additional data, i think that i prefer to just use base item name instead (and this will also be consistent with the extended format i just mentioned).

DarkFenX commented 6 years ago

So i implemented exporting in latest git (w/o UI side), result is:

[Tiamat, Shield]

Syndicate Damage Control
Entropic Radiation Sink II
Entropic Radiation Sink II
Entropic Radiation Sink II
Tobias' Modified Tracking Enhancer

Gistum A-Type 50MN Microwarpdrive
  unstable: capacitorNeed 108, cpu 40, power 150.4, signatureRadiusBonus 273, speedFactor 569.8
Thukker Large Shield Extender
  unstable: capacityBonus 3575, cpu 32, power 124, signatureRadiusAdd 10.5
Pithum A-Type Adaptive Invulnerability Field
X-Large Ancillary Shield Booster, Navy Cap Booster 400
  unstable: capacitorNeed 792, cpu 160, duration 4000, power 400, reloadTime 42000, shieldBonus 1176
Dark Blood Small Capacitor Booster, Navy Cap Booster 400

Heavy Entropic Disintegrator II, Mystic M
'Smokescreen' Covert Ops Cloaking Device II
Medium 'Ditch' Energy Neutralizer
Corpii A-Type Small Energy Neutralizer
Corpii A-Type Small Energy Neutralizer

Medium Polycarbon Engine Housing II
Medium Polycarbon Engine Housing II

High-grade Snake Alpha
High-grade Snake Beta
High-grade Snake Gamma
High-grade Snake Delta
High-grade Snake Epsilon
High-grade Snake Omega
Zainou 'Gnome' Shield Management SM-706
Zor's Custom Navigation Hyper-Link
Inherent Implants 'Lancer' Gunnery RF-906
Zainou 'Gnome' Weapon Upgrades WU-1006

Strong Frentix Booster

Mystic M x1000
Occult M x1000

Doesn't look very pretty, but I don't know how to make it better.

Square brackets:

Gistum A-Type 50MN Microwarpdrive
  [unstable: capacitorNeed 108, cpu 40, power 150.4, signatureRadiusBonus 273, speedFactor 569.8]
Thukker Large Shield Extender
  [unstable: capacityBonus 3575, cpu 32, power 124, signatureRadiusAdd 10.5]
Pithum A-Type Adaptive Invulnerability Field
X-Large Ancillary Shield Booster, Navy Cap Booster 400
  [unstable: capacitorNeed 792, cpu 160, duration 4000, power 400, reloadTime 42000, shieldBonus 1176]
Dark Blood Small Capacitor Booster, Navy Cap Booster 400

Square brackets + on the same line:

Gistum A-Type 50MN Microwarpdrive  [unstable: capacitorNeed 108, cpu 40, power 150.4, signatureRadiusBonus 273, speedFactor 569.8]
Thukker Large Shield Extender  [unstable: capacityBonus 3575, cpu 32, power 124, signatureRadiusAdd 10.5]
Pithum A-Type Adaptive Invulnerability Field
X-Large Ancillary Shield Booster, Navy Cap Booster 400  [unstable: capacitorNeed 792, cpu 160, duration 4000, power 400, reloadTime 42000, shieldBonus 1176]
Dark Blood Small Capacitor Booster, Navy Cap Booster 400
blitzmann commented 6 years ago

Thanks for creating this ticket, I've been wanting to create a discussion about this since it was released. Agree that we should have an extended EFT format to handle this, and I like the idea of having checkboxes for the various options that we may select.

i have no idea how hard is it to get mutaplasmid from grade and base item name.

I don't believe it would be difficult with current items, though I can't check right now. We do similar things for things like wormhole effect projection selector. However, if CCP ever changes the format of these mutaplasmids, we may have to change the format the we use, which may potentially break other programs. That being said, I think it would be pretty rare for something like that to happen.

Gistii A-Type 1MN Afterburner
  Unstable: capacitorNeed 21.3, cpu 16.2, speedFactor 179, power 13.5

I like this, as it is easy on the eyes, and gives us all the information we would need (of course, would need to test the grade name portion). CCP currently exports Abyssal modules as Abyssal Webifier or whatever, and I agree that on import we should simply ignore these (which I think is current mechanism) since we don't have enough information to make it work. I'm not sure if they would be open to changing this format, as I think it might break a lot of other programs if they aren't aware of the format change.

I'll ping Larrikin to see if he, or anyone at CCP, wants to give a bit of insight or opinion into this

blitzmann commented 6 years ago

lol just saw the newest post. I think it looks as good as it will ever look. Unfortunately, the eft format doesn't really give a lot of room for additions.

Would another possibility be not inline with the fit, but instead another block of text at the end? Kind of like:

[Name]
Mod 1
Abyssal Stasis Webifier
...

Abyssal Stasis Webifier; Unstable; attr1: 3.0, attr 2: 4.0

This might be better for supporting other applications that may not expect additional information inline, and will also keep the fact that the abyssal module is still listed as a module, with the extra information appended.

Literally gave that like 2 seconds of thought, so... might not even work well

DarkFenX commented 6 years ago

This will not work when you have several mutated mods based off the same item type, e.g. 2 abyssal webs based on fednavy web or 2 abyssal reps based on centus x-type rep etc. To work around it, i think you have to have change EFT format somehow: give some kind of identifier for these modules and then specify stats below main fit section.

[Tiamat, Shield]

Syndicate Damage Control
Entropic Radiation Sink II
Entropic Radiation Sink II
Entropic Radiation Sink II
Tobias' Modified Tracking Enhancer

Abyssal 50MN Microwarpdrive [1]
Abyssal Large Shield Extender [2]
Pithum A-Type Adaptive Invulnerability Field
Abyssal X-Large Ancillary Shield Booster, Navy Cap Booster 400 [3]
Dark Blood Small Capacitor Booster, Navy Cap Booster 400

Heavy Entropic Disintegrator II, Mystic M
'Smokescreen' Covert Ops Cloaking Device II
Medium 'Ditch' Energy Neutralizer
Corpii A-Type Small Energy Neutralizer
Corpii A-Type Small Energy Neutralizer

Medium Polycarbon Engine Housing II
Medium Polycarbon Engine Housing II

High-grade Snake Alpha
High-grade Snake Beta
High-grade Snake Gamma
High-grade Snake Delta
High-grade Snake Epsilon
High-grade Snake Omega
Zainou 'Gnome' Shield Management SM-706
Zor's Custom Navigation Hyper-Link
Inherent Implants 'Lancer' Gunnery RF-906
Zainou 'Gnome' Weapon Upgrades WU-1006

Strong Frentix Booster

Mystic M x1000
Occult M x1000

[1] Gistum A-Type 50MN Microwarpdrive  (unstable: capacitorNeed 108, cpu 40, power 150.4, signatureRadiusBonus 273, speedFactor 569.8)
[2] Thukker Large Shield Extender (unstable: capacityBonus 3575, cpu 32, power 124, signatureRadiusAdd 10.5)
[3] X-Large Ancillary Shield Booster (unstable: capacitorNeed 792, cpu 160, duration 4000, power 400, reloadTime 42000, shieldBonus 1176)

So we have to break EFT format one way or another, and have to choose between these variants.

blitzmann commented 6 years ago

Ah, true true. Yeah, in that case might as well do it in the way it makes sense and put it inline

DarkFenX commented 6 years ago

Well, both formats have their advantages. 1st one puts mutated stats into fit itself - so they are easier to find when you need them, but produce visual clutter. Way with references is reverse: visual clutter is minimal but if you want to see abyssal stats, you have to see completely other part of fitting. Maybe doing them inline is better?

I am going to repost all the variants I generated so far (so it's easier to check them in full context and discuss).

1) Separate line, minimal:

[Tiamat, Shield]

Syndicate Damage Control
Entropic Radiation Sink II
Entropic Radiation Sink II
Entropic Radiation Sink II
Tobias' Modified Tracking Enhancer

Gistum A-Type 50MN Microwarpdrive
  unstable: capacitorNeed 108, cpu 40, power 150.4, signatureRadiusBonus 273, speedFactor 569.8
Thukker Large Shield Extender
  unstable: capacityBonus 3575, cpu 32, power 124, signatureRadiusAdd 10.5
Pithum A-Type Adaptive Invulnerability Field
X-Large Ancillary Shield Booster, Navy Cap Booster 400
  unstable: capacitorNeed 792, cpu 160, duration 4000, power 400, reloadTime 42000, shieldBonus 1176
Dark Blood Small Capacitor Booster, Navy Cap Booster 400

Heavy Entropic Disintegrator II, Mystic M
'Smokescreen' Covert Ops Cloaking Device II
Medium 'Ditch' Energy Neutralizer
Corpii A-Type Small Energy Neutralizer
Corpii A-Type Small Energy Neutralizer

Medium Polycarbon Engine Housing II
Medium Polycarbon Engine Housing II

High-grade Snake Alpha
High-grade Snake Beta
High-grade Snake Gamma
High-grade Snake Delta
High-grade Snake Epsilon
High-grade Snake Omega
Zainou 'Gnome' Shield Management SM-706
Zor's Custom Navigation Hyper-Link
Inherent Implants 'Lancer' Gunnery RF-906
Zainou 'Gnome' Weapon Upgrades WU-1006

Strong Frentix Booster

Mystic M x1000
Occult M x1000

2) Separate line, square brackets (for visual separation):

[Tiamat, Shield]

Syndicate Damage Control
Entropic Radiation Sink II
Entropic Radiation Sink II
Entropic Radiation Sink II
Tobias' Modified Tracking Enhancer

Gistum A-Type 50MN Microwarpdrive
  [unstable: capacitorNeed 108, cpu 40, power 150.4, signatureRadiusBonus 273, speedFactor 569.8]
Thukker Large Shield Extender
  [unstable: capacityBonus 3575, cpu 32, power 124, signatureRadiusAdd 10.5]
Pithum A-Type Adaptive Invulnerability Field
X-Large Ancillary Shield Booster, Navy Cap Booster 400
  [unstable: capacitorNeed 792, cpu 160, duration 4000, power 400, reloadTime 42000, shieldBonus 1176]
Dark Blood Small Capacitor Booster, Navy Cap Booster 400

Heavy Entropic Disintegrator II, Mystic M
'Smokescreen' Covert Ops Cloaking Device II
Medium 'Ditch' Energy Neutralizer
Corpii A-Type Small Energy Neutralizer
Corpii A-Type Small Energy Neutralizer

Medium Polycarbon Engine Housing II
Medium Polycarbon Engine Housing II

High-grade Snake Alpha
High-grade Snake Beta
High-grade Snake Gamma
High-grade Snake Delta
High-grade Snake Epsilon
High-grade Snake Omega
Zainou 'Gnome' Shield Management SM-706
Zor's Custom Navigation Hyper-Link
Inherent Implants 'Lancer' Gunnery RF-906
Zainou 'Gnome' Weapon Upgrades WU-1006

Strong Frentix Booster

Mystic M x1000
Occult M x1000

3) Inline, square brackets:

[Tiamat, Shield]

Syndicate Damage Control
Entropic Radiation Sink II
Entropic Radiation Sink II
Entropic Radiation Sink II
Tobias' Modified Tracking Enhancer

Gistum A-Type 50MN Microwarpdrive  [unstable: capacitorNeed 108, cpu 40, power 150.4, signatureRadiusBonus 273, speedFactor 569.8]
Thukker Large Shield Extender  [unstable: capacityBonus 3575, cpu 32, power 124, signatureRadiusAdd 10.5]
Pithum A-Type Adaptive Invulnerability Field
X-Large Ancillary Shield Booster, Navy Cap Booster 400  [unstable: capacitorNeed 792, cpu 160, duration 4000, power 400, reloadTime 42000, shieldBonus 1176]
Dark Blood Small Capacitor Booster, Navy Cap Booster 400

Heavy Entropic Disintegrator II, Mystic M
'Smokescreen' Covert Ops Cloaking Device II
Medium 'Ditch' Energy Neutralizer
Corpii A-Type Small Energy Neutralizer
Corpii A-Type Small Energy Neutralizer

Medium Polycarbon Engine Housing II
Medium Polycarbon Engine Housing II

High-grade Snake Alpha
High-grade Snake Beta
High-grade Snake Gamma
High-grade Snake Delta
High-grade Snake Epsilon
High-grade Snake Omega
Zainou 'Gnome' Shield Management SM-706
Zor's Custom Navigation Hyper-Link
Inherent Implants 'Lancer' Gunnery RF-906
Zainou 'Gnome' Weapon Upgrades WU-1006

Strong Frentix Booster

Mystic M x1000
Occult M x1000

4) References wide:

[Tiamat, Shield]

Syndicate Damage Control
Entropic Radiation Sink II
Entropic Radiation Sink II
Entropic Radiation Sink II
Tobias' Modified Tracking Enhancer

Gistum A-Type 50MN Microwarpdrive [1]
Thukker Large Shield Extender [2]
Pithum A-Type Adaptive Invulnerability Field
X-Large Ancillary Shield Booster, Navy Cap Booster 400 [3]
Dark Blood Small Capacitor Booster, Navy Cap Booster 400

Heavy Entropic Disintegrator II, Mystic M
'Smokescreen' Covert Ops Cloaking Device II
Medium 'Ditch' Energy Neutralizer
Corpii A-Type Small Energy Neutralizer
Corpii A-Type Small Energy Neutralizer

Medium Polycarbon Engine Housing II
Medium Polycarbon Engine Housing II

High-grade Snake Alpha
High-grade Snake Beta
High-grade Snake Gamma
High-grade Snake Delta
High-grade Snake Epsilon
High-grade Snake Omega
Zainou 'Gnome' Shield Management SM-706
Zor's Custom Navigation Hyper-Link
Inherent Implants 'Lancer' Gunnery RF-906
Zainou 'Gnome' Weapon Upgrades WU-1006

Strong Frentix Booster

Mystic M x1000
Occult M x1000

[1] Gistum A-Type 50MN Microwarpdrive (unstable: capacitorNeed 108, cpu 40, power 150.4, signatureRadiusBonus 273, speedFactor 569.8)
[2] Thukker Large Shield Extender (unstable: capacityBonus 3575, cpu 32, power 124, signatureRadiusAdd 10.5)
[3] X-Large Ancillary Shield Booster (unstable: capacitorNeed 792, cpu 160, duration 4000, power 400, reloadTime 42000, shieldBonus 1176)

5) References narrow:

[Tiamat, Shield]

Syndicate Damage Control
Entropic Radiation Sink II
Entropic Radiation Sink II
Entropic Radiation Sink II
Tobias' Modified Tracking Enhancer

Gistum A-Type 50MN Microwarpdrive [1]
Thukker Large Shield Extender [2]
Pithum A-Type Adaptive Invulnerability Field
X-Large Ancillary Shield Booster, Navy Cap Booster 400 [3]
Dark Blood Small Capacitor Booster, Navy Cap Booster 400

Heavy Entropic Disintegrator II, Mystic M
'Smokescreen' Covert Ops Cloaking Device II
Medium 'Ditch' Energy Neutralizer
Corpii A-Type Small Energy Neutralizer
Corpii A-Type Small Energy Neutralizer

Medium Polycarbon Engine Housing II
Medium Polycarbon Engine Housing II

High-grade Snake Alpha
High-grade Snake Beta
High-grade Snake Gamma
High-grade Snake Delta
High-grade Snake Epsilon
High-grade Snake Omega
Zainou 'Gnome' Shield Management SM-706
Zor's Custom Navigation Hyper-Link
Inherent Implants 'Lancer' Gunnery RF-906
Zainou 'Gnome' Weapon Upgrades WU-1006

Strong Frentix Booster

Mystic M x1000
Occult M x1000

[1] Unstable Gistum A-Type 50MN Microwarpdrive
  capacitorNeed 108
  cpu 40
  power 150.4
  signatureRadiusBonus 273
  speedFactor 569.8
[2] Unstable Thukker Large Shield Extender
  capacityBonus 3575
  cpu 32
  power 124
  signatureRadiusAdd 10.5
[3] Unstable X-Large Ancillary Shield Booster
  capacitorNeed 792
  cpu 160
  duration 4000
  power 400
  reloadTime 42000
  shieldBonus 1176

6) References hybrid

[Tiamat, Shield]

Syndicate Damage Control
Entropic Radiation Sink II
Entropic Radiation Sink II
Entropic Radiation Sink II
Tobias' Modified Tracking Enhancer

Gistum A-Type 50MN Microwarpdrive [1]
Thukker Large Shield Extender [2]
Pithum A-Type Adaptive Invulnerability Field
X-Large Ancillary Shield Booster, Navy Cap Booster 400 [3]
Dark Blood Small Capacitor Booster, Navy Cap Booster 400

Heavy Entropic Disintegrator II, Mystic M
'Smokescreen' Covert Ops Cloaking Device II
Medium 'Ditch' Energy Neutralizer
Corpii A-Type Small Energy Neutralizer
Corpii A-Type Small Energy Neutralizer

Medium Polycarbon Engine Housing II
Medium Polycarbon Engine Housing II

High-grade Snake Alpha
High-grade Snake Beta
High-grade Snake Gamma
High-grade Snake Delta
High-grade Snake Epsilon
High-grade Snake Omega
Zainou 'Gnome' Shield Management SM-706
Zor's Custom Navigation Hyper-Link
Inherent Implants 'Lancer' Gunnery RF-906
Zainou 'Gnome' Weapon Upgrades WU-1006

Strong Frentix Booster

Mystic M x1000
Occult M x1000

[1] Gistum A-Type 50MN Microwarpdrive (unstable)
  capacitorNeed 108, cpu 40, power 150.4, signatureRadiusBonus 273, speedFactor 569.8
[2] Thukker Large Shield Extender (unstable)
  capacityBonus 3575, cpu 32, power 124, signatureRadiusAdd 10.5
[3] X-Large Ancillary Shield Booster (unstable)
  capacitorNeed 792, cpu 160, duration 4000, power 400, reloadTime 42000, shieldBonus 1176
DarkFenX commented 6 years ago

Another consideration corpmate brought up: how it looks in relatively narrow screens (e.g. non-maximized discord window, mobile). He prefers references because of it.

He liked inline and references. How inline looks on discord (quite a mess): image

References: image

I am starting to like approach with references as well: it allows you to see good fit overview like current EFT format (amount of slots, what kind of modules are there - additional lines with attributes quite significantly spoil that).

If we decide to stick to references, then another question arises: footer format. 1) Should it include full name of mutaplasmid? It makes parsing much easier. 2) Should it duplicate full base module name in footer? To not jump back and forth i suppose it should. 3) How to lay it out? One single long line, many short lines, or combination of these two?

Height-optimized reference footer: image

Width-optimized reference footer (whole fit itself becomes inconveniently high): image

Hybrid reference footer (compact): image

Hybrid reference footer (parsing-friendly): image

DarkFenX commented 6 years ago

I implemented export reference-based parsing-friendly format on abyssal_export branch:

[Tiamat, Shield]

Syndicate Damage Control
Entropic Radiation Sink II
Entropic Radiation Sink II
Entropic Radiation Sink II
Tobias' Modified Tracking Enhancer

Gistum A-Type 50MN Microwarpdrive [1]
Thukker Large Shield Extender [2]
Pithum A-Type Adaptive Invulnerability Field
X-Large Ancillary Shield Booster, Navy Cap Booster 400 [3]
Dark Blood Small Capacitor Booster, Navy Cap Booster 400

Heavy Entropic Disintegrator II, Mystic M
'Smokescreen' Covert Ops Cloaking Device II
Medium 'Ditch' Energy Neutralizer
Corpii A-Type Small Energy Neutralizer
Corpii A-Type Small Energy Neutralizer

Medium Polycarbon Engine Housing II
Medium Polycarbon Engine Housing II

High-grade Snake Alpha
High-grade Snake Beta
High-grade Snake Gamma
High-grade Snake Delta
High-grade Snake Epsilon
High-grade Snake Omega
Zainou 'Gnome' Shield Management SM-706
Zor's Custom Navigation Hyper-Link
Inherent Implants 'Lancer' Gunnery RF-906
Zainou 'Gnome' Weapon Upgrades WU-1006

Strong Frentix Booster

Mystic M x1000
Occult M x1000

[1] Gistum A-Type 50MN Microwarpdrive
  Unstable 50MN Microwarpdrive Mutaplasmid
  capacitorNeed 108, cpu 40, power 150.4, signatureRadiusBonus 273, speedFactor 569.8
[2] Thukker Large Shield Extender
  Unstable Large Shield Extender Mutaplasmid
  capacityBonus 3575, cpu 32, power 124, signatureRadiusAdd 10.5
[3] X-Large Ancillary Shield Booster
  Unstable X-Large Ancillary Shield Booster Mutaplasmid
  capacitorNeed 792, cpu 160, duration 4000, power 400, reloadTime 42000, shieldBonus 1176

So far i like it the most, especially as it removes unnecessary guesswork when importing fit.

By the way, if anyone could help me with UI side, I'd appreciate that, I am really bad at it.

blitzmann commented 6 years ago

By the way, if anyone could help me with UI side, I'd appreciate that, I am really bad at it.

I'll take a stab at it by this weekend, shouldn't be too difficult :)

DarkFenX commented 6 years ago

Apparently current import function doesn't even see distinction between drones in dronebay and drones in cargo. I guess it should be easier to rewrite it to support all the new stuff.

DarkFenX commented 6 years ago

Just finished both import and export for abyssal mods, you can test it out on abyssal_export branch. I moved EFT stuff to separate file because i had to write much more code to achieve following things:

1) drones/fighters in cargo now stay in cargo after import (except for very rare edge-cases when cargo has nothing but drones) 2) now during import you do not lose 'dummy slots' between your modules (this has always been rather annoying for me) 3) made it fairly resilient to format corruption (like absent blank lines/extra blank lines/corrupt names) 4) few other small things i do not remember

So we have to do UI side, and on the backend side - i left importEftCfg in original port.py, so have to decide if it should be left there or moved out too. There's possible circular import, but nothing which cannot be resolved. Maybe port should become package with several submodules, as there will be 4 modules if you resolve circular imports by putting PortProcessing to separate file.

blitzmann commented 6 years ago

Maybe port should become package with several submodules, as there will be 4 modules if you resolve circular imports by putting PortProcessing to separate file.

I've been thinking about doing this, just don't have the time. I'm all for it though if you, or anyone else, wants to whack it. It makes sense.

Will take a look at current progress today and try to develop out a GUI.

blitzmann commented 6 years ago

Some more ideas on possibly splitting these all out into their own package. Might be very useful to be able to allow each format to register itself with pyfa as being available, the same way our stats views, data columns, context menus, etc register themselves. Right now there's various different places you have to touch in order to get it to work - if we can have them all encapsulated into their own class, could be nice. Each format would register itself and be required to have an import and export methods. Other required properties would be format name, description, and any "extended export options" for that format (right now only useful for EFT for implants/abyssal stats, but other formats may want to turn on/off different things). The only thing I don't know about is how we would do the "auto import" feature - that requires knowledge of all formats, unless we also have a "test" method requirements for all format classes that we can then loop for, "test", and that would determine if it's the format that class represents. Dunno.

That's a much bigger refactor, though. For now, I'm going to keep it simple and create two checkboxes on the GUI for EFT: one for Implants, and one for Abyssal stats. The combination of these will be stored as a bitmask that we can then send into the export function (as well as store as a preference).

blitzmann commented 6 years ago

I've got a rough implementation working for this:

image

That options section only shows if EFT is selected. We now pass in options, which is a bitmask of various options the format can support, into the required functions. Only useful for EFT right now. Additionally, if other formats ever need options, the GUI will need to be tweaked as it's hardcoded for EFT at the moment (shouldn't be too difficult to get it to work though, if needed).

Probably a lot of clean up that should happen, but this should at least get use started with a way to test this more easily. Can also make a test build to post here whenever you're ready @DarkFenX :D

DarkFenX commented 6 years ago

Re refactoring - IDK, will see when doing it. Some of formats are quite specific (some are import only like eftcfg, some are export only like efs if i understand correctly), so maybe it's not worth, but still doable.

Re context menus - they might be convenient, but IMO currently context menus are a mess (at least in fitting panel). There're quite many menu items and they are grouped quite un-intuitively - i'm not sure if it's intentional or consequence of such loose coupling (or just my taste). Even if such way is used in import/export there still should be a way to strictly control order of items on UI, and order of test functions during format detection when autoimporting.

Re export UI - is it possible to place checkboxes right below EFT radiobutton? Or is it bad idea? Or is it too hard in wx? Anyway, current layout also looks alright to me.

Edit: actually it doesn't save export option values (but i think you didn't mean to implement it anyway) and looks a bit stripped on gnome 3: image

Re test build - i think export/import code is ready for beta, but I'd hold off until we move port to package (which might introduce new bugs).

blitzmann commented 6 years ago

Edit: actually it doesn't save export option values (but i think you didn't mean to implement it anyway) and looks a bit stripped on gnome 3:

Yeah, didn't get that far with it yet. That's next on my list. And there does seem to be a bug with wx.StaticBox, so I added a spacer at the top - I guess it's a bug that only affects Windows. See https://github.com/wxWidgets/Phoenix/issues/974

Could you remove the following line from the copySelectDialog:

self.bsizer1.AddSpacer(10)

And see if that helps?

I added it to the bottom of the list because, in the future, there may be other formats that require options, and I figured it would be a good thing to have them in the same place always.

re: refactoring, good points, I forgot there's a few formats that are one way only. Still, I think it might be a good idea to look into it at some point - obviously not necessary for this feature, nor am I suggesting we try to completely refactor import / exports right now lol. I think it would suffice for now to just get the code moved to separate modules and cleaned up a bit.

Awesome work so far on it though, kudos 👍

blitzmann commented 6 years ago

update: it now saves the settings. It's ugly, but so is everything about the import/export lol.

DarkFenX commented 6 years ago

If i comment out self.bsizer1.AddSpacer(10): image

Also, I'd change 'Abyssal' label to something else (mostly because abyss is just place where you get all these modules) - like 'Mutated Attributes', 'Mutaplasmids', overall words based on 'mutation' hint what this option changes more clearly (that's also question of personal taste though) .

DarkFenX commented 6 years ago

Regarding UI, few more thoughts:

1) Checkboxes which appear and disappear are rather annoying because window size changes. On gnome, as window is centered, its elements jump from one position to another. I would always show them, just gray out when non-eft format is selected. 2) Placement. Just realized that you said that checkboxes are on the bottom because other formats might have options (missed it when reading 1st time). Would it be possible to just put checkboxes needed for each format below that format? Like 2 current checkboxes between eft and xml, if xml needs anything - between xml and dna, and so on. Or you think it's rather ugly/complex solution?

blitzmann commented 6 years ago
  1. Ah, yeah, I guess that would be annoying lol. Honestly, been thinking of actually redesigning this dialog completely - I've always wanted one that generated the format in a read-only text control that was visible. Want to play around with this idea when I get a chance, but for now...
  2. I think for now that would be absolutely fine, but yeah if other formats ever start adopting different options could get pretty messy. But that's a future thing, and one that itself is even a big "what if".

Will move the section to under the EFT option. On the spacing issue, looks like it did reduce the space, but something else funky is going on with it, might just remove the StaticBox - this would actually work out pretty well moving the options under the format selection, doesn't need a label then 😄

blitzmann commented 6 years ago

Done and done! Please pull changes and let me know if all's good on that front (at least good enough for a test :D)

DarkFenX commented 6 years ago

Looks great now! Thank you. I will try to finish splitting stuff into their own files (without reorganizing code so that formats register themselves) today, and hopefully everything will be ready by your tomorrow.

DarkFenX commented 6 years ago

I have finished splitting port.py, you can check abyssal_export. I tested as much as i could - every format except for EFT cfg files (because i do not have any), but didn't test every feature of every format (tested only EFT format thoroughly).

You can do few tests as well (especially if you have EFT cfg) and then merge this branch to master.

I'm closing this issue as functionality has been implemented.

blitzmann commented 6 years ago

Keeping this open until release to remind me to note it with release notes :)