Skip to content
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

NetCore3.1 port #299

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

NetCore3.1 port #299

wants to merge 1 commit into from

Conversation

vbousquet
Copy link
Contributor

This is an early commit, not ready for merge.

Everything build fine and DMDExt tests are fine, but:

  • DMDDevice fails from VPinMame (it runs ok through direct dll access).
  • Installer is untested.

This is an early commit, not ready for merge. Everything build fine and DMDExt tests are fine, but:
- DMDDevice fails from VPinMame (it runs ok through direct dll access).
- Installer untested.
@vbousquet vbousquet changed the title NetCore3.1 ^port NetCore3.1 port Jun 1, 2021
@freezy
Copy link
Owner

freezy commented Jun 2, 2021

Awesome! I'll test it later tonight. Thanks already!

@freezy
Copy link
Owner

freezy commented Jun 2, 2021

Tested, and seems to run fine both on command line and through VPM?

image

I'm wondering about the packing though. Currently as you noticed we're using Fody/Costura to pack the dependencies into a single file - How does that work with dotnet core? Last time I tested the built-in feature, it packed the entire framework into it, resulting in a 70mb+ file size. Here it only increased a little (~2-3mb).

@vbousquet
Copy link
Contributor Author

For VPM, it is not working here. I have upgraded PinMameTest to perform some more test (access by project reference / DLL loading / through COM) but did not have time to go further.

For the packing, there are 2 things;

The installer has not yet been updated to trigger the publish command and use its output. So this is pending too. For the time being, you can test by right clicking on DMDExt in VS and select 'Publish' with one of the provided profiles.

@freezy
Copy link
Owner

freezy commented Jun 2, 2021

Okay, great. Let me know when I need to test more.

@freezy
Copy link
Owner

freezy commented Jul 6, 2021

Hey @vbousquet is this ready to test and merge? Looks like AppVeyor shortened the life time of their artifacts to one month only so people started sharing betas though Dropbox and whatnot.

So a new version soon would be good. You think this is already mature enough?

@vbousquet
Copy link
Contributor Author

I have very little free time at the moment, so I was not able to work again on it. I don't think it is ready for merging for the point I identified previously:

  • dmdevice.dll seems to not be packaged correctly (I had situation where it would fail with a FileIO issue, I guess linked to the failure of loading unpacked dll),
  • the installer has to be adapted to the new publish process.

Sadly, I don't think I will have time to work on it in the coming weeks so I think this should wait a bit more (unless you want to tackle with it). I don't think this is really a problem; a release with all the already merged improvments would be good anyway. This one is mainly to prepare for future improvments and eventually porting for VPE.

@freezy
Copy link
Owner

freezy commented Jul 6, 2021

Alright, no worries, just wasn't sure whether an action from my side was needed. I might push out a release before this goes in then. Cheers!

@freezy
Copy link
Owner

freezy commented Jan 25, 2022

Hey, any update? We'll be dropping a new version soon, having this finalized would be awesome. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants