Chat UI state fix#48
Conversation
the-only-queen-anna
left a comment
the-only-queen-anna
left a comment
There was a problem hiding this comment.
Bitte schau dir mal die Kommentare durch, ich probier gleich mal, ob die Funktionalität vom Chat funktioniert und kommentier dann nochmal.
| @@ -199,16 +194,17 @@ fun GameboardScreen( | |||
|
|
|||
| if (!isSpymaster && isChatOpen) { | |||
There was a problem hiding this comment.
Hier müssen wir auch noch was ändern, es gibt doch den "lobby-chat", den sollten Spymaster auch sehen können
| val trimmedMessage = chatInput.trim() | ||
| if (trimmedMessage.isNotBlank()) { | ||
| onSendChatMessage(tab, trimmedMessage) | ||
| chatInput = "" |
There was a problem hiding this comment.
man sollte das mit onChatInputChange auf "" setzen glaube ich, nicht manuell
| remainingGuesses = gameState.remainingGuesses, | ||
| cards = cards, | ||
| ), | ||
| gameState = gameState.copy(chatLists = chatState), |
There was a problem hiding this comment.
Keine Ahnung ob das so gelöst werden sollte. Könnte zu unvorhergesehenem Update-Verhalten führen, aber ich werde es eh gleich mal testen und schauen, ob es noch funktioniert.
| } | ||
| } | ||
|
|
||
| ChatTab.OPERATIVES -> { |
There was a problem hiding this comment.
Wenn der Operatives Chat nur für Operatives in einem Team ist, sollte er auch nur sichtbar sein, wenn es mehr als einen Operative im Team gibt, nur kurze Anmerkung
| isHost: Boolean = false, | ||
| ) { | ||
| job?.cancel() | ||
| _chatState.value = ChatLists() |
There was a problem hiding this comment.
Diese Zeile kann entfernt werden. Oben steht doch, _chatState = MutableFlowOf(ChatLists()), was das gleiche macht, Oder übersehe ich etwas?
|
|
||
| try { | ||
| client.connectStomp() | ||
| client.sendReconnectMessage(WebSocketJoinMessage(username, lobbyCode)) |
There was a problem hiding this comment.
nein das gehört hier nicht rein, schon gar nicht ohne if etc. Das müssen wir abhängig vom Android Lifecycle aufrufen, wenn sich die App schließt und wieder öffnet, und den aktuellen Lobby Code müssen wir irgendwo zwischenspeichern. Also so einfach ist das reconnect leider nicht gelöst.
|
Also ich habs mir jetzt mal angesehen. Mit den beiden Änderungen in GameViewModel.connect() startet nicht einmal das Spiel, das zerstört die Verbindung zum Websocket. Wenn man die beiden Lines auskommentiert, kann man das Spiel starten, aber Chats senden geht nicht (bzw. senden schon, aber empfangen nicht, zumindest wird die UI nicht upgedatet, nicht einmal am Sender-Handy. |
the-only-queen-anna
left a comment
the-only-queen-anna
left a comment
There was a problem hiding this comment.
Hab wieder ein paar Sachen anzumerken, was code Quality und Strukturierung betrifft. Getestet hab ich ebenfalls nochmal, bisschen detaillierter diesmal und es werden Nachrichten korrekt gesendet, aber das Backend muss gefixt werden, da dort jedes mal noch eine Exception geworfen wird, sobald eine Nachricht ankommt, also kann ich leider aktuell nicht sagen, ob die Subscriptions und das Empfangen von Nachrichten funktionieren würde.
| val team = currentPlayer?.team | ||
| val lobbyCode = lobbyState.lobbyCode.orEmpty() | ||
| val cards = gameState.cards | ||
| val availableChatTabs = getAvailableChatTabs(userRole, lobbyState.players, team) |
There was a problem hiding this comment.
Keine Ahnung wie genau wir das machen wollen, aber theoretisch könnten sich die available Tabs noch ändern, wenn im Spiel z.B. ein zweiter Operative joint oder so viele leaven, dass nur noch einer da ist, also müsste das auch ein State sein, damit sich die UI entsprechdend aktualisiert
| gameViewModel = gameViewModel, | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
alle Funktionen ab hier gehören nicht in einen Screen, bitte einfach in das view Model verschieben bzw schauen, ob sie überhaupt benötigt werden (send Chat message z.B., da kann mMn einfach direkt die Methode des viewModels aufgerufen werden, was dann alle nötigen Bedingungen prüft. Screens sind wirklich nur zur Anzeige gedacht :)
|
the-only-queen-anna
left a comment
the-only-queen-anna
left a comment
There was a problem hiding this comment.
LGTM, Issues beseitigt
2 participants
Context
fixed chat ui game state update functionality
Description
Changed some chat_ui_state_update behavior to better fit the existing ui_state_update solution
Changes in the codebase
Files changed: GameState.kt, GameScreenWrapper.kt, GameViewModel.kt, GameboarScreen.kt, GameViewModelTest.kt, NavGraph.kt, GameboarScreenTest.kt
Files ChatTab, ChatViewModel, ChatMessage, ChatUiState are not neccessary for current solution, but might be useful later on if we decide to better seperate chat and ui state update