Rewrite 2022 #20
Merged
Rewrite 2022 #20
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
when migrating from PhoenixJS, I only added the unsubscribe code but didn't remove channel from socket channels array
when joining a new channel, I need to make sure to filter duplicates from socket based on the state as per PhoenixJS
|
This PR is failing the integration tests at the very last step... |
|
🟢 |
when passing null opts, we should use the default opts set to the instance property
|
Fixes #19 |
instead of shoe horning the onMessage callbacks into Message object, I should introduce a generic approach. Perhaps it's just an object, it can even be a string not necessarily an object, which can then be parsed into an arbitrary type.
|
Finally tested and working on an actual unity project 🎉 |
Nice! 🥳 I'll try this updated version in our project when I find the time, but it could take a few days |
Just grab them from NuGet
Also, forces us not to use ToString, which is much less performant than a simple switch with static strings
Also, switch from TimerDelayedExecutor to TaskDelayedExecutor 1. DelayedExecutor now returns an interface IDelayedExecution. This allows custom implementations of IDelayedExecutor to roll out their own DelayedExecution context 2. TimerDelayedExecutor is replaced with TaskDelayedExecutor. Using Tasks fits the use case better and paves the way for async/await APIs and synchronizationContext approach to leverage directly with unity as well.
- Reformat the whole project to be based on C# standards (as per Rider) - increase test coverage and cleanup test assertions
This weird trick makes the callback on the same thread in Unity
# Conflicts: # README.md
|
I think this PR dragged on for long enough 😅 .. Will merge it in order to get it into as many users hands as possible. The feedback will hopefully help us reach v1.0 faster. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Attempt to rewrite to match new PhoenixJS implementation.
Fixes #1
Fixes #4
Fixes #19
TODO
Background
This library was initially written back in 2017 or something, when I had a lot more free time on my hands. I had refactored a lot of bits and written them differently than how they were implemented on Phoenix JS.
Now in 2022, Phoenix JS is much more mature and stable implementation, and using it as a reference implementation will help support the maintainability of this project (I hope!). This also unlocks benefits of new Phoenix JS features to be easily migrated almost line by line.
As much as any developer out there, I dislike breaking changes. However, after 5 years, this library is due for one big rewrite to restructure the internals better, and upgrade the syntax for a new C# language version as supported by Unity. Not to mention, I think I learned a thing or two in those five years, so the code should be of higher quality and performance, generally speaking. Finally, it should also be .NET Core compatible for portability and CI support for tests and coverage.
Regardless, I haven't seen what other developers in the OSS community out there have put out during this time. So if anyone know of a better library that solves the problem this project hopes to address, please share.