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

Check short name for remote extensions #1053

Merged
merged 1 commit into from
Nov 12, 2019

Conversation

nosoop
Copy link
Contributor

@nosoop nosoop commented Jul 29, 2019

Checks the filename passed in via LoadExternal for remote extensions for same-file tests.

This fixes FindExtensionByFile not finding an existing remote extension in the case where the filepath is not equal to the filename (and subsequently loading it twice) and allows plugins to successfully load when specifying that name as a dependency.

@asherkin
Copy link
Member

What are you using the remote extension API for? As per the existing comment here we don't really expect there to be any consumers.

This change looks fine - is there any reason to check the path rather than just the short name?

@nosoop
Copy link
Contributor Author

nosoop commented Jul 29, 2019

I'm exposing some natives from RCBot2 (branch not pushed yet) while keeping it as a MetaMod:Source plugin. I'm sure there's other ways to handle it, but the first useful resource I found while working on it was this wiki entry, so I went with that.

I kept the path handling as-is for backwards compatibility since it's already there.

@KyleSanderson KyleSanderson requested a review from dvander November 6, 2019 06:28
@dvander
Copy link
Member

dvander commented Nov 12, 2019

Indeed this code is basically untested as we never really supported it. This change looks harmless though so let's take it.

@dvander dvander merged commit d6e5188 into alliedmodders:master Nov 12, 2019
@nosoop nosoop deleted the remote-ext-filename-check branch December 12, 2019 04:05
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.

3 participants