marceldev89 / BattleNET

BattlEye Protocol Library and Client
GNU Lesser General Public License v3.0
76 stars 44 forks source link

New changes #45

Closed tym32167 closed 6 years ago

tym32167 commented 6 years ago

What was done: 1) Now it will use async stuff instead of creation new Thread per connection - this will economy resources. Tested on WPF app and .NET Core Web app - worked fine. 2) Moved to .NET Standard 2.0. Worked fine on WPF (fw 4.6.1) and .NET Core 2 apps. Not sure about Nuget Packaging - not checked 3) small refactoring 4) added proper gitignore 5) client version was changed to fw 4.6.1

regarding support. Keep in mind that according this .NET Standard 2.0 is supported on the following platforms (source):

As result:

Need to keep in mind that for .NET Core 2 apps support for Encoding 1252 should be setted up manually (SO).

tym32167 commented 6 years ago

Hi, @marceldev89. Can you pls take a look on this?

marceldev89 commented 6 years ago

Yep, I will take a look this weekend. :)

tym32167 commented 6 years ago

About breaking change - .Net Standard 2 will not be compatible with .Net Framework less than .NET Framework 4.6.1 - this is very important to know. Also I had plans to add another very annoying breaking change - related to #34.

Actually, this pull request and that commit - everything what I wanted to add - so if you want to have backward compatibility with old .NET FW and you dont want to fix typo in API - then no reason for me to create pull request at all.

marceldev89 commented 6 years ago

I have a different idea for the enum changes; instead of removing the lowercase entries, add the uppercase ones and mark the lowercased as obsolete with the ObsoleteAttribute. Then if there will ever be a 2.x version that justifies API changes they can be removed.

I'm not too worried about the .NET Framework 4.6.1+ requirement, it's nearly 3 years old and anyone using the library that hasn't updated their tool in those 3 years will probably also not update the library.

As for that commit related to #34, it's already part of this PR and I kinda missed that. I'm not sure what to do here, I do feel that the library should keep handling the recovery after a connection loss and not move that to the user. The stack overflow related to that must be fixed though. If you can remove this commit from the PR, at least for now, and if the above is fine with you then I'll do the merge.

tym32167 commented 6 years ago

About enum - hmm, that make sense. I will do this. Commit for #34. This is annoying because stackoverflow exception impossible to handle and it killing process each time. Actually, I have different solution for keeping connection alive - I using timer for this - take a look on it and if you will like it, I can add it to your library as well.

tym32167 commented 6 years ago

Take a look on this again pls. Seems I fixed all stuff which you asked. (added 2 new commits)

marceldev89 commented 6 years ago

Yep, nice. Thanks for the work, I'll take a look at the timer/recursion stuff at a later time. Maybe there are other solutions as well but I'll have to find some time and a server to test on. :)