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

Disablesteamworks - Compile errors on android #149

Closed
eintopf opened this issue Feb 28, 2017 · 12 comments
Closed

Disablesteamworks - Compile errors on android #149

eintopf opened this issue Feb 28, 2017 · 12 comments

Comments

@eintopf
Copy link

eintopf commented Feb 28, 2017

Hi,

we currently try to get our game to compile for mobile (and use the DISABLESTEAMWORKS symbol). However we get serveral compile errors related to Steamworks. Example:
Assets/Plugins/Steamworks.NET/types/SteamClientPublic/CGameID.cs(23,18): error CS0246: The type or namespace name AppId_t' could not be found. Are you missing an assembly reference?`

We tried adding the #if !DISABLESTEAMWORKS stuff to the CGameID class, but then other classes start complaining. Is this a known issue? Anything we have to do beside the DISABLESTEAMWORKS define symbol?

Best regards
Bennet

eintopf added a commit to eintopf/Steamworks.NET that referenced this issue Feb 28, 2017
@eintopf
Copy link
Author

eintopf commented Feb 28, 2017

I created a pull request for that issue #150

@gresolio
Copy link

gresolio commented Mar 2, 2017

@eintopf: your commit doesn't cover all missing directives. Full list:
types\MatchmakingTypes\gameserveritem_t.cs
types\MatchmakingTypes\servernetadr_t.cs
types\SteamClient\SteamAPIWarningMessageHook_t.cs
types\SteamClient\SteamAPI_CheckCallbackRegistered_t.cs
types\SteamClientPublic\CGameID.cs
types\SteamClientPublic\CSteamID.cs

Also you changed #region formatting style and there is no new line at end of the files.

@gresolio
Copy link

gresolio commented Mar 2, 2017

Also I have a question to the author: why DISABLESTEAMWORKS was chosen?

This directive is very useful for multi-platform projects. But imho ENABLESTEAMWORKS is more appropriate. Let's say we have 4 different platforms, so currently we must define 3 DISABLESTEAMWORKS for non PC platforms, and use #if !DISABLESTEAMWORKS ... #endif for the Steam-specific code.

I think it's better to define only one directive ENABLESTEAMWORKS for PC, and use #if ENABLESTEAMWORKS ... #endif. To activate it by defaults, SetScriptingDefineSymbolsForGroup can be used via editor script, or maybe just a simple define in the SteamManager and some notes in the docs to explain how it can be changed if required.

@eintopf
Copy link
Author

eintopf commented Mar 2, 2017

@gresolio Thanks for your replies.

  1. For me the project compiled by just adjusting the mentioned files. I just wanted to touch as few files as possible
  2. I think your ENABLESTEAMWORKS proposal is a good idea, because there are more platforms that don't use it than using ones. I think a related issue would be Automatically define DISABLESTEAMWORKS when running on non-Steam platforms #126, there you can also finde some sort of workaround for this.

@rlabrecque
Copy link
Owner

Crap yeah, sorry about that! All that stuff is autogenerated and it looks like I forgot to add it to one of the templates! I get back home from GDC on Wednesday and I'll take a look at it then.

I don't think I'll be doing ENABLESTEAMWORKS because I want it to work with no setup by default. (If you're including it you'd hopefully want to use it on at least one platform!) And the majority of users are only shipping on Windows/Linux/macOS.

Unfortunately defining it SteamManager won't work because defines aren't global :( It would have been the way I would have done it for sure though! The SetScriptingDefineSymbolsForGroup could be interesting though!

@gresolio
Copy link

gresolio commented Mar 5, 2017

I agree, defining in the script won't work because it is not global. But SetScriptingDefineSymbolsForGroup can solve the issue. And there is already a good place to add this editor code: "RedistInstall.cs". Unfortunately one cannot fully avoid setup, so it would be nice to take into account all pros & cons. Currently DISABLESTEAMWORKS is not the most convenient and logical, issues #125 #126 are good reasons to think about this improvement. Anyway thanks for the great wrapper :)

@rlabrecque
Copy link
Owner

Yeah I'll definitely check out SetScriptingDefineSymbolsForGroup when I get back, I think we can make something good happen with that.

As long as it can work out of the box, and provide flexibility. (Right now importing the unitypackage into an empty project, adding SteamManager to a game object and then entering play mode is all that needs to happen to get up and running. And then just editing the appid.)

@fredsa
Copy link

fredsa commented Mar 6, 2017

Note that many developers will likely be using these player settings for their own custom defines, including possibly writing hard coded values use their own custom Editor scripts, so I might suggest:

  1. When writing to this value using SetScriptingDefineSymbolsForGroup , be sure to prefix to the existing values using PlayerSettings.GetScriptingDefineSymbolsForGroup. (I'm suggesting prefix instead of append, because an appended value is more likely to be missed when the list of values is long than a prefixed value).
  2. Consider a preference system which a) prompts the developer before adding the custom define, b) allows the developer to opt-out of the setting, in the case that the developer wants to script her own logic for when to add this value. (For example, they may wish to build two Standalone clients, one with Steamworks.NET integration, and one without,)

@fredsa
Copy link

fredsa commented Mar 6, 2017

For #1, something like:

string SYMBOL_DEFINE_ENABLESTEAMWORKS = "ENABLESTEAMWORKS";
string oldSymbolDefines = PlayerSettings.GetScriptingDefineSymbolsForGroup (BuildTargetGroup.Standalone);
foreach (string symbol in oldSymbolDefines.Split (';')) {
	if (symbol.Equals (SYMBOL_DEFINE_ENABLESTEAMWORKS)) {
		// Symbol define found. Nothing to add.
		return;
	}
}
// Prefix new symbol
PlayerSettings.SetScriptingDefineSymbolsForGroup (BuildTargetGroup.Standalone, SYMBOL_DEFINE_ENABLESTEAMWORKS + ";" + oldSymbolDefines);

@rlabrecque
Copy link
Owner

This should be fixed with 460e5e2.

I didn't end up taking your pull request eintopf because of the reasons gresolio laid out, sorry!

@tosiabunio
Copy link

In Unity 2017.3.1 declaring DISABLESTEAMWORKS leads to code that cannot be compiles as Steamworks namespace and other symbols becomes unavailable in SteamManager.cs. SteamManager.cs requires #if !DISABLESTEAMWORKS exclusion as well.

@rlabrecque
Copy link
Owner

I guess that's the reason why I was hesitant to do something like this in the first place. Now all of your own code that uses Steamworks functions would need something like DISABLESTEAMWORKS, and just generally becomes a mess 😕

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

5 participants