Removing UnityEngine reference inside com.rlabrecque.steamworks.net.asmdef

[coty-crg]

To improve compile times, I've removed the UnityEngine from the base of the plugin. The one reference was simply to pass along exception logging, so I've instead replaced it with a static delegate. In SteamManager.cs, you can hook into this delegate in OnEnable()/OnDisable().

I've attached some before and after examples of how compile times an be affected by this dependency chain change.

[coty-crg]

Opened a pull request for the corresponding change for SteamManager.cs here: rlabrecque/Steamworks.NET-SteamManager#7

[rlabrecque]

I like this A LOT; I wonder if it's possible to avoid breaking compatibility with this though as It's not really expected that people have to update or change their SteamManager (or equivalent). Might need to think on this one for a minute

[coty-crg]

Ah, yeah.. I can see how that's a bit of a pain. It's too bad the SteamManager script itself isn't also in a package manager repo, so you could just have the packages in sync via dependencies.

[rlabrecque]

Yeah, though it's explicitly not just because it was originally just a place to start from as 'user code'. As the initial users each had unique ways of handling their 'managers', like some didn't have or want that type of code as a MonoBehavior aaat all. Being unopinionated here is the only reason why Steamworks.NET is still around at all.

Trying to use Steamworks.NET via the package manager again this last week for the first time in a year or two was somewhat frustrating because of this though; so I was already having some thoughts about how we could ship SteamManager but with an option (somehow) to copy it into the users Scripts directory (and then also allow people to update their copy if they are using an untouched version). I'm not even quite sure how such an option would interact with this specific change too.

Some other options we have:

1. Warn somewhere, somehow, if that delegate isn't bound?
2. Do this in two distinct steps, adding the new callback, and then removing the UnityEngine reference in the future?

[coty-crg]

Option 2 makes sense to me! Gives people ample time to eventually go to the callback system. I don't want to cause any issues with this pull request, I know how frustrating breaking changes can be.

Having the ability to simply copy in the manager seems good too, although personally I never actually edit that file. I assumed we were meant to inherit from it, and then make any changes necessary.

[rlabrecque]

Maybe this code initially for option 2? Also Let's us get the SteamManager change in ASAP too.

		public delegate void OnSteamException(Exception e); 
		public static OnSteamException OnSteamExceptionEvent; 

		public static void ExceptionHandler(Exception e) {
			if (OnSteamExceptionEvent != null) {
				OnSteamExceptionEvent.Invoke(e); 
			}
#if UNITY_STANDALONE
			else {
				UnityEngine.Debug.LogException(e);
			}
#elif STEAMWORKS_WIN || STEAMWORKS_LIN_OSX
			else {
				Console.WriteLine(e.Message);
			}
#endif
		}

[JamesMcGhee]

Trying to use Steamworks.NET via the package manager again this last week for the first time in a year or two was somewhat frustrating because of this though; so I was already having some thoughts about how we could ship SteamManager but with an option (somehow) to copy it into the users Scripts directory (and then also allow people to update their copy if they are using an untouched version). I'm not even quite sure how such an option would interact with this specific change too.

Unity Package Manager has a concept of "Samples"
Once you install a package you can select it in UPM and you will see a "Samples" tab or option depending on your version ... that is a list of things you can "import" that go into your asset folder, not your Packages folder

It is where you would put things like that
This can be optionally imported by users and if they choose to import them they come into the Asset folder, so perfect place for things like your tests, samples, etc.

That said SteamManager.cs as it is I think should be looked at a bit.
SteamManaager.cs is fine as a bit of sample code meant for people to read and learn from but not use in production.
It has a few gatcha issues when its just blindly flung into a project and used like basically 99.99% of Unity users will do

What we do with our samples to drive home that its a sample for learning and not production marks them Obsolete
This will cause Unity Editor to put a yellow comment at the top of the component in the inspector window and show a Warning in the console. It will still compile and work of course it just makes it real obvious that maybe they should look into it and adjust for their needs.

And or you could brush up the SteamManager to not act like a singleton making it a bit safer for when that 99.99% just copy and pastes and "ah sure its grand" the whole thing :)

[ProbablePrime]

Hello, we (Yellow Dog Man Studios), have a fork of this where I just came across a similar issue.

In this case this: Yellow-Dog-Man@fda7fd5

was the methodology that I chose.

Once I had completed it, I came over here and found this PR. I wanted to provide my solution as a reference.

With my methodology, consumers who want the new behavior can opt in with:

// Overwrite the default exception handler, to control logging flow.
CallbackDispatcher.ExceptionHandler = SteamAPIExceptionHandler;

But users who are happy with, or do not want the breaking change do not have to make any changes. This preserves backwards compatibility.

Ultimately, either solution will work for my requirements, but I wanted to provide some information on our solution in case it helped the discussion, thanks!

[rlabrecque]

I like that a lot, it looks like a simple change that moves us closer to this?

Maybe the next step could literally just be some sort of #if to exclude the unity reference?