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

Add versioning #30

Closed
elizagamedev opened this issue Sep 29, 2018 · 19 comments
Closed

Add versioning #30

elizagamedev opened this issue Sep 29, 2018 · 19 comments
Assignees
Milestone

Comments

@elizagamedev
Copy link
Contributor

Currently it is not clear when an API breaks or a feature is added. Tracking versions and releases (perhaps via something similar to GitFlow?) would make it much easier to integrate this software in another project without worries about if a change will break things out from under it.

@zpostfacto
Copy link
Contributor

Will do. I probably should have done that before my last major update. I plan on making a version here that matches every major Steamworks SDK release. My next big task is to get this into the Steamworks SDK.

Keeping this issue open until I create the first stable release.

@nxrighthere
Copy link
Contributor

@fletcherdvalve Any ETA when the initial release will be available?

@zpostfacto
Copy link
Contributor

For the Steamworks SDK, I'm shooting for a few weeks. So probably a month or so realistically.

@zpostfacto zpostfacto self-assigned this Jan 13, 2019
@nxrighthere
Copy link
Contributor

nxrighthere commented Jan 13, 2019

@fletcherdvalve I just wonder, does it make sense to update the C# wrapper right now (people are asking for it) since the code is still subject to change.

By the way, can you please provide a function in the flat interface for releasing networking message using this pointer? So we will be able to feed it to a function on the managed side.

@zpostfacto
Copy link
Contributor

Yes, I am trying to avoid making any more big changes to the API until the first release, which should have an API very similar to this. So if you want to target that release, I think you could start with this, and you won't waste much work.

The biggest changes to the API were:

  • Abstracting SteamID to a generic identity thing.
  • IPv6 support.

I will make a function to release a message that has plain C linkage. Is that what you are asking? My long term plan for this is to allow the app to create a message, and pass it to the API when sending a message, and we could avoid copying the payload. I am not sure that is important in managed code or not.

To be honest, I don't understand managed code very well, so I don't really know the best way to support your efforts, which I appreciate. So just tell me what you think would be best. In particular, the callbacks I think are still a bit of a mess. What is the standard way to dispatch callbacks, and is there a better interface that I could provide that would make it more natural to do things in the standard C# way.

@nxrighthere
Copy link
Contributor

I will make a function to release a message that has plain C linkage. Is that what you are asking?

Yes, because right now we can't release received messages.

My long term plan for this is to allow the app to create a message, and pass it to the API when sending a message, and we could avoid copying the payload. I am not sure that is important in managed code or not.

That would be great. This is what ENet provides with NoAllocate flag, so we can track using the callback when a packet is being destroyed in order to release memory with a payload.

To be honest, I don't understand managed code very well, so I don't really know the best way to support your efforts, which I appreciate. So just tell me what you think would be best. In particular, the callbacks I think are still a bit of a mess. What is the standard way to dispatch callbacks, and is there a better interface that I could provide that would make it more natural to do things in the standard C# way.

In general, the current approach is quite convenient to work with I would say. But if you are looking for improving usability and so on then I would recommend you to look at this fork of ENet which we are maintaining primarily for C#.

@zpostfacto
Copy link
Contributor

What functions do I need to provide for dealing with SteamNetworkingIPAddr and SteamNetworkingIdentity? Do I basically need to take all of the inline functions at the bottom of steamnetworkingtypes.h and provide plain C versions of them?

@nxrighthere
Copy link
Contributor

Do I basically need to take all of the inline functions at the bottom of steamnetworkingtypes.h and provide plain C versions of them?

Yes, this is a quite traditional way of wrapping an encapsulated functionality, so we will be able to feed a marshaled structure on the managed side with those unions and other data to it.

@nxrighthere
Copy link
Contributor

nxrighthere commented Jan 13, 2019

You can just follow a plain C principle if a particular functionality will work in plain C then it most likely that we can wrap it on the managed side without any problems since we can marshal structures or work directly with pointers.

@zpostfacto
Copy link
Contributor

OK, the plain C interface is all set, I think. Let me know if you see anything missing or awkward.
And I hope to not make any more big API changes for the next release.

If you make some changes in your project, let me know, and I'll add a link in our readme.

@nxrighthere
Copy link
Contributor

@fletcherdvalve I almost finished updating the wrapper, so far I found only one thing: SteamNetConnectionInfo_t structure still has outdated m_unIPRemote and m_unPortRemote fields, I believe that there should be SteamNetworkingIPAddr instead with the union of addresses.

@zpostfacto
Copy link
Contributor

@nxrighthere Ah yes, I haven't fixed that yet, but I'll do that now.

@nxrighthere
Copy link
Contributor

@fletcherdvalve When I'm trying to bind the socket to ::0 using SteamAPI_SteamNetworkingIPAddr_SetIPv6 on the server the following error occurs: Debug - Type: 2, Message: Cannot create listen socket. Failed to bind socket. Error code 0x00002741.

@zpostfacto
Copy link
Contributor

zpostfacto commented Jan 16, 2019

What is the actual call that is failing?
And are you selecting a port? ::0 is a valid IP to bind to (interpreted as "any interface"). But a port is required, so you'd need to bind to [::0]:port.

@zpostfacto
Copy link
Contributor

zpostfacto commented Jan 16, 2019

This is the correct thing to do in C++:

		SteamNetworkingIPAddr serverLocalAddr;
		serverLocalAddr.Clear();
		serverLocalAddr.m_port = nPort;
		m_hListenSock = m_pInterface->CreateListenSocketIP( serverLocalAddr );

@zpostfacto
Copy link
Contributor

Hm, I'll bet that you must have supplied a port, because you would have gotten a more specific error message, generated by this code:

bool CSteamNetworkListenSocketDirectUDP::BInit( const SteamNetworkingIPAddr &localAddr, SteamDatagramErrMsg &errMsg )
{
	Assert( m_pSock == nullptr );

	if ( localAddr.m_port == 0 )
	{
		V_strcpy_safe( errMsg, "Must specify local port." );
		return false;
	}

@nxrighthere
Copy link
Contributor

Oh, I'm a bit incorrectly encoded the address into a byte array. Now it works fine. Thanks.

@nxrighthere
Copy link
Contributor

@fletcherdvalve The wrapper updated to the latest version. Cheers.

@zpostfacto zpostfacto added this to the 1.0 milestone Jan 18, 2019
@zpostfacto
Copy link
Contributor

Oh hey, we released 1.0.

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

3 participants