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

[EXILED::Events] Fix downloading NWAPI plugins with hub install #117

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

bladuk
Copy link

@bladuk bladuk commented Sep 25, 2024

Description

Describe the changes
This PR fixes the NWAPI plugin installation with the hub install command.

What is the current behavior? (You can also link to an open issue here)
hub install fetches every single asset in the plugin release and downloads it.

What is the new behavior? (if this is a feature change)
hub install will now filter assets with the -NWAPI postfix in the name.

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No.

Other information:


Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentations

Submission checklist

  • I have checked the project can be compiled
  • I have tested my changes and it worked as expected

Patches (if there are any changes related to Harmony patches)

  • I have checked no IL patching errors in the console

Other

  • Still requires more testing

@bladuk
Copy link
Author

bladuk commented Sep 25, 2024

Build actions has been failed due to GH Actions outage and should be restarted.

@VladTheCow VladTheCow added the enhancement New feature or request label Sep 25, 2024
Copy link

@iamalexrouse iamalexrouse left a comment

Choose a reason for hiding this comment

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

While I'm not too happy about the hub install command residing within the events library, I'm not too fussed.

The issue I have with this PR is the requirement for a standardized naming scheme which considering the large amount of new developers entering the EXILED plugin development scene, would be difficult to communicate easily especially when newer developers may have been used to a completely different naming schemes or none at all.

If the naming conventions will be applied web-side, I'm not too opposed to adding this change. Though I have some nitpicks which you can see below.

Secondly, with plugin downloads, is there any plans to add a validation stage for ensuring plugins install without corruption? Validation is usually used or detecting problems like a bad download which, may help avoid having corrupted plugins from potentially causing issues after install.

Thanks,
Alex

EXILED/Exiled.Events/Commands/Hub/Install.cs Outdated Show resolved Hide resolved
@bladuk
Copy link
Author

bladuk commented Sep 26, 2024

@iamalexrouse, Hello

Of course, this is a temporary hotfix to a big problem. The naming “SomePlugin-NWAPI.dll” isn't really a standard, but has been widely used by many developers since the NWAPI release. We are currently working on a more accurate and user-friendly solution to this issue.

Regarding the integrity check for the downloaded files, it's currently not planned but we'll think about your suggestion.

Thanks for the feedback anyway!

Co-authored-by: Alex Rouse <alex@leronix.net>
@VALERA771 VALERA771 merged commit 244172f into ExMod-Team:dev Sep 30, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Events
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants