Invalid Moves of Spiders are returned by 'getPossibleMoves'

[Voker13]

Invalid moves of spiders are returned by 'getPossibleMoves'. Those sent moves are correctly marked as wrong by the server.

I have chosen and send a move from (5, -1, -4) to (5, -2, -3) which is not allowed by the game rules.

SpiderBug.xml.zip

[Voker13]

Sollte als Bug gekennzeichnet werden.

[Yasamato]

@Voker13 danke für dein Feedback, kannst du uns ein Replay geben bei dem dieser Fehler auftritt? Verstehe ich das richtig, dass der Fehler beim Spielen durch den Client entstanden sind (Soche-Plugin verwendet, nicht von Hand bei der Grafischen Oberfläche?

@Xerus2000 die diff zwischen den beiden ist eigentlich kaum vorhanden, was zumindest die Logikprüfung am Ende betrifft. GUI-Implementierung, Plugin-Implementierung

Ich kann den Fehler in der GUI zumindest nicht reproduzieren mit der neusten Version.

[Voker13]

Zum Setup: Ich lasse den Server mit dem SimpleClient gewappnet und einem Port offen auf mein Go warten und starte dann den SimpleClient aus Eclipse heraus.

Der Fehler wird schon im Plugin sein, da der Zug ja eigl nicht zulässig ist. Der Server erkennt den falschen Zug ja richtig. Nur doof, dass mir der Zug zur Auswahl überhaupt zur Verfügung steht.

Ich habe btw die selbe Plugin-Implementierung wie oben im link.

Das Replay ist in meiner ersten Nachricht 'gezipt' drin. ;-)

[Yasamato]

Hab ich doch glatt das Replay übersehen und 10min lang versucht das zu reproduzieren 😅...

[xeruf]

@Agent77326 wie ich gesagt habe. Deine Implementation in der GUI lässt ein hin- und zurückgehen im Zug zu, also dass man dasselbe Feld mehrfach betritt. Das wird im Server rausgefiltert mit filterNot { it in currentPath }

[Yasamato]

Hm... das wird in der GUI aber auch abgefangen durch die Überprüfungen in jedem Schritt, dass keins der Felder eins der vorherigen sein darf:

Mit der Funktion this.getNeighbours Tiefe 1: this.getNeighbours(board, from) [...] Tiefe 2: this.getNeighbours(board, depth1) [...] .filter(e => !e.equal(from) [...] Tiefe 3: this.getNeighbours(board, depth2) [...] .filter(e => !e.equal(from) && !e.equal(depth1) [...]

Tut mir leid ich erkenne da meinen Fehler zumindest nicht, sicher dass es durch die GUI ist? Soweit ich weiß fragt der Client doch den Server und nicht die GUI nach den möglichen Zügen oder?

[xeruf]

achso, ich habe den Bug falsch verstanden, ich hatte irgendwie gedacht der Move wird in der GUI fälschlicherweise angezeigt. Ich schreibe mal nen Test im Server dazu...

[xeruf]

@Voker13 hast du auch im client in den libs die neuste version?

[xeruf]

nevermind, konnte den Bug reproduzieren - das ist seltsam...

[xeruf]

Ich habe mir ewig über diesen Fehler den Kopf zerbrochen. getPossibleMoves gibt einen Move zurück, bei dem validateMove korrekterweise einen Fehler schmeißt. Allerdings nutzt getPossibleMoves bei der Bestimmung der möglichen Züge selbst bereits validateMove, sodass scheinbar zwei Aufrufe derselben Methode mit den selben Argumenten unterschiedliche Ergebnisse brachten.

Nach etwas Analyse kam ich darauf, dass der Fehler irgendwo im pathfinding der Spinne ist, denn sie überquert dasselbe Feld zweimal, obwohl das die validation nicht zulässt. Im ersten Moment dachte ich, ich hätte damit den Fehler gefunden, aber das erklärte ja noch nicht warum der Move einmal akzeptiert wurde.

Im Debugging sah ich dann, warum ein Feld mehrfach hinzugefügt wurde: Es war verändert - einmal war ne Spinne darauf und einmal nicht. Doch eigentlich sollten doch nur die Koordinaten des Felds verglichen werden, daher sollte das keinen Unterschied machen? Siehe getAccessibleNeighboursExcept, speziell neighbour.coordinates != except

Doch da kam das nächste Problem ins Spiel: in einer der letzten Änderung hatte ich dafür gesorgt, dass der getter coordinates beim Feld das Field selbst zurückgibt, statt eines neuen Objekts. Zwar praktisch und schnell, sorgte aber hier in dem speziellen Fall, da überall Fields mitgegeben wurden, dafür, dass die Field.equals Methode verwendet wurde, wodurch das Fehlen der Spinne auf dem Feld zu einem neuen möglichen Zug führte.

Bei den Tests dagegen wurde überall nur mit puren CubeCoordinates gearbeitet, sodass dort der Fehler nicht auftrat und ein Konflikt entstand.

Es gibt nun 3 Möglichkeiten weiter zu verfahren:

• es so lassen, auf die Gefahr hin, dass solche Probleme erneut auftreten die Änderung des coordinates-getters behebt hier zwar dieses Problem, schließt aber nicht aus, dass in Zukunft wieder scheinbare Coordinates als Fields verglichen werden
• Field nicht mehr von CubeCoordinates abstammen lassen, was zwar mühsam wäre aber schon auch irgendwie korrekt - ein Feld ist nicht Koordinaten, es hat Koordinaten - OOP ist nervig. Eigentlich bräuchte man implizite Konverter für Methodenaufrufe, denn wenn wir das wieder ändern sind auch viele Code-Änderungen fällig...
• Keine eigene equals-Method für Field definieren - damit würde das System jedes Feld mit gleichen Koordinaten als gleich ansehen, egal was sich darauf befindet. Diese Änderung wäre sehr einfach und dürfte meines Wissens nach auch keine Probleme verursachen.

[xeruf]

Ist vorerst gefixt durch https://github.com/CAU-Kiel-Tech-Inf/socha/commit/23e3269bc12b0ca143c4c885256aab03960787e0 - weiteres Verfahren steht noch zur Debatte

[SKoschnicke]

Ich bin dafuer, eine eigene equals Methode fuer Field zu definieren. In der Spiellogik ist es sehr hilfreich, dass Felder gleich sind, wenn deren Koordinaten gleich sind. Man sollte aber zumindest in der Doku auf dieses Verhalten hinweisen.

[xeruf]

Also willst du, dass wir keine eigene equals-Methode für Field definieren, sondern die von Coordinates übernommen wird. Dann sind wir uns wohl einig und ich implementiere das so :)

[SKoschnicke]

Ja, ich meinte "keine", sorry.

[maggeno1]

Die Fehlermeldung taucht bei mir (Version 20.3.0) immer noch auf... Der entsprechende Move aus den possibleMoves war: Drag from (3,-1,-2) to (3,-2,-1) SpiderMove_Fehler.zip

[xeruf]

Ja, weil die Versionen von server und GUI leider völlig out of sync sind. Wir besprechen das intern nochmal...

[maggeno1]

Gibts denn schon einen Anhaltspunkt, wann in etwa mit der Behebung des Fehlers zu rechnen ist? Langsam geht ja die heiße Phase an.

[SKoschnicke]

@maggeno1 Im gerade gemachten Release 20.3.2 ist der Bugfix fuer dieses Problem enthalten: https://github.com/CAU-Kiel-Tech-Inf/socha-gui/releases/tag/20.3.2 https://github.com/CAU-Kiel-Tech-Inf/socha/releases/tag/20.3.2