-
Notifications
You must be signed in to change notification settings - Fork 65
Add ScanOrganic schema 1.0 #230
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
base: develop
Are you sure you want to change the base?
Conversation
|
Original issue for reference: #212 |
|
Given that Orvidius from ED Astro has been using |
|
Just for informational purposes, though I'm not sure if it's useful, the third scan actually triggers a second 'Sample' event when the scan is first completed. The 'Analyse' event is then triggered after the aforementioned animation. That could theoretically be used to flag 'finished' scans even without the 'Analyse' event. @Tkael Lat Long should be available. I suppose that could be added, and it is reported with Codex Entries, so I can add that to the schema. |
|
I'm generally following the format of CodexEntry events as it comes to reporting BodyName (which is not normally in the event data). But if that's not necessary I can remove it. |
|
Arguably all would be needed to indicate whether the organic is present is one of the two ("Log" or "Sample"). If that is all that we need then latitude / longitude wouldn't be important, we wouldn't need to worry about adding that data as an augmentation, and we could suppress any repetitions (e.g. the Whether latitude and longitude are important really is a question for the consumers (e.g. NoFoolLikeOne, Orvidius, etc.). |
|
Cross posting from the issue comments:
|
|
Artie commented as follows on the EDCD Discord:
|
|
Generally waiting for more feedback but certainly understand not requiring starpos in this case. From Matt G:
|
Interesting. So you might get the codex event once per star system rather than once per region or once per body? I imagine consumers might still want to know if multiple bodies in the system have a particular variant / species / genus. |
|
LCU No Fool Like One has expressed preferences for:
Orvidius also already has a processor for the event and may have the same preference? |
|
I support bodyid over body due to making the EDDN schemas orthogonal. I'm not happy with getting lat/long from status.json - status.json is not synchronous with the journal entries, and crosses systems in EDD. |
|
In my opinion, latitude and longitude should probably be optional at best.
EDDI likewise process status.json asynchronously. Because EDDI doesn't keep a detailed history of the state of status.json, we would not be able to support lat/long for any events which aren't sent in real time. For journal events sent in real-time, if the status service encounters an error or becomes out of sync with journal events then there is a risk of creating a series of bad data submissions in EDDN. Clients would need to implement code to protect against this and that protection might be imperfect. Further, status.json data is unavailable to Journal Limpet and similar services which rely on CAPI journal data rather than local files. They would not be able to support a schema which requires processing status.json. If we feel like lat/long either impinge too much on user privacy or introduce too much complication / impediment for senders then we may want to drop those fields entirely. If latitude and longitude are dropped then we would only need to send once per species variant per body. Any duplication between |
|
I could just capture log types, then, if we aren't concerned about trying to detect 'completed' scans. I'm fine with making lat/long optional. So far I think I can sum up with:
|
|
I just did a quick test of this and did not get a CodexEntry event for the first bio I scanned in a system of an already-logged region bio. So I believe it may only reliably trigger for newly discovered region bios on ScanOrganic. |
I think the general consensus is to augment with SystemName, StarPos, and BodyName. Consumers will not necessarily use all values but this would provide enough info to create basic records for otherwise unknown systems and bodies.
Yes. Status.json data would need to be optional at most. We would need to establish rules for when status.json could be used. My preference would be to omit these augmentations entirely. Consumers need to make the case for keeping lat/long in if they want them. Given that bios can be scattered far around the body, is a sample size of 1-3 data points over a very small region of the body even useful?
Agreed. The consensus amongst consumers seems to lean towards Body while producers /senders would prefer BodyID. My personal preference is to use BodyID for clarity.
Not necessarily. Sample scan types are also fine. I've just pointed out that producers can de-dupe the data if lat/long are not required. |
|
Right now my code uses "Body" since that's what has been provided. I can update my code to accept either, then it wouldn't matter at that point. For the most part, I've really only cared about the system, since I plot maps that have, at best, 10ly resolution. ;) |
Tkael
left a comment
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.
Thank you for your work on this @Silarn. I've commented on the specific items that I think we ought to refine before we finalize this. o7
|
Meant to make changes sooner, but the Oblivion Remastered release is occupying some of my time. Will make these changes now as per the review. |
|
Was this still waiting on anything? |
Tkael
left a comment
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.
There's a mismatch between the readme.md and the schema. Per the readme.md the StarSystem and StarPos augments are required but per the schema they are not allowed.
|
Sorry, I'll update this shortly. Forgot to check back on this. |
|
I believe the schema and readme should be in line with discussion now. For some reason I'd forgot to add SystemName to the schema, StarPos was there but not listed as required. These are both listed and required in the schema now.
There was some initial discussion about whether to include StarPos, but I think we decided on keeping it. I believe the rest should be correct now. |
While CodexEntry does log many bios, since it gets triggered on any new codex entries when using the sampler tool, it does not trigger for any species already logged in your current region (or possibly system). This leaves gaps in the coverage for known bio locations.
Currently, 'Analyse' ScanType events should be skipped due to problems in the way these are reported by the game. Because it only logs the 'Analyse' scans after a short delay (animation) - the tool can be put away before this completes and may only generate the journal event when reequipped in a completely different system and body.
'Variant' data should be reported if present but is not required as this was only added to the event in later journal versions.
Finally, while the event contains the current Body ID, it is reported using the key 'Body' which I've renamed to 'BodyID' to comport with most other events. I also assume this should be corroborated with 'BodyName' like some other schemas.
Let me know if there are any questions / concerns.