parse-community / Parse-SDK-dotNET

Parse SDK for .NET, Xamarin, Unity.
http://parseplatform.org
Apache License 2.0
323 stars 260 forks source link

Support for Unity runtime and editor scripting #298

Closed TobiasPott closed 5 years ago

TobiasPott commented 5 years ago

The pull request contains additional projects with compiler directives which switch versioning and files & folder paths related code to adopt to the Unity3D Editor environment and the Unity3D runtime.

The build process requires an UnityEngine.dll assembly specific to the targeted Unity version to be available and referenced when building the project with Visual Studio. This assembly is property to Unity Technologies and cannot be provided via github.

codecov[bot] commented 5 years ago

Codecov Report

Merging #298 into master will increase coverage by 0.64%. The diff coverage is 61.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #298      +/-   ##
==========================================
+ Coverage   59.26%   59.91%   +0.64%     
==========================================
  Files          89       91       +2     
  Lines        6258     6204      -54     
==========================================
+ Hits         3709     3717       +8     
+ Misses       2549     2487      -62
Impacted Files Coverage Δ
Parse/Public/ParseCloud.cs 75% <ø> (ø) :arrow_up:
Parse/Internal/Push/ParsePushModule.cs 0% <ø> (ø)
...nal/Push/Controller/ParsePushChannelsController.cs 0% <ø> (ø)
...ernal/Cloud/Controller/ParseCloudCodeController.cs 100% <ø> (ø) :arrow_up:
Parse/Internal/Utilities/ReflectionHelpers.cs 78.04% <ø> (ø) :arrow_up:
Parse/Internal/Utilities/TaskQueue.cs 94.11% <ø> (ø) :arrow_up:
Parse/Internal/Analytics/ParseAnalyticsPlugins.cs 100% <ø> (ø)
Parse/Internal/Push/Coder/ParsePushEncoder.cs 75% <ø> (ø)
Parse/Internal/Utilities/ParseQueryExtensions.cs 66.66% <ø> (ø) :arrow_up:
Parse/Internal/Push/ParsePushPlugins.cs 59.52% <ø> (ø)
... and 52 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 674a6a0...8b26ad2. Read the comment docs.

flovilmart commented 5 years ago

Thanks for the PR. I am curious why there are some commits from @TheFanatr in this PR.

TobiasPott commented 5 years ago

@flovilmart I actually forked from TheFanatr repository instead of the original one as I faced more problems with that one originally inside Unity. If that's a problem I can check compatibility with the original repository and reapply my changes.

TobiasPott commented 5 years ago

@TheFanatr Hi Alex, I just came back to office from the christmas and new years festivities and recognized that you did an merge of your repo to the main repo of Parse for NET.

I did not yet checked if it is working with Unity in my case and would like to check with you on how to get either my additions into the repository or on how to reestablish Unity support.

The repo I've been working on was forked from your previously forked one and I'll assume it will be helpful to redo the fork and merge my changes into the new state of the current main repository.

If you have any suggestions or hints please let me know and I'll will keep them in mind when I get back to Parse in the next days! =)

Edit: I'm not that used to the Appveyor and CodeCoverage tools but if I can support general maintainence and testing in any way, please let me also know.

Best regards Tobias

TheFanatr commented 5 years ago

Hi, I'm a little busy at the moment, but I'll try to explain my thought process on this project as a whole, and then answer the merging question. Basically, the gist of it is that I'd really like to avoid merging any platform-specific code into the main project, because the whole point of moving to .NET Standard was to be able to write common code that works on all platforms, and then provide platform-specific plug-ins or something similar in order to enable better support for individual platforms, helping to avoid the need to include preprocessor directives for edge-case platforms where the generic code doesn't work, because eventually, these statements could end up being exhaustive, as in inclusive of a case for every platform compatible with Parse, making development more annoying, and possibly leaving users targeting other platforms in the dark if the genetic code doesn't work on their platform either. Having a generic version that works on the base .NET platforms eases development while increasing the platform coverage. The added benefit of this plug-in approach is that one compile generates an assembly that at least should somewhat-work on all platforms, meaning that if you are targeting a .NET platform that is functionally similar to or based on .NET Core or .NET Framework, the SDK should just work with no hunting for the platform-specific versions required. Platforms that don't fit that description needed something specific anyways. To state this a different way, the main project should have code that should work on all platforms, and we should provide plug-ins for cases we know don't. Before becoming occupied with other matters, I was working on separating what should be overrideable via plug-ins from what shouldn't be, and providing APIs in order to actually implement those overrides. At the same time, I was trying to compact the code and remove redundant structures that only added to clutter in the source. I was also trying to make it so that ParseClients could be instantiated for scenarios where multiple users need to be authenticated in one application, adding support for more easily using Parse with C#-based server backends such as ASP.NET Core, where the current design of the SDK could only permit a maximum of one user to be properly logged in for the entire site. Long story short, until I finish those changes, having a version that can be specifically built for the purpose of generating a version of the SDK that works perfectly with Unity would not be a bad idea, but merging it into master would just hinder the process of allowing a more generalized cross-platform version as the one I've described to exist, so I think it is best if it is kept on a side branch. That way, Unity support could be easily worked on separately, and when it comes time to launch the fully cross-platform version, the specialized code can be converted into plugins for the Unity platform, and the users can chose to provide implementations for their specific use-cases and environment scenarios, or they could choose to use the implementations specifically developed for the platform, allowing all bases to be covered, with a much more convenient development process; no worrying that unnoticed logic errors in preprocessor directives or build scripts enabled broken generic-case code to be compiled for the Xamarin SDK, for example.

TheFanatr commented 5 years ago

Oops sorry for he wall of text.

TheFanatr commented 5 years ago

Also, something to note is that the tests for the SDK currently cover a comically small subset of important code in the source, which is especially dangerous for the kinds of changes that need to be made, considering the amount of functionality-heavy APIs (internal and not) that are compiled; any help with those at all would be great. For example, the new source contains a bug fix for something so fundamental that it sound crazy that it made it into the previous master commit: only the last where clause in a query restriction chain was actually respected. Yes, you read that right, and yes all 196 (or so) unit tests failed to catch that. The other thing I added is the ability to access the internal APIs of the Parse from the Parse.Test project as long as compilation is in the Debug | AnyCPU configuration. This will now allow the ability to write unit tests that target internal APIs while also not leaving the release-compiled assembly vulnerable to the same sort of access. I also added assembly auto-versioning to the SDK, so that when referencing the assemblies straight from the compile folder in a separate testing project or when the output folder is set up as a NuGet package repository, building and applying changes to the assembly will generate a package with a new name and an assembly with a new version, meaning that the update will be noticed by the project; no need to remove and re-add the reference.

TobiasPott commented 5 years ago

@TheFanatr Thank you for the detailed answer on this. I see and understand what your plan is and like the idea of avoiding compiler directives to the project as I found it not making things easier to get into for users.

I'll keep my Unity adjustments to my fork for now and update it to the latest commit of the main repository and will stand prepared to put it into the plugin structure as soon as one is defined and available.

Regarding the tests, I already suspected that some issues are not covered with those, although this assumption originated from issues I had running the .net standard assembly inside Unity Editor and Unity runtime. I'll check on the tests the next time I get my hands on parse and will look for the actual features and functions they check and whether they are appropiate or need changes.

TobiasPott commented 5 years ago

Closed to clarification about the current state of Unity support with Alex. For the current adoption of the latest commit in the main repository for Unity check https://github.com/TobiasPott/Parse-SDK-dotNET/tree/feature/recode-unity-support (this is not an official repo and mainly used for my personal and my jobs requirements).