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

Fix local build & Add github action CI #1

Merged
merged 7 commits into from
Jul 22, 2021

Conversation

hanabi1224
Copy link
Contributor

@hanabi1224 hanabi1224 commented Jul 20, 2021

The prebuilt DLLs do not work for me, I tried to build it locally but found it does not build out of box, so I made a fix and contribute it back, also add a CI job. I've verified the output DLLs and they all work properly with my local BIE 5.4.12 Let me know your concerns, thanks!

P.S. here is the CI result in my fork

Copy link
Member

@ManlyMarco ManlyMarco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR, but it looks like it breaks the current workflow and some of the requirements.

  1. The plugins have to target .Net Framework v3.5 because they are used in games that don't support v4.x dlls
  2. Build output is no longer present inside a single bin folder and the release script does not work anymore (all plugins have to be placed in separate .zip files with a correct directory structure inside, with the plugin version in the .zip filename, this is because often only one or two plugins are useful in a game, and some might even break it).
  3. File and assembly versions are not set to the plugin version, which can cause problems with apps managing plugins, and more importantly they make it not possible to see the plugin version by hovering over the dll or checking its properties (this is a requirement to reduce end user confusion and support tickets).

You mentioned not being able to build the project after cloning, I assume it's because the required game dlls are not present in the repository and you didn't copy them into the lib folder. The dlls are not included because we have not decided how to handle these files. This could be easily fixed without any changes to csproj files by using a custom nuget feed that has stripped versions of all of the required dlls like https://github.com/IllusionMods/IllusionLibs/ (it's meant for modding games by a specific publisher but it would work fine, ideally we would use a separate bepinex nuget feed instead).

You can join the BepInEx discord server if you'd like to discuss this https://discord.gg/th9JFAfsvn

@hanabi1224
Copy link
Contributor Author

hanabi1224 commented Jul 21, 2021

@ManlyMarco Thanks for ur feedback.

The plugins have to target .Net Framework v3.5 because they are used in games that don't support v4.x dlls

Just downgraded netfx to 3.5 by downgrading Mono.Cecil to 0.10.x.

Build output is no longer present inside a single bin folder and the release script does not work anymore (all plugins have to be placed in separate .zip files with a correct directory structure inside, with the plugin version in the .zip filename, this is because often only one or two plugins are useful in a game, and some might even break it).

It actually does, if you look at the build artifacts in my CI result, it produces all-in-one archive and per-plugin archives

File and assembly versions are not set to the plugin version, which can cause problems with apps managing plugins, and more importantly they make it not possible to see the plugin version by hovering over the dll or checking its properties (this is a requirement to reduce end user confusion and support tickets).

Have restored those AssemblyInfo.cs files

Copy link

@ghorsington ghorsington left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR!

I added some notes in addition to what @ManlyMarco already said. Some of these should be addressed, while some of these are optional notes. I noted which of these are optional. Please review and either address or comment on my notes. Feel free to ask if you have any questions.

BepInEx.InputHotkeyBlock/InputHotkeyBlock.cs Outdated Show resolved Hide resolved
Directory.Build.props Outdated Show resolved Hide resolved
BepInEx.MessageCenter/Properties/AssemblyInfo.cs Outdated Show resolved Hide resolved
Directory.Build.props Outdated Show resolved Hide resolved
@hanabi1224
Copy link
Contributor Author

hanabi1224 commented Jul 21, 2021

@ghorsington I've addressed all ur comments, plz take another look

@ManlyMarco ManlyMarco merged commit 67e51e9 into BepInEx:master Jul 22, 2021
@hanabi1224 hanabi1224 deleted the fix-build branch July 24, 2021 14:18
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.

None yet

3 participants