Tkael
commented
6 years ago - [x] Handle stolen cargo flags in Cargo/Inventory
- [x] Add and handle LaunchDrone event
Closed Tkael closed 6 years ago
Tkael
commented
6 years ago 
Hoodathunk
commented
6 years ago Both of these items are coded into the new Cargo Monitor
Hoodathunk
commented
6 years ago Revised Cargo Monitor to be implemented in 3.0.1.
Hoodathunk
commented
6 years ago To be corrected by PR #496
rodan123
commented
6 years ago @Hoodathunk : Testing the new cargo inventory script - Great job.
Minor nitpick; There is an extra 'of' on line 41 of the cottle script 'Cargo report' which causes "of of" to be spoken.
_{cargo.total} tonne{if cargo.total != 1:s} of {if cargo_commodityarray[0] = "Limpet": of limpets |else: of {cargo.name} }
Hoodathunk
commented
6 years ago Thanks for the heads up. I will take care of this in the next beta release.
rodan123
commented
6 years ago @Hoodathunk Thanks. Unfortunately I have run into another problem. See below. Update: I think I have isolated the problem in the code. See PR #497
I am testing the cargo inventory and am out mining. I found that if I eject an amount of cargo the amount removed from the cargo report is doubled. So if I have 2 tons of an item and eject one, cargo reports the item and its count as zero. If I eject both it'll report a count of -2. When I retrieve the ejected items. the inventory is corrected and the amounts are reported as expected. This bug happens consistently, upon each cargo ejection. I monitored the commodity ejected event and it is returning correct amounts. Also, I found that the cargo report doubles a commodity I have refined and adds two units to inventory instead of one, if I already have one of the item. The first refined commodity is reported correctly, subsequent additions are doubled. Confirmed. Happens consistently. Again, retrieving affected cargo after ejection restores proper inventory levels.
Hoodathunk
commented
6 years ago Yup, it's getting updated twice. Another good catch. This is why we're in beta... keep 'em coming. :-)
Thanks for the PR but we will not be merging any branches that do not originate in EDCD/EDDI. I'll take it from here.
Tkael
commented
6 years ago Thanks for the PR but we will not be merging any branches that do not originate in EDCD/EDDI. I'll take it from here.
This would be primarily due to the pending changes for internationalization... after that I believe we'll be more open to outside PRs again.
rodan123
commented
6 years ago @Tkael Interesting, withdrew PR #497.
@Hoodathunk Another cargo reporting bug; Mission accepted events are not handled properly. Does not filter fetch mission types and assigns cargo that is to be fetched to inventory when the mission is accepted. Looking at the code; all fetch, salvage, and altruism missions will be affected, since the code doesn't consider the mission type.
I had cargo reporting working using VA with the variables and events available within EDDI before switching over to your code. It is a lot of work to identify and handle the different scenarios.
Hoodathunk
commented
6 years ago Yes, mission type filtering needs some work and thanks for drawing my attention to it. I have some changes that I'm testing now, I expect to include in 3.0.1-b2.
Hoodathunk
commented
6 years ago @rodan123 I've just pushed hotfix/454-cargo-monitor. If you're so inclined, please try it out and give me some feedback.
rodan123
commented
6 years ago @Hoodathunk Thanks I will try it.
I came here, however, to document another cargo reporting issue. A missing cargo sold event; SearchAndRescueEvent
I just took a quick glance at the code in your new branch and the SearchAndRescueEvent handler doesn't seem to have been added.
Update: a bug in the new code; after selling all of a given commodity in the market, then asking for a cargo report, the sold commodity is still reported by name with a zero count. Note: was able to fix by changing cargo.haulageamounts == null to cargo.haulageamounts.Count < 1 in handleCommoditySoldEvent
Hoodathunk
commented
6 years ago Yes, Search & Rescue event not yet added... will have to wait until Internationalization is merged in.
Hoodathunk
commented
6 years ago I can't reproduce the 'commodity with zero count' being announced in the cargo report... Currently have three items in manifest, with one having a zero count (black box for salvage mission). Cargo report only reporting the two with non-zero counts.
rodan123
commented
6 years ago @Hoodathunk Unfortunately I was able to reproduce the 'zero count' bug. I rechecked to make sure. I went out and collected a bunch of stuff from USSs. Returned to station sold all of 1 Performance Enhancers. Requested cargo report. "0 tons of Performance Enhancers" You can see from the json that the commodity still exists with a zero count. Found the issue: I was not using the default cottle script, I see in the default script you do a check for zeroed items; {for cargo in inventory: {if cargo.total > 0: Not sure that is the best solution, but it explains the differences in our tests. I think it is better to remove the item from the json, rather than to leave a zeroed entry.
Otherwise, looking good. Thanks.
Hoodathunk
commented
6 years ago @rodan123 I intentionally create 'zero-ed' commodity entries as placeholders for tracking active missions, which requires the for cargo in inventory: {if cargo.total > 0:
snippet in the cargo report script.
The reason a zero-ed (total=0) non mission-related commodity remained was because I only tested whether the haulage data list was null. should be: if (cargo.haulageamounts == null || !cargo.haulageamounts.Any())
Fixed.
Now that Internationalization is PR'd, I'll get a handler for 'Search and Rescue' implemented
Thanks for your help!
rodan123
commented
6 years ago @Hoodathunk No problem. I think it's almost there.
But, I am having an issue with certain mission completions. It has happened a few times now. The cargo inventory is not updating upon completion and the code is throwing an error. (see below). I had pulled your latest commits. It may be due to the name not having an underscored third field xxxx_xxxx_xxxx and is simply 'Mission_Delivery'.
cargomonitor.json excerpt;
_"EDName": "Superconductors" }, "haulageamounts": [ { "id": 371874916, "name": "Mission_Delivery", "amount": 30, "expiry": "2018-04-27T22:55:55Z" } ]_
Hoodathunk
commented
6 years ago @rodan123 I've added handling for the 'SearchAndRescue' event and the 'rescue' mission type. I have a 'potential fix' for the '.AtEllement()' exception by parsing @event.name instead of haulageamount.name.
Committed & pushed. Give this a spin.
rodan123
commented
6 years ago @Hoodathunk Will do.
Typos in latest commit; handleMissionFailedEvent and handleMissionAbandonedEvent
case "resuce": s/b case "rescue":
Hoodathunk
commented
6 years ago Good catch. Fixed
rodan123
commented
6 years ago Missing mission type? Piracy? Is there an FDEV list of all the possible types somewhere?
{ "timestamp":"2018-04-27T22:10:52Z", "event":"MissionAccepted", "Faction":"Clan of HIP 16538", "Name":"Mission_Piracy_Anarchy", "LocalisedName":"Steal 3 units of Exhaust Manifold", "Commodity":"$ExhaustManifold_Name;", "Commodity_Localised":"Exhaust Manifold", "Count":3, "DestinationSystem":"Yemakinni", "DestinationStation":"Chaviano Colony", "Target":"KosPilot", "Expiry":"2018-04-28T09:16:30Z", "Wing":false, "Influence":"Med", "Reputation":"Med", "Reward":16265, "MissionID":372253622 }
Hoodathunk
commented
6 years ago Short answer is "nope". It's been a bit hit & miss as I try to run missions and observe their behavior.
I will add 'piracy'.
From observation, I've deduce that commodities that are unique to the mission are 'haulage'. What I haven't completely fleshed out is all the 'illegal' missions... such as piracy.
This is new territory... We'll keep added as we run across new ones.
Once I'm done with the Mission Monitor, I will be adding a mission types .csv to the EDCD database.
rodan123
commented
6 years ago Allowing a fetch mission to fail without holding any mission cargo leaves an empty entry in cargomonitor.json
{ "timestamp":"2018-04-27T22:37:49Z", "event":"MissionAccepted", "Faction":"Independents of HIP 16405", "Name":"Mission_Rescue", "LocalisedName":"Liberate 6 Commercial Samples", "Commodity":"$ComercialSamples_Name;", "Commodity_Localised":"Commercial Samples", "Count":6, "DestinationSystem":"Yemakinni", "DestinationStation":"Chaviano Colony", "Target":"Jackson Hall", "Expiry":"2018-04-30T04:09:09Z", "Wing":false, "Influence":"Med", "Reputation":"Med", "Reward":388063, "MissionID":372263469 }
{ "timestamp":"2018-04-28T01:09:31Z", "event":"MissionFailed", "Name":"Mission_Rescue_name", "MissionID":372263469 }
Hoodathunk
commented
6 years ago Updated to include 'piracy' as a watched mission type. Tweaked stripping of empty commodity manifests.
darth-crunchus
commented
6 years ago Okay, in regards to my previous issue, it finally purged the cargo from the variables; I just logged into the game, and Eddi just gave me the "What's good in the neighborhood" report, with no reminder message. That was strange, though, because it even persisted for close to five hours after having quit the game multiple times and reloading Voice Attack several times to try to get it to clear.
Hoodathunk
commented
6 years ago @darth-crunchus There are some issues with the 1st pass of the Cargo Monitor in 3.0.1-b1, including a wonky Commodities sale check script, but those are resolved in the revision included in b2. Please be patient and we will have the update out soon.
darth-crunchus
commented
6 years ago @Hoodathunk Oh, absolutely! I was just following up to let Tkael know that the issue resolved itself, as the ticket was closed. I would like to say, though, that this is one heck of an amazing project, and I've been getting quite a large amount of enjoyment out of it in the time that I've been using EDDI. Thanks for the hard work y'all are doing; it's a fantastic addition to Elite.
Darkcyde13
commented
6 years ago Hi,
I've just been looking at the new(ish) cargo monitor stuff, as I was intending to remove all references to my cargo log functionality. I've found a couple of things with the 'Cargo report' script.
I noticed it's a slightly reworked copy of my original 'CargoLog_Report' script and it seems a line of code has been accidentally left behind/not updated. It says:
{if cargo_commodity_array[0] = "Limpet":
Should this be something like:
{if cargo.name = "Limpet":
I've also removed the full stop at the end of
Your cargo {OneOf("bay", "hold")} is full.
as this was creating an unnecessary sentence pause in the middle of the spoken report.
"Your cargo bay is full. of beer." This wasn't in my original script for just this reason.
Finally, not a bug, but a personal preference, I've slightly updated the illegal/stolen section at the end to give a little more variety:
{if stolen || illegal:
Warning: You are carrying
{if stolen: stolen {OneOf("cargo","goods")}}
{if illegal:
{if stolen:, and }
{OneOf("cargo that is","goods that are")} illegal
{Occasionally(2,"contra-band")} in this jurisdiction
}.
}You can use it, or not, it's up to you. :)
Keep up the great work guys! I'm hoping to spend a bit more time back in Elite soon. :)
Tkael
commented
6 years ago It's good to have you back. :-) @Hoodathunk's the guy to review your recommendations.
Hoodathunk
commented
6 years ago Hey @Darkcyde13 ! Your first comment has already been addressed. I've incorporated your 2nd comment in a recent commit. Both are merged into the 'develop' branch but not yet made public in 3.0.1-b2.
Darkcyde13
commented
6 years ago Thanks for taking my idea on-board! :)
Now I have another. ;) I noticed that the haulage variable is calculated, but not used (yet), so here's my take on that. I've added this between the section listing inventory, and the part detailing any free space.
{if haulage > 0:
{if haulage = ship.cargocapacity:
This is all
|else:
{haulage} tonne{if cargo.total != 1:s are |else: is}
}
haulage mission cargo.
}I've not had a chance to test this yet, but it should work in theory. :)
Tkael
commented
6 years ago @Hoodathunk Are you ready to close this issue? I assume you've had a chance to review @Darkcyde13's suggestion.
Darkcyde13
commented
6 years ago I'm glad you haven't closed this yet. After reading Tkael's comment above this morning at work, I realised I had made a couple of big errors in that haulage section. I don't know what I was thinking at the time, I must have been asleep at the keyboard! :(
Anyway, here's a corrected version, and I've tested this in game just now...
{if haulage > 0:
{if haulage = cargocarried:
This is all
|else:
{haulage} tonne{if haulage != 1:s are |else: is}
}
haulage mission cargo.
}Sorry about that!
Hoodathunk
commented
6 years ago Hey Darkcyde, thanks for your input. I tweaked it a bit... this feels like it flows a bit better.
{_ Report current cargo manifest }
{set cargocarried to 0}
{set haulage to 0}
{set illegal to false}
{set stolen to false}
{if len(inventory) > 0:
{for cargo in inventory:
{set cargocarried to cargocarried + cargo.total}
{set haulage to haulage + cargo.haulage}
{if cargo.stolen > 0: {set stolen to true}}
{if status.docked:
{for prohibited in station.prohibited:
{if cargo.name = prohibited: {set illegal to true}}
}
}
}
}
{if cargocarried > 0:
{if cargocarried = ship.cargocapacity:
Your cargo {OneOf("bay", "hold")} is full
|else:
You are carrying {cargocarried} tonne{if cargocarried != 1:s}
}
{if len(inventory) = 1:
{if inventory[0].name = "Limpet":
of limpets.
|else:
of {inventory[0].name}.
{if haulage = cargocarried:
Note: All cargo is mission related haulage.
}
}
|else:
{if cargocarried = ship.cargocapacity:
.
|else:
of cargo.
}
{Occasionally(2, "Cargo")} {OneOf("Manifest", "Inventory")} is as follows:
{set c to []}
{for cargo in inventory:
{if cargo.total > 0:
{set c to cat(c, [cargo])}
}
}
{set position to 0}
{for cargo in c:
{cargo.total} tonne{if cargo.total != 1:s} of
{if cargo.name = "Limpet":
limpets
|else:
{cargo.name}
}
{set position to position + 1}
{if position = len(c) - 1: and |else:,}
}
{if haulage > 0:
Note: {haulage} tonne{if cargo.total != 1:s} of your manifest is mission related haulage.
}
}.
{if cargocarried < ship.cargocapacity:
{set free to ship.cargocapacity - cargocarried}
You have {free} tonne{if free != 1:s} {OneOf("free","of free space")}.
}
{if stolen || illegal:
Warning: You are carrying
{if stolen: stolen goods}
{if illegal:
{if stolen:, and }
goods that are illegal in this jurisdiction
}.
}
|else:
{OneOf("Your cargo bay is empty.","You have no cargo.","You are carrying no cargo.")}
}Let me know what you think.
Darkcyde13
commented
6 years ago Hi Hoodathunk!
I'm glad that you like my idea, and have chosen to take it on-board, thank you. :)
I've tested your version in game and it works fine. I was surprised that it produces almost exactly the same results as my original proposal, I thought you'd make more radical changes:
You are carrying 100 tonnes of cargo. Inventory is as follows: 98 tonnes of Clothing and 2 tonnes of Biowaste, Note: 90 tonnes of your manifest is mission related haulage.. You have 332 tonnes of free space.
You are carrying 100 tonnes of cargo. Inventory is as follows: 98 tonnes of Clothing and 2 tonnes of Biowaste. 90 tonnes are haulage mission cargo. You have 332 tonnes of free space.
I did find a couple of things though. First, you seem to have copied my error from my first iteration of the haulage code.
Note: {haulage} tonne{if cargo.total != 1:s} of your manifest is mission related haulage.
Instead of cargo.total, this needs to be haulage. In the rare event that you are only carrying one tonne of haulage, cargo total would still make it 'tonnes' if you have other cargo on board. Also (a minor nit-pick) you don't need the full stop at the end of that line as there is one in the code just at the end of that block which terminates that sentence no matter what text it ends with. ;)
Second, I'm a little confused by the following code:
{set c to []}
{for cargo in inventory:
{if cargo.total > 0:
{set c to cat(c, [cargo])}
}
}
{set position to 0}
{for cargo in c:
{cargo.total} tonne{if cargo.total != 1:s} of
{if cargo.name = "Limpet":
limpets
|else:
{cargo.name}
}
{set position to position + 1}
{if position = len(c) - 1: and |else:,}
}
I can't see the reason for setting up an array of c, as the second part is the same as my original code and does exactly the same job. The only thing I can think of is if you have a cargo item but have zero tonnes of it, but wouldn't that be a bug in the cargo system code, as I've never heard of it happening in game?
I'm sorry that I sound like an ass, picking little things in your code. I've always tried to make things as short and efficient as I possibly can. Like this line:
{if cargocarried < ship.cargocapacity: of cargo}.
Does exactly the same as your code of:
{if cargocarried = ship.cargocapacity:
.
|else:
of cargo.
}It's just the way I am and how I was taught to code. :(
I don't think you and I will see eye-to-eye on script content for the default personality, but I think that's a good thing. If we did, then I would have no reason to make my own personality. :)
Ugh, I think i'm digging a hole for myself now. I'll shut up. :/
Tkael
commented
6 years ago Rather than You have 332 tonnes of free space, perhaps You can carry another 332 tonnes? :-)
Hoodathunk
commented
6 years ago Hey DC, that piece of script is deliberate, I am stripping out manifest entries with 0 amounts. In order to properly track all mission related cargo, I create 0 amount entries for ‘collect’, ‘salvage’, etc mission types.
Darkcyde13
commented
6 years ago Hi Hoodathunk,
Ahh I thought that there would be some reason for it, I just couldn't see what it was, and it made no sense by itself.
Apologies for my previous rant. :/
Hoodathunk
commented
6 years ago We're down to suggestions on personality scripts now, I think it's time we can close this out.
Thanks to all for the invaluable help.