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

Make get_client_by_socket() more robust #77

Open
adamgulde opened this issue Jan 29, 2025 · 3 comments · May be fixed by #112
Open

Make get_client_by_socket() more robust #77

adamgulde opened this issue Jan 29, 2025 · 3 comments · May be fixed by #112
Assignees
Labels
bug Something isn't working correctly or something crashes the program medium Simple enough issue but probably overlooked during initial development. MVP Required for minimum viable product (ideal deadline: Dec. 11)

Comments

@adamgulde
Copy link
Collaborator

Relevant TODO can be found in banker.py get_client_by_socket() function. Search for "TODO" using ctrl-f and you'll find it in a large commented block. Please read the whole comment.

The idea here is, when the banker is searching through its locally saved list of players, it will return the first player with the same IP address which it's searching for. An issue will arise, then, for example, whenever you expect the second player to be returned in this context, but you are running everything locally [on the same IP]. Thus, you will return the first player, not the second, which will lead to undesirable behavior. I also think this is what is causing tic tac toe to fail locally, so it's pretty important that this becomes more robust.

DM on Discord if you need help or more clarification (username: indev31).

@adamgulde adamgulde added bug Something isn't working correctly or something crashes the program medium Simple enough issue but probably overlooked during initial development. MVP Required for minimum viable product (ideal deadline: Dec. 11) labels Jan 29, 2025
@fmcubium
Copy link

Hey, can I take this issue for SWE?

@adamgulde
Copy link
Collaborator Author

@fmcubium Update on this? Also, for this issue especially, make sure you're testing with the latest code from the Github. Some changes may have impacted and fixed part of this issue

@fmcubium
Copy link

I will be wrapping up work on this issue by the end of the week.

@fmcubium fmcubium linked a pull request Feb 15, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly or something crashes the program medium Simple enough issue but probably overlooked during initial development. MVP Required for minimum viable product (ideal deadline: Dec. 11)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants