Feature 47 test: cargo monitor missed a collection

[richardbuckle]

This refers to the unreleased branch that implements the cargo monitor (issue #47)

Steps to reproduce

1. Not tried to repro. I was in a degraded emissions USS scavenging items with zero cargo, and picked up 2x Personal Effects and 1x Encrypted Data Storage.

Expected

Both EDSM and EDDI show all the above items in cargo

Observed

EDSM shows both

but EDDI only shows the personal effects

Investigation

Log excerpt:

{ "timestamp":"2017-11-08T23:21:47Z", "event":"USSDrop", "USSType":"$USS_Type_Salvage;", "USSType_Localised":"Degraded emissions detected", "USSThreat":0 }
{ "timestamp":"2017-11-08T23:21:48Z", "event":"SupercruiseExit", "StarSystem":"Dethall", "Body":"Dethall", "BodyType":"Star" }
{ "timestamp":"2017-11-08T23:21:48Z", "event":"Music", "MusicTrack":"Exploration" }
{ "timestamp":"2017-11-08T23:23:24Z", "event":"MaterialCollected", "Category":"Manufactured", "Name":"mechanicalequipment", "Count":3 }
{ "timestamp":"2017-11-08T23:23:46Z", "event":"MaterialCollected", "Category":"Manufactured", "Name":"chemicalprocessors", "Count":3 }
{ "timestamp":"2017-11-08T23:24:02Z", "event":"CollectCargo", "Type":"EncriptedDataStorage", "Stolen":true }
{ "timestamp":"2017-11-08T23:25:26Z", "event":"CollectCargo", "Type":"PersonalEffects", "Stolen":true }
{ "timestamp":"2017-11-08T23:30:32Z", "event":"Friends", "Status":"Offline", "Name":"DeltaSnake" }
{ "timestamp":"2017-11-08T23:30:47Z", "event":"MaterialCollected", "Category":"Manufactured", "Name":"wornshieldemitters", "Count":3 }
{ "timestamp":"2017-11-08T23:31:13Z", "event":"CollectCargo", "Type":"PersonalEffects", "Stolen":true }

[richardbuckle]

Possibly a commodity def issue from my test merge. Let's see: CommodityDefinitions.cs line 285

            {128672124, new Commodity(252, "EncriptedDataStorage", "Encrypted Data Storage", "Salvage", 806, false) },

[Tkael]

Looking at the diff on the PR, that commodity definition line appears to be untouched. I've occasionally found that doing a complete rebuild after switching branches is needed for everything to work properly. Did you rebuild prior to testing, or did you go straight to 'Start debugging'?

[Hoodathunk]

From first pass, I don’t see anything that would indicate why it didn’t register the Cargo collection. Link/paste your cargomonitor.json file and we'll see what it says.

[richardbuckle]

Yup, I did a clean and full rebuild (and a code analysis build into the bargain). Here is cargomonitor.json:

{
  "cargo": [
    {
      "name": "Personal Effects",
      "stolen": 1,
      "haulage": 0,
      "other": 0,
      "total": 1,
      "price": 379,
      "category": "Salvage",
      "commodity": {
        "name": "Personal Effects",
        "category": "Salvage",
        "avgprice": 379,
        "rare": false,
        "buyprice": null,
        "stock": null,
        "stockbracket": null,
        "sellprice": null,
        "demand": null,
        "demandbracket": null,
        "StatusFlags": null,
        "EDDBID": 253,
        "EDName": "PersonalEffects"
      },
      "haulageamounts": []
    }
  ],
  "cargocarried": 1
}

I just tried re-loggin in (I'm in space, not docked) and there was no change to either cargomonitor.json or the displayed amount. EDDiscovery is showing the cargo event from my log-in and it contains all 3 items.

[richardbuckle]

Stepping thru in the debugger and just noticed that the personal effects count is wrong in EDDI, see screenshots and JSON, Should be 2 but is 1.

[richardbuckle]

CargoMonitor.cs line 172

                    cargo.other = cargo.total - cargo.stolen - cargo.haulage;

Shouldn't the LHS be inventoryCargo.other ?

[richardbuckle]

Exception caught: "This type of CollectionView does not support changes to its SourceCollection from a thread different from the Dispatcher thread."

[richardbuckle]

Back to CargoMonitor.cs line 172, shouldnt we be setting inventoryCargo.stolen and inventoryCargo.haulage as well?

[richardbuckle]

Oh I get it, the CargoInventoryEvent doesn't tell us the ownership types.

[richardbuckle]

Testing this fix:

            // CargoInventoryEvent does not contain stolen, missionid, or cost information so fill in gaps here
            foreach (Cargo cargo in @event.inventory)
            {
                Cargo inventoryCargo = inventory.FirstOrDefault(c => c.name == cargo.name);
                if (inventoryCargo != null && inventoryCargo.total != cargo.total)
                {
                    inventoryCargo.total = cargo.total;
                    // we aren't given cargo.stolen or cargo.haulage so we just have to put the balancing amount in inventoryCargo.other
                    inventoryCargo.other = inventoryCargo.total - inventoryCargo.stolen - inventoryCargo.haulage;
                }
                else
                {
                    // We haven't heard of this cargo so add it to the inventory directly
                    AddCargo(cargo);
                }
            }

[richardbuckle]

The threading issue I'll leave to @Hoodathunk but this seems to work. Some unit tests around this might be a good idea.

handleCommodityCollectedEvent() :

                if (@event.stolen)
                {
                    newCargo.stolen = 1;
                }
                else
                {
                    cargo.other = 1;
                }

That doesn't look right either.

Some unit tests here would help a lot!

[richardbuckle]

Suggested changes pushed to branch feature/47-cargo-monitor-VB-possible-fix.

[richardbuckle]

Thread exception when jettisoning, here's the call stack (commit cf51b4e7 on branch feature/47-cargo-monitor-VB-possible-fix):

>   EddiCargoMonitor.dll!EddiCargoMonitor.CargoMonitor.RemoveCargo(string commodityName) Line 602   C#  Symbols loaded.
    EddiCargoMonitor.dll!EddiCargoMonitor.CargoMonitor.handleCommodityEjectedEvent(EddiCargoMonitor.CommodityEjectedEvent event) Line 247   C#  Symbols loaded.
    EddiCargoMonitor.dll!EddiCargoMonitor.CargoMonitor.PreHandle(EddiEvents.Event event) Line 109   C#  Symbols loaded.
    EDDI.exe!Eddi.EDDI.OnEvent(EddiEvents.Event event) Line 761 C#  Symbols loaded.
    EDDI.exe!Eddi.EDDI.eventHandler(EddiEvents.Event journalEvent) Line 743 C#  Symbols loaded.
    EddiJournalMonitor.dll!EddiJournalMonitor.JournalMonitor.ForwardJournalEntry(string line, System.Action<EddiEvents.Event> callback) Line 31 C#  Symbols loaded.
    EddiJournalMonitor.dll!EddiJournalMonitor.JournalMonitor..ctor.AnonymousMethod__1_0(string result) Line 24  C#  Symbols loaded.
    EddiJournalMonitor.dll!EddiJournalMonitor.LogMonitor.start() Line 122   C#  Symbols loaded.
    EddiJournalMonitor.dll!EddiJournalMonitor.JournalMonitor.Start() Line 2575  C#  Symbols loaded.
    EDDI.exe!Eddi.EDDI.keepAlive.AnonymousMethod__0() Line 592  C#  Symbols loaded.
    [External Code]     Annotated Frame

[richardbuckle]

After jettisoning all, I now have -1 stolen and 1 other personal effects for a total of 0.

ANTIMATTER CONFIRMED :-P

[richardbuckle]

[Hoodathunk]

Verified now working in-game using latest commit of CargoMonitor in feature/47-cargo-monitor

[Hoodathunk]

To be corrected by PR #496

[Tkael]

Incorporated.