Skip to content
This repository was archived by the owner on Dec 21, 2024. It is now read-only.

Update for EDMC 5.0 features and compatibility #10

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

Silarn
Copy link

@Silarn Silarn commented May 25, 2021

  • Current with 5.0.2
  • Ported new journal entry types for Odyssey
  • Stripped down core plugins to generate ship/system/station URLs
  • Many missing features

* Current with 5.0.2
* Ported new journal entry types
* Stripped down core plugins to generate ship/system/station URLs
* Many missing features
@Silarn
Copy link
Author

Silarn commented May 25, 2021

This should address #8 and #9.

I wouldn't be surprised if there are some small bugs lingering, it was a fairly extensive upgrade of the core EDSM code.

This does require some tweaks to the core journal parsing in the monitor.py file. Since EDMC 5 wants to read from the new JSON files in the Journal directory in addition to the main log. I imagine there some extra code that could be stripped down here and there as well, but I didn't have more time to review it all.

I'm not sure if the stations.p and systems.p files are actually needed?

@robbyxp1
Copy link
Contributor

Hmmm you should have talked to me before you wanted to push an update.

I can't just accept large updates in from people i don't know, since I'm basically sending out an executable to people and they are in effect trusting me to not write dangerous code.

A small update, i can triage, i can cope with. 35 files I can't. And you reintroduced stuff, such as the inara plugin, which on purpose should not be in EDD-EDMC. EDD deals with external interactions with those sites, the EDD-EDMC plug in is for just trying to host as best it can EDMC plugins.

If you want to update it, you'll going to have to give me small pushes which move the code towards EDMC, and talk on the EDD discord as well, so I can understand what your aiming for at each time.

Thanks though for the thought. I will gladly accept PRs which meet the criteria.

Rob

@robbyxp1 robbyxp1 closed this May 25, 2021
@Silarn
Copy link
Author

Silarn commented May 25, 2021

The core plugins are not just about pushing data to those systems, they also provide functionality to generate links to those platforms. As I briefly mentioned, I stripped out that other functionality. They do not do any kind of data upload in this update.

Honestly, I'm not sure how I could break this down into smaller pieces. You can compare most of these files to the actual EDMC release since they are, with the exception of monitor.py and eddedmc.py, copied from EDMC 5.0.2 with minor - if any - changes.

The changes to the other files are a combination of the EDD-EDMC files and EDMC 5.0.2 files.

@robbyxp1
Copy link
Contributor

Thanks for the reply.

First the the inara/edsm py's should not be there, that would reduce the count.

Then I can look at it, but not yet, I need to get EDD stable with odyssey.

cheers

@Silarn
Copy link
Author

Silarn commented May 25, 2021

I just stripped out a number of bits that were not needed:

  • Journal locking. Technically this means EDD-EDMC can have multiple copies running but there was no real purpose here.
  • Commodity CSV exporting. This was already removed from the app options, so there was no reason to keep it.
  • Any parts of the inara/eddb/edsm etc. plugins not directly related to creating links for the Station/Ship/System display.

@robbyxp1 robbyxp1 reopened this May 25, 2021
@Silarn
Copy link
Author

Silarn commented Jun 8, 2021

I mistakenly thought the hotkey manager was something plugins could use to bind their own hotkeys, but I guess it's just for EDMC's 'refresh' function.

With that in mind I will strip this component out soon. I could potentially rebase my commits since there have been a few updates at this point.

Silarn added 7 commits June 14, 2021 01:26
- Remove hotkey manager and other irrelevant settings
- Redundant file checking whether edd files already exist or not
- Better stored parsing
- Keep stored data if same commander found in current
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants