mcpiroman / UnityNativeTool

Allows to unload native plugins in Unity3d editor
MIT License
184 stars 19 forks source link

Add upm support #14

Closed rogerbarton closed 4 years ago

rogerbarton commented 4 years ago

Users can then nicely add the repo as a submodule in the Packages folder and so get updates.

If users do not want to use upm, e.g. if they are on an older version, it should still work.

(The "revision": "6c6b..." is currently the lastest commit on the master branch)

mcpiroman commented 4 years ago

Well, admittedly, I've never heard of upm, but it looks really cool. (I guess it works similarly to other package managers I know ;)). So yes, thanks for bringing that in. The only thing I'm worried about are the .asmdef files. Are they necessary? Because as you pointed out, they may interfere with assembly resolution (and for this little code there is here, this seems like little overkill?). I'm nevertheless not familiar with that solution, so I'll rely on your words.

rogerbarton commented 4 years ago

Yes upm is based on npm (it's been around since 2018.1 I believe). From what I know the you need an asmdef per package and a separate one for any editor code, which we have here. It is overkill to have them but it doesn't work without them.

rogerbarton commented 4 years ago

a5fd4553cbb908905af607886c99a39c318b4a20 also links to https://github.com/mcpiroman/UnityNativeTool/issues/17#issuecomment-605617438 and implements the whitelisting of assemblies, by their name (without extension). By default Assembly-CSharp and Assembly-CSharp-Editor are included. Our two asmdefs are always included, and so searched for the attributes. So #17 should be resolved by this.

(042c4aa383876d43b3eb6a52a6881029d27fef22 and 06abb19cdb524cd1cc17ae1f627f28152490437c) I've now adapted the GUI for selecting the assemblies, it should be quite similar to before with a dropdown. I've slightly modified how _allKnownAssemblies is calculated, so Unity assemblies are ignored (theres a IGNORED_ASSEMBLY_PREFIXES string[] for exact control). The Find dropdown allows you to select from this list. image

rogerbarton commented 4 years ago

Added some headers and reordered. Any suggestions? image

mcpiroman commented 4 years ago

Added some headers and reordered. Any suggestions?

Yea, I very like that,, maybe because I have thought about sth like this either :).

Generally, please let me know when I can review this. Btw Github allows you to declare a PR as preview, idk if they have already enabled marking existing ones as preview, which was requested a long time ago.

rogerbarton commented 4 years ago

Ah I didn't know about the marking as preview, I'll do it in the future. This PR and #15 aren't ready for review yet. I'll tell you when they are...

rogerbarton commented 4 years ago

Once #15 is merged, this PR is also ready for review.

mcpiroman commented 4 years ago

Thanks

rogerbarton commented 4 years ago

Thanks for reviewing!! I hope its not too much of a pain. I've got two small PRs left and then that should be it :D