-
Notifications
You must be signed in to change notification settings - Fork 156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement FSSSignalDiscovered in EDDN plugin #1586
Conversation
logger.warning("USSType is $USS_Type_MissionTarget;, dropping") | ||
return 'Dropping $USS_Type_MissionTarget;' | ||
# Can check SystemAddress here, but we'll remove it from this signal list, to be added to the batch | ||
if this.systemaddress is None or this.systemaddress != entry['SystemAddress']: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now this is a case for having the augmentations be optional (but that needs discussing over on EDCD/EDDN#154 .
My thinking here is that it's better to have all the FSSSignalDiscovered events for a system, even if quirks of them mean we can't always have more than the SystemAddress
for them.
Whilst the proposed method of "queue events until a different event is seen" might work out OK, it is going to result in often seeing two batches of events per system. Also, have you investigated to be sure that any following event will definitely be an appropriate one, with system information ? On a cursory check I can see that it can at least be any of: I am more inclined to push for Listeners just accepting that we send the FSSSignalDiscovered events as-is (in batches), without augmentations. |
For following events I also get "SupercruiseExit", "SupercruiseEntry", "ScanBaryCentre", "Scan", "CodexEntry" and more with SystemAddress, but I do also get "bad" ones like Music and ReceiveText [1]. I think USSes spawning at any time means that FSSSignalDiscovered can appear anywhere in the journal. So it sounds like we should send without augmentation if we don't want to lose signals. This does also mean that some batches would have a size of 1. Or, idea: wait to send the batch until we have enough information, which may mean waiting.. but could also lose data. e.g. [2] [1]
[2]
|
It's not a matter of "until we have enough information". Once we're in a system then the FSDJump, CarrierJump or Location event will have given that augmentation information. The only concern with newly popped USS is how often they occur and how often we'd end up sending a batch with only a single signal in it. We don't need to batch perfectly, just enough to not spam so many messages through EDDN, as we would if sending literally each event in its own message (and there's this thing with them preceding the arrival in system). |
So, this code will definitely need to both be tracking "prior events" location and checking if the new non-FSSSignalDiscovered event is a "Location" one. In Odyssey when you login in space you get the signals and then the In Horizons when you login in space you get the |
@@ -1657,6 +1745,18 @@ def journal_entry( # noqa: C901, CCR001 | |||
# drop those events, or if the schema allows, send without those | |||
# augmentations. | |||
|
|||
elif event_name == 'fsssignaldiscovered': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: Will need to check if we have any signals queued up when a new Location/FSDJump/CarrierJump comes in, cross-check SystemAddress, send.
Keep in mind that in Horizons the Location/FSDJump comes before the new system's signals, in Odyssey it's after.
Thanks for the initial work, I've cloned your branch and am now beating it into shape, see #1590 . Closing this. |
Re:
EDCD/EDDN#152
EDCD/EDDN#154
This version 1 of extending the EDMC EDDN plugin to send FSSSignalDiscovered. Open for discussion. It does send OK, but of course schema validation fails as the schema isn't deployed anywhere; and beta is not currently up.
Example of a generated message: