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

Refactor to a more encapsulated package #389

Open
pbhogan opened this issue Nov 19, 2020 · 16 comments
Open

Refactor to a more encapsulated package #389

pbhogan opened this issue Nov 19, 2020 · 16 comments

Comments

@pbhogan
Copy link
Collaborator

pbhogan commented Nov 19, 2020

Hey Riley,

I'm currently looking more seriously into adding SteamInput support to InControl. To do that I would need to add a dependency on Steamworks.NET but that is currently problematic.

I'd like to suggest that Steamworks.NET be restructured to be more neatly and easily dropped into a project, or pulled in through the Package Manager.

I'd be happy to help with this process, and even do most of it if you don't have time!

Here's what I recommend:

  1. Move files around into the file structure that Unity packages have standardized on:
Steamworks (root folder)
- Editor (current Editor/Steamworks.NET contents goes here)
- Runtime (current Plugins/Steamworks.NET contents goes here)
- Plugins (rest of plugins goes in here)
  1. Add assembly definitions in Runtime (Steamworks) and Editor (Steamworks.Editor) above. Plugins does not get an assembly definition. This allows setting a reference in InControl's assembly definition along with a preprocessor that gets defined when the package is present.

  2. Add a package definition which would allow Steamworks.NET to be more easily installed via tarball or git.

  3. Fix the native plugins metadata. Currently they have a few errors and have to be manually fixed, sometimes with a bunch of Unity restarts involved. For InControl, I have a development editor script that sets up the plugins properly. I run this in each version of Unity before I package and submit for that Unity version to the Asset Store. Older versions of Unity have slightly different import configurations (and even bugs!) that aren't compatible with later versions. But it would be good to focus on setting it correctly for 2019 LTS onwards (and going forward, set for maybe the last LTS or if there's a serious conflict, for latest release Unity).

The Standalone folder throws a bit of a wrench in the works—I'm not actually sure if it's safe to just have that hanging out in the package folder, but I can test that. Worst case, that folder could just be a ZIP archive since its contents isn't really super important to version control in the sense of line by line code, but only as a whole.

@pbhogan
Copy link
Collaborator Author

pbhogan commented Nov 19, 2020

Hah, actually as I dig into it, I see you're already doing 4. in RedistInstall, but I guess it's just not setting things correctly.

Maybe just time to update all of this to assume at least 2018 or 2019 LTS.

@rlabrecque
Copy link
Owner

I was just thinking about this last night.

I agree it's time to drop support for older versions of Unity. What versions does InControl support at this point? Has Aras released editor usage numbers recently? Currently Steamworks.NET still supports back to 4.7.

Some thoughts on each point:

  1. I think that stems from older versions of Unity requiring the Assets/Editor/, Assets/Plugins/ structure. IIRC support for Assets/*/Editor/ (etc) got added around Unity5 or 2017? My only other aversion to reorganizing previously was to maintain the consistent upgrade flow. If we're dropping support for older Unity and reorganizing for other reasons let's go all in with Point 3 as well. 👍

  2. I don't fully understand without digging in more but that sounds great to me!

  3. @gekidoslair Has this fork with package support over here: https://github.com/PixelWizards/Steamworks.NET/tree/package We should probably work with him to merge this in as well. Unity Package support is definitely a priority for me with this.
    One additional consideration is SteamManager, it sits in a separate repo and gets pulled in to .unitypackage releases, I guess we should probably move that over here for this?
    He seems to have it working well enough with the Standalone / Non-Unity folder still there too, which is still important.
    One actual question I have is how do code edits work with Unity Packages, is it easy enough for someone to change something like SteamManager (which you pretty much have to do) with them?

  4. This stuff is an absolute mess. Every major release of Unity (read: 4.7/5/2017.x/2018.x ...) handles native plugins completely differently and has historically had major bugs in native plugin importing. I think this is probably a completely separate task to go back through with RedistInstall disabled on what ever versions of Unity we decide to support, and see what we need at this point.

I think 1 & 3 are related and need more discussion, 2 we can probably start on ASAP if it isn't blocked by any others, 4 we can start on as soon as we select the minimum Unity version.

@pbhogan
Copy link
Collaborator Author

pbhogan commented Nov 19, 2020

InControl is technically code compatible back to Unity 4.7 or 5 or something like that. But what I decided to do some time ago was only support and submit with LTS versions... so 2018 LTS is the current supported version.

The way I work is I have a project for each Unity version (2018, 2019, 2020, 2021, etc.). I work on the base (lowest version) and have a script to rsync the code to the others. I'll open and test on the various versions as I go, and then I submit with each version too.

InControl has historically had version dependent stuff going on, however, since 2017 LTS it's all been basically identical. I've actually decided recently to move my base up to 2019 LTS just because I don't think that many are still using 2018 and if they are they can still use InControl with it anyway. Consoles used to be the big hold up, but it seems like Unity has helped streamline things to the point that they're pretty good at keeping up with current Unity releases.

All that said, I think 2018 LTS is an easy baseline for Steamworks.NET or 2019 LTS if you want to be a little bit more aggressive, and I don't think you need to have multiple projects like I do... InControl is a different beast. Honestly, other than plugin imports, you have nothing version specific in here, right?

  1. I think this is a no-brainer. InControl hasn't supported the old plugin system in years and it's been fine. I totally understand not wanting to break the upgrade path—I'm fastidious about that with InControl and it's held me back from certain things, but if you're going to do anything then a clean break is the way to go with a nice prominent warning in the docs about deleting the old code before importing updates. I think most users understand doing that now. The experience going forward will be so much better for everyone.

  2. This is actually really easy to do. It's two files made in the Unity editor and done. You still occasionally run into someone who might get a compilation error due to a missing reference, but the way it's implemented in Unity is quite easy going. If their code has an assembly definition, then they need to add a reference to the things it uses. If not, they don't have to worry about it and it just works. Unity creates a default assembly that references everything in the project. So for 99% of people it just works as expected. A simple note in the getting started guide would pretty much cover it for those who need more info.

  3. A merge isn't necessary. If you move to an encapsulated structure (everything under one root folder) then all you need is a single json file in the root. The structure in that project is not what I'd choose, personally, and I don't think it's directly Package Manager compatible as is. I need to do a little testing on this. I'm going to move SteamManager discussion to 5.

  4. I digged in a bit more to the code, and yeah, this just needs to get cleaned up. Personally, I don't like automagical things running, especially when it messes with Unity's importing, so I'd prefer to see this under a Steamworks menu that can get manually triggered. There's actually a couple other potential things that could go under there related to setting the App ID and locking/unlocking Steam to it, but that's another matter.

  5. This can be handled easily enough. You can put it in a .unitypackage and then it's possible to put a button in the Package Manager that imports it. Packages do this for example code sometimes. But I'd like to hear more about what kind of modifications are typically needed and whether there is a better way to refactor SteamManager to make it extensible and always present in the codebase, though not necessarily required. One of the things I'd need to be able to do, for example, is call the equivalent of SteamManager.Initialized from InControl. So that get a bit messy if I can't guarantee it exists. But I could work around it if I needed to, especially if we added an interface (ISteamManager) and default implementation included in the package, and then InControl could use reflection to find compatible custom SteamManagers which can be selected in the InControl settings UI. I'd just like it to be as out-of-the-box as possible. :)

I've actually got 1-3 in my test project already. Takes just a couple minutes to shuffle things around.

I'm not sure how you want to proceed with this. If you like, I could make a separate repo (as a temporary sandbox) and kind of restructure, iterate and test things there without messing with this repo until we figure it all out?

@rlabrecque
Copy link
Owner

Yeah, let's get some PR's up for some of these for sure 👍

I can chat this weekend or on Wednesday if you want to sit down and hash out some of this stuff.

@gekidoslair
Copy link

gekidoslair commented Mar 10, 2021

Sorry for the lag, but I've already done the re-structuring of the package in my fork fwiw (in the 'package' branch):
https://github.com/PixelWizards/Steamworks.NET/tree/package

I'm not 100% sure that it's up to date with the source repo, but should be fairly straightforward to update with whatever changes if desired.

@pbhogan
Copy link
Collaborator Author

pbhogan commented Mar 18, 2021

Yeah, same. I hit a roadblock with SteamInput and moved on to other things. I'll circle back at some point. :)

@rlabrecque
Copy link
Owner

rlabrecque commented Mar 22, 2021

Progress has been made on this 👍

https://github.com/rlabrecque/Steamworks.NET/tree/unity-package

Should check off 1, 2, 3 and a bit of 4 (by including the meta files). Based largely on Mike's fork!

I agree that it's time to only start supporting LTS historically, and then probably what ever the latest non LTS release is.

@rlabrecque
Copy link
Owner

Steamworks.NET can now be installed as a Unity package 🎉

Unity Package path: https://github.com/rlabrecque/Steamworks.NET.git?path=/com.rlabrecque.steamworks.net

image

Validation remains.

@wagenheimer
Copy link

Please update the documentation to indicate the availability from the Package Manager.

@fghl
Copy link

fghl commented Jun 2, 2021

@rlabrecque is there a reason the install docs haven't been updated to the git URL yet? Is it safe to use? If you'd like someone to do it for you just point me to it and I'll open a PR.

@rlabrecque
Copy link
Owner

The key missing piece right now is what to do with SteamManager. That's user editable code and the UnityPackage releases have it included for convenience. We likely need to include it in this repo, and then have a script that copies it out of the repo into Assets/ or such?

In the meantime you can find SteamManager here, you'll need to download it and copy it into your project (or write a similar version yourself):

https://github.com/rlabrecque/Steamworks.NET-SteamManager/blob/master/SteamManager.cs

This is safe to use other than needing to do this extra manual step.

@fghl
Copy link

fghl commented Jun 3, 2021

Ah thanks @rlabrecque, I copied it from the MLAPI SteamP2P transport.

That's a hard one. Perhaps you can find a place in the Unity inspector to place a creation button?

The Unity Input System does this:
image

And URP has a similar UI feature if I remember correctly.

@rlabrecque
Copy link
Owner

Thanks yeah, I'll investigate something like that, could also be a Menu dialog to copy it out of the package into the Assets folder

@wagenheimer
Copy link

I'm using the Package Manager Version, but my build failed to run on Mac M1. Is there anything I can try?

Termination Reason:    DYLD, [0x9] <unknown>

Application Specific Information:
dyld: launch, loading dependent libraries
DYLD_LIBRARY_PATH=
DYLD_INSERT_LIBRARIES=/Users/cezarwagenheimer/Library/Application Support/Steam/Steam.AppBundle/Steam/Contents/MacOS/steamloader.dylib:/Users/cezarwagenheimer/Library/Application Support/Steam/Steam.AppBundle/Steam/Contents/MacOS/gameoverlayrenderer.dylib:/Users/cezarwagenheimer/Library/Application Support/Steam/Steam.AppBundle/Steam/Contents/MacOS/gameoverlayrenderer32.dylib
DYLD_FALLBACK_LIBRARY_PATH=/Users/cezarwagenheimer/Library/Application Support/Steam/Steam.AppBundle/Steam/Contents/MacOS:/Users/cezarwagenheimer/Library/Application Support/Steam/steamapps/common/Storm Tale 2:/Users/cezarwagenheimer/Library/Application Support/Steam/steamapps/common/Storm Tale 2/bin:/usr/local/lib:/lib:/usr/lib

Dyld Error Message:
  could not load inserted library '/Users/cezarwagenheimer/Library/Application Support/Steam/Steam.AppBundle/Steam/Contents/MacOS/steamloader.dylib' because no suitable image found.  Did find:
	/Users/cezarwagenheimer/Library/Application Support/Steam/Steam.AppBundle/Steam/Contents/MacOS/steamloader.dylib: no matching architecture in universal wrapper
	/Users/cezarwagenheimer/Library/Application Support/Steam/Steam.AppBundle/Steam/Contents/MacOS/steamloader.dylib: stat() failed with errno=1

@wagenheimer
Copy link

I changed the Architecture from Intel 64-bit + Apple Silicon to only Intel 64 bit and now it worked!

Will Apple Silicon be supported?

@yaakov-h
Copy link

Apple Silicon support is dependent entirely on Valve. See #422 and the Steamworks discussion linked there.

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

No branches or pull requests

6 participants