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

Refactor #5

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Refactor #5

wants to merge 9 commits into from

Conversation

sebllll
Copy link
Contributor

@sebllll sebllll commented Jan 19, 2024

refactors Server and Client into digestable operations.

this includes everything from #4

Server:
image

Client:
image

@sebllll sebllll mentioned this pull request Jan 19, 2024
@sebllll
Copy link
Contributor Author

sebllll commented Jan 20, 2024

This doesn't change the functionality/principle of the nodes. the idea was just to give it structure and comments, so that it's easier in the future to maintain the lib.

along the way 2 bugs are fixed:

  1. connection behavior on app start. (wasn't working reliable in some situations)
  2. disabling a server now also disconnects all clients (clients were still connected before)

btw. i just spotted a potential bugger in the screenshot: the Data node in the servers Start operation should be inside the delegate region. i'll push another fix commit for this.

@sebllll
Copy link
Contributor Author

sebllll commented Jan 21, 2024

to be more precice:

ad 1:
sometimes (quite seldom) it can happen that you end up in a state, where the connected pin is misleading, like in this screenshot:
image
the client thinks he is connected, but is not. i 'achieved' this situation with this test-patch i built and after pressing some F8/F5s, it happens really rarely, so not a super big issue, but i investigated a bit and found that it doesn't happen, when the server delays the connection in the very first frame only:
image
This is really debatable and maybe should be reversed?

ad 2:
i stumbled over the following exeption when en-/disabling the server (in the actual master):
image

This exception can be avoided if Stop() is only executed when IsListening() = true like so:
image

then, i saw a situation where disabling the server still had a client think he is connected (like in the first screenshot). i can't reproduce that now though.
To avoid those things, i added this to the stop operation:
image

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

Successfully merging this pull request may close these issues.

1 participant