Skip to content

Conversation

@xeruf
Copy link
Member

@xeruf xeruf commented Dec 18, 2020

not even sure whether this makes sense, but wanted to put it on record just in case
here's what I think we should do:

  • ensure the CancelRequest works (refactor(server): log game leave backend#228)
  • return a new object called GameHandle with the possibility to cancel the game, attach listeners (so the startNewGame method doesn't have to handle all that by itself) and stop listening to a game altogether
  • make it clear that there is one LobbyManager per Game (didn't notice that until recently)
  • fix our weird event propagation by making it more consistent with the backend listeners - which should also use events rather than individual listener interfaces with lots of methods

@xeruf xeruf requested a review from SKoschnicke December 18, 2020 16:07
Copy link
Contributor

@SKoschnicke SKoschnicke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works and is better 👍

@SKoschnicke
Copy link
Contributor

This general plan is also good, but should be split into multiple PRs, right?

@xeruf
Copy link
Member Author

xeruf commented Jan 15, 2021

is better

in what regard? It doesn't really change any behavior so far ;)

@SKoschnicke
Copy link
Contributor

in what regard? It doesn't really change any behavior so far ;)

Using a Future is better than using a callback.

@xeruf
Copy link
Member Author

xeruf commented Jan 21, 2021

well, currently I have implemented an unused Future in addition to the callback ;) but ofc that is supposed to change

@xeruf
Copy link
Member Author

xeruf commented May 11, 2021

This isn't the way to go, different approach is already underway

@xeruf xeruf closed this May 11, 2021
@xeruf xeruf deleted the rooms branch May 11, 2021 07:38
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.

3 participants