software-challenge / backend

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

fix(plugin): make SkipMoves work properly, including a round limit #304

Closed anarchuser closed 3 years ago

anarchuser commented 3 years ago

Related to https://github.com/CAU-Kiel-Tech-Inf/gui/issues/39 Fixes #316

SKoschnicke commented 3 years ago

Tests fail with current master merged into this branch (pushed that state as fix/skipmove/current-master e91714fa3d7ab473ea5adf90daa56824dce6245f :

> Task :plugin:test
Picked up _JAVA_OPTIONS: -Dawt.useSystemAAFontSettings=on -Dswing.aatext=true

sc.plugin2021.GameTest > A game in which everyone skips only ends eventually in a draw FAILED
    java.lang.AssertionError at GameTest.kt:83
        Caused by: java.lang.AssertionError at GameTest.kt:83
            Caused by: java.lang.IndexOutOfBoundsException at GameTest.kt:51

30 tests completed, 1 failed

> Task :plugin:test FAILED

Should this already be reviewed?

anarchuser commented 3 years ago

Tests fail with current master merged into this branch (pushed that state as fix/skipmove/current-master e91714f :

> Task :plugin:test
Picked up _JAVA_OPTIONS: -Dawt.useSystemAAFontSettings=on -Dswing.aatext=true

sc.plugin2021.GameTest > A game in which everyone skips only ends eventually in a draw FAILED
    java.lang.AssertionError at GameTest.kt:83
        Caused by: java.lang.AssertionError at GameTest.kt:83
            Caused by: java.lang.IndexOutOfBoundsException at GameTest.kt:51

30 tests completed, 1 failed

> Task :plugin:test FAILED

Should this already be reviewed?

That error was caused because the exception thrown if no more colors are available got changed without adjusting the test. I adjusted the test now.

Also, yes, this is ready for review

anarchuser commented 3 years ago

That is very weird because it definitely works for me

image

SKoschnicke commented 3 years ago

Strange, will test again later today.

SKoschnicke commented 3 years ago

Now I merged the fix/skipmoves branch into the current backend head of the gui master, and that worked better (last time I checked out the fix/skipmoves branch in the GUI). The game ends now after 100 turns for two human players. But I'm getting strange results when one player is a computer (tested with integrated computer client as player one, human as player two): image The game before that ended after 92 turns. The logs indicate a "regular" end. Shouldn't the game always end after turn 100 if one player always skips? As you can see, player two (green and yellow) are still able to place pieces.

anarchuser commented 3 years ago

That is most likely because the computer couldn't place anymore pieces in the end. After that, only Yellow and Green get turns. But it counts rounds, not turns, thus it ends after 92 turns already - but still after round 25

SKoschnicke commented 3 years ago

You mean in the last few rounds, red and blue didn't get a turn because they couldn't place a piece anymore, right?

Okay, I understand that. But that is not the intuition a human has about the game, turns and rounds. For me, a round always has 4 turns, even if the color is skipped because it can't place anymore. But I understand it from a technical point of view.

So the fix seems to work, but we have to work on the usability :)

anarchuser commented 3 years ago

The round limit is working for now; I can make a separate PR to improve usability

SKoschnicke commented 3 years ago

Yeah, I think it's sufficient to just show the rounds in the GUI instead of the turns. Thers is already an issue for that. No need to change the backend.

anarchuser commented 3 years ago

I'll keep it in mind with Issue #318. I think this'd make some backend code a bit more intuitive