software-challenge / backend

Server, Client und Spiel-Plugins der Software-Challenge Germany
https://www.software-challenge.de
11 stars 10 forks source link

onGameOver throws exception #278

Closed SKoschnicke closed 4 years ago

SKoschnicke commented 4 years ago

When the handler in AbstractClient is not set, a GameOver throws an exception, although the case of a not initialized handler should be tested and handled gracefully.

kotlin.UninitializedPropertyAccessException: lateinit property error has not been initialized
    at sc.plugin2021.AbstractClient.onGameOver(AbstractClient.kt:129)
    at sc.networking.clients.LobbyClient.onGameOver(LobbyClient.java:140)
    at sc.networking.clients.LobbyClient.onObject(LobbyClient.java:89)
    at sc.networking.clients.XStreamClient.receiveThread(XStreamClient.java:113)
    at sc.networking.clients.XStreamClient$1.run(XStreamClient.java:77)
    at java.base/java.lang.Thread.run(Thread.java:834)
anarchuser commented 4 years ago

What should happen when trying to access a non-initialised handler?

SKoschnicke commented 4 years ago

https://github.com/CAU-Kiel-Tech-Inf/server/blob/edc99c9311085a3cf537ae5836af76a8ce015d8e/plugin/src/client/sc/plugin2020/AbstractClient.java#L163-L165

I think it is already tried to do the right thing: Not calling the handler if no handler is set.

anarchuser commented 4 years ago

https://github.com/CAU-Kiel-Tech-Inf/server/blob/3738f6d71155ec3b456be23054046b31dd7eb38e/plugin/src/client/sc/plugin2021/AbstractClient.kt#L128-L130

This was supposed to be the equivalent in the Blokus AbstractClient. I'll check why it isn't working

anarchuser commented 4 years ago

What exactly did you do to get this error?

SKoschnicke commented 4 years ago

Start the GUI with the getPossibleMoves method active in ClientController.kt. Then start a game by clicking on the "Start Client" button. Because the getPossibleMoves method will error, the client will leave the game and the server sends a gameover event, which leads to this error in the other client.

anarchuser commented 4 years ago

Is this still valid?

xeruf commented 4 years ago

Since having no handler is a valid state, the variably should be nullable rather than lateinit - the latter is something to very rarely use, usually for dependency injection.

xeruf commented 4 years ago

If there is no use-case for having no handler, set it as constructor parameter. Either way, no lateinit and unexpected behavior ;)

xeruf commented 4 years ago

fd60e793 should solve this