SabreCSG icon indicating copy to clipboard operation
SabreCSG copied to clipboard

Using SabreCSG as a package

Open andybak opened this issue 6 years ago • 15 comments

Hi,

I'm doing something a little unusual and it breaks with SabreCSG. I'm not familiar enough with "The Unity Way" to know whether it makes more sense for me to patch SabreCSG to get round this or whether what I'm doing is going to be break so many 3rd party assets I should rethink my whole strategy.

Inspired by how Keijiro has been using the Unity 2018 packages directory to keep 3rd party code separate from his project code I've devised a workflow that involves package manifests and git submodules. (I got more inspiration from LotteMakesStuff here: https://gist.github.com/LotteMakesStuff/6e02e0ea303030517a071a1c81eb016e )

Basically - I fork any repo I want to use in my project, add a package.json and use it as a submodule which lives in the project's Packages directory.

See https://gist.github.com/andybak/f7573d2985aa6d1b905ed6519cb6a343 for a step by step guide I was hoping to put out for others.

I had dreams of grandeur. Others might adopt this workflow and it might get official Unity blessing/tooling support. I also really really wanted a tidy Assets directory :-(

This looked like it was going to work beautifully until I added SabreCSG to a project.

SabreCSG does lots of stuff where it loads shaders and textures via a hardcoded path. Both Unity's Resource.Load and the code in Sabre's LoadObject seem to break once I move SabreCSG into Packages.

Now - this all seems fixable - and maybe in a way that might be worth a pull request upstream - but I wanted to ask if you think this is a fool's errand?

I've tried simpler Unity assets and it works fine. It's just code that uses explicit paths to certain resource types that seem to break.

andybak avatar Jun 18 '18 18:06 andybak

We have pinpointed the problem and are looking into a solution.

By the way how is this repository supposed to work? What if people open issues or PRs there? Are you going to keep it updated manually? Can't you link to this repository if we include the package.json?

Henry00IS avatar Jun 19 '18 10:06 Henry00IS

By the way how is this repository supposed to work?

My initial assumption was that I'd be maintaining my forks. I was planning maybe to write a quick shell script that pulled in upstream changes on all the repos at once. (This also assumes that the only change in my fork is the inclusion of package.json)

If upstream actually is happy to add their own package.json then I'd probably remove my fork. I'd have to think a bit more deeply about this if other people start using my approach and there's already people using my repo url. I think Github allows repos to redirect automatically - or maybe just maintaining a fork indefinitely isn't so bad if pulling upstream changes is fully automated.

I could sidestep this by asking upstream repos first - and only forking if they aren't interested.

andybak avatar Jun 19 '18 11:06 andybak

Hey andybak,

I am not sure that you check Discord often enough so I will post some recent findings here thanks to @Kerfuffles.

We have to wait a Unity 2018 version before introducing package support because there's an issue with the AssetDatabase class that we rely on. It refuses to look anywhere except for the 'Assets/' path so we can't find some important resources in 'Packages/'. While there are alternative methods to fix this issue it will require a significant amount of work and rewriting chunks of SabreCSG.

Here's Kerfuffles report:

So I'm finding an issue with fixing #155. AssetDatabase and Resources are completely ignored in 'Packages/', and additionally, scripts from those packages are not included or visible in the editor. They have to be assigned from scripts inside the package. I don't think that's intended behaviour. So after doing some reading I found here: https://github.com/JetBrains/resharper-unity/issues/476 that it looks like Unity will support using AssetDatabase on the 'Packages/' directory in the future. Possibly in 2018.2 or 2018.3. It's possible to force it to work by moving the assets that are loaded, into the 'Assets/' folder. I tried using System.IO commands to get paths and such, but AssetDatabase refused to use those paths, even after replacing path characters. It expects a local path starting with 'Assets/' so, for now, out of the box, SabreCSG has partial package support until Unity tech can fix their internal code to accept 'Packages/' as a valid asset path. At least, if SabreCSG didn't need to rely on local script location... a special case, which unfortunately will not work.

Henry00IS avatar Jun 23 '18 08:06 Henry00IS

@Henry00IS Thanks for the update. I'm curious if this does work in 2018.2 - if it's not already there then it's pretty late in the cycle for it to appear. We're on b9 already so I'm guessing it's a couple of weeks from a release.

If not I'd guess/hope it's slated for 2018.3

The good news for my general approach is that this doesn't seem like it will affect the majority of code that I would want to reuse. I can always fall back to having some code in my Assets directory where necessary. The real win is keeping most of it hidden away.

andybak avatar Jun 23 '18 10:06 andybak

I have a comment on the assetdatabase.find page that specifies if you include "Assets" and "Packages" as your search terms you can find in both. They changed the search to not include packages in 2018.2.0b7 so before then the normal search works fine.

On Sat, Jun 23, 2018, 8:49 PM Andy Baker [email protected] wrote:

@Henry00IS https://github.com/Henry00IS Thanks for the update. I'm curious if this does work in 2018.2 - if it's not already there then it's pretty late in the cycle for it to appear. We're on b9 already so I'm guessing it's a couple of weeks from a release.

If not I'd guess/hope it's slated for 2018.3

The good news for my general approach is that this doesn't seem like it will affect the majority of code that I would want to reuse. I can always fall back to having some code in my Assets directory where necessary. The real win is keeping most of it hidden away.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sabresaurus/SabreCSG/issues/155#issuecomment-399664894, or mute the thread https://github.com/notifications/unsubscribe-auth/AU8jxcHg0eDPWHCytatzCj22DyGUfXFqks5t_h1NgaJpZM4UsP9a .

vertxxyz avatar Jun 23 '18 22:06 vertxxyz

@vertxxyz That's worrying. Is there an actual bug tracker for Unity or is it "post something in the forums and hope"?

andybak avatar Jun 24 '18 10:06 andybak

They did it on purpose now there's a filter on project search. I've got one bug reported about having no filters on object fields search, but it may have been fixed already. I just find this stuff out myself because I'm working with the latest beta :P, and fix it by throwing a bunch of ideas at it or sometimes going through the decompiled code to see if there's a new implementation. Very irritating when they change stuff I rely on, it seems to happen every update but that's the beta life.

On Sun, Jun 24, 2018, 8:14 PM Andy Baker [email protected] wrote:

@vertxxyz https://github.com/vertxxyz That's worrying. Is there an actual bug tracker for Unity or is it "post something in the forums and hope"?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sabresaurus/SabreCSG/issues/155#issuecomment-399745239, or mute the thread https://github.com/notifications/unsubscribe-auth/AU8jxTCuUi2ZeVyRcJILoIWH-BPamCLyks5t_2aggaJpZM4UsP9a .

vertxxyz avatar Jun 24 '18 10:06 vertxxyz

So. I'm using the searchInFolders parameter and getting this message back in return, on 2018.1.5f1

image

string[] guids = AssetDatabase.FindAssets("CSGModel t:Script", new string[] { "Packages", "Assets" });

found in CSGModel, line 1433. It seems to be failing to find Packages entirely, as if it doesn't exist.

nukeandbeans avatar Jun 25 '18 07:06 nukeandbeans

They only changed it in 2018.2.0b7, so before then the search shouldn't need to contain any further parameters and functioned normally including packages for me, but I didn't do too much testing apart from finding they broke my searches on that version, and fixing it via that code.

On Mon, Jun 25, 2018, 5:02 PM Daniel Cornelius [email protected] wrote:

So. I'm using the searchInFolders parameter and getting this message back in return, on 2018.1.5f1

[image: image] https://user-images.githubusercontent.com/355687/41834859-3fffdd60-781b-11e8-894f-3e8090e99a89.png

string[] guids = AssetDatabase.FindAssets("CSGModel t:Script", new string[] { "Packages", "Assets" });

found in CSGModel, line 1433. It seems to be failing to find Packages entirely, as if it doesn't exist.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sabresaurus/SabreCSG/issues/155#issuecomment-399852335, or mute the thread https://github.com/notifications/unsubscribe-auth/AU8jxR77268VsFK9ivWD34KDDRFVXHIcks5uAIr5gaJpZM4UsP9a .

vertxxyz avatar Jun 25 '18 07:06 vertxxyz

I tested both using searchInFolders and without. Even tried putting the direct path. It refuses to use Packages/ at all. Mirroring the behaviour in the asset browser.

Maybe this is a bug? Or lack of feature therein?

nukeandbeans avatar Jun 25 '18 09:06 nukeandbeans

Hrm, damn, I mustn't have been using Find back in 2018.1 - does using a direct AssetDatabase.Load("Packages/com.example.example/...") work? I've just tested 2018.1.0b12, so hopefully it should work in 2018.1.5f1 You should be able to load regardless of location if you append the search with the "Packages" directory. Basically take the path from whatever Unity takes you to in the project window after selecting a package script.

Sucks that the search is nonexistent in earlier versions... I'll have to do my own testing to find out when and ammend my own codebase ):

On Mon, 25 Jun 2018 at 19:28, Daniel Cornelius [email protected] wrote:

I tested both using searchInFolders and without. Even tried putting the direct path. It refuses to use Packages/ at all. Mirroring the behaviour in the asset browser.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sabresaurus/SabreCSG/issues/155#issuecomment-399889766, or mute the thread https://github.com/notifications/unsubscribe-auth/AU8jxbswtUGVRUGhf_iSJrowhacNydHTks5uAK0igaJpZM4UsP9a .

vertxxyz avatar Jun 26 '18 00:06 vertxxyz

Just tested a direct load like that and got nothing. Using both the package name and the directory path.

nukeandbeans avatar Jun 26 '18 00:06 nukeandbeans

I've just launched 2018.1.5f1 with the latest post processing stack installed and

[InitializeOnLoadMethod] static void LoadLensDirt() { Debug.Log(AssetDatabase.LoadAssetAtPath<Texture2D>("Packages/com.unity.postprocessing/PostProcessing/Textures/Lens Dirt/LensDirt02.png")); }

returns the lens dirt texture. So it should be possible. (bloody email replies don't even support markdown via an edit) https://i.snag.gy/q1JGC5.jpg

vertxxyz avatar Jun 26 '18 03:06 vertxxyz

Interestingly enough, I cannot get this to work, even with the simplest test.

using UnityEngine;
using UnityEditor;

[CustomEditor(typeof(BoxCollider))]
public class AssetTestGUI : Editor
{
	public static Texture2D TestTex
	{
		get
		{
			return m_TestTex;
		}
		set
		{
			m_TestTex = value;
		}
	}

	private static Texture2D m_TestTex;

	public override void OnInspectorGUI()
	{
		if(TestTex == null)
			Debug.Log(TestTex = AssetDatabase.LoadAssetAtPath<Texture2D>("Packages/kerfufflestudios/ALT/testtex.tga"));

		GUILayout.Label(TestTex, GUILayout.ExpandWidth(true), GUILayout.ExpandHeight(true));
	}
}

image image image

nukeandbeans avatar Jun 26 '18 07:06 nukeandbeans

Just an update from my perspective. I've abandoned attempts to create a workflow based on submodules and packages - not because of anything Unity related (I actually found most other assets worked pretty well with this system).

The sticking point was in two places: Github desktop has horrible, patchy and inconsistent support for submodules. I was really hoping this workflow might be usable by people who weren't happy with arcane command line incantations.

But even worse was the fact that git itself is still overly complex and unpredictable in how it handles submodules. If you make a mistake it's hard to fix and easy to leave your repo in a confused state if you do the wrong thing. The Stack Overflow page on "removing a submodule" is one of the most complex I've seen!)

I do sometimes shed a tear for poor old Mercurial...

andybak avatar Jun 26 '18 13:06 andybak