EDCD / EDDI

Companion application for Elite Dangerous
Other
444 stars 81 forks source link

In some cases, EDDI may be reporting obsolete economy data to EDDN #1437

Closed Tkael closed 5 years ago

Tkael commented 5 years ago

Reported by Artie (Inara): guys, I am not sure where the issue is, but it seems that some stations may be wrongly reported with Damaged economy to EDDN sometimes, see: https://ross.eddb.io/eddn/log?parsing_result=&parsing_result_detail=&header_software_name=&schema=&schema_main_group=&schema_sub_group=&system_id=&station_id=32872&body_id=

EDDI version 3.4.1

To clarify - the Damaged state is alright, it's a valid state, but the station shouldn't have it set and should be with the Repair state/economy. I think this station was damaged weeks ago (but not sure about exact timeframe), thus the thought it may be originating in some old records smiley

https://ross.eddb.io/eddn/log/203323338

Investigation: It looks like the last message setting the economy was using the commodities endpoint. The commodities endpoint uses saved information from the CurrentStation object to provide economy information. https://github.com/EDCD/EDDI/blob/9e7b91b11b7c5d25cb892449a952bc80c78af2cb/EDDNResponder/EDDNResponder.cs#L369 It is conceivable that we're picking up old data from log loading and reporting it with a Market event entry.

@VerticalBlank (EDDI/EDRefCard) @Hoodathunk (EDDI) This endpoint does not require sending economy data. https://github.com/EDSM-NET/EDDN/blob/master/schemas/commodity-v3.0.json I suggest that we remove economy information from messages for this endpoint (it'll still be recorded when Docked messages from the journal are sent to EDDN).

Alternate proposal: only send economy data for updates of type profile (since we know it's fresh at that time), but not for updates of type market (where stale data may be possible).

Tkael commented 5 years ago

Consensus for fixing: Delete line 373, lines 387-390, and lines 402-416 in EDDNResponder.cs