Eintragsdetails ansehen

IDProjektKategorieSichtbarkeitZuletzt aktualisiert
0002228EresseaNACH/ROUTEöffentlich2017-12-05 19:50
ReporterEnnoBearbeitung durchEnno 
PrioritätnormalAuswirkungkleinerer FehlerReproduzierbarnicht getestet
Status geschlossenLösungerledigt 
Produktversion3.8.16 
Zielversion3.9.1Behoben in Version3.9.1 
Zusammenfassung0002228: Piraterie erlaubt "tunneln"?
Beschreibung

In Bug 0002191 hat @Solthar gemeldet, dass man mit Piraterie Unsinn machen kann, ich extrahiere das mal in einen eigenen Report.

Schritte zur Reproduktion

Solthar sagt: Deine Erwähnung von Piraterie hat mich an was erinnert: Das ist wirklich kaputt und erlaubt Schiffen das "Tunneln":

function test_piracy()
local r = region.create(0, 0, "plain")
local r2 = region.create(1, 0, "plain")
local r3 = region.create(-1, 0, "ocean")
local f = faction.create("pirate@eressea.de", "human", "de")
local f2 = faction.create("elf@eressea.de", "human", "de")
local u1 = unit.create(f, r2, 1)
local u2 = unit.create(f2, r3, 1)

u1.ship = ship.create(r2, "longboat")
u2.ship = ship.create(r3, "longboat")
u1:set_skill("sailing", 10)
u2:set_skill("sailing", 10)

u1:clear_orders()
u1:add_order("PIRATERIE")
u2:clear_orders()
u2:add_order("NACH o")

process_orders()

-- write_reports()
assert_equal(r2, u1.region) -- should pass, but fails!!!
end

Partei0
SpielE2
Report0

Notizen / Dateien

Enno

Enno

2016-08-19 18:19

Administrator   ~0006729

Ich kann nachvollziehen, dass der Test bei mir auch fehlschlägt. Jetzt muss ich mal schauen, was der eignetlich tut, entscheiden ob das in der Tat falsch ist, und warum es passiert.

Enno

Enno

2016-08-20 11:01

Administrator   ~0006730

Es gibt da einen Unit-Test: test_piracy_cmd_land_to_land, der ist aber deaktiviert, weil er anschlägt. Das ist kein gutes Zeichen. Deaktivierte Tests sind böse.

Enno

Enno

2016-08-20 12:23

Administrator   ~0006731

test_piracy_cmd_land_to_land tut nicht, was er sagt, der Pirat steht in einer SEA_REGION.

Enno

Enno

2016-08-20 12:27

Administrator   ~0006732

Oh! Das letzte Mal wurde an dem Test in 78fa6d3a etwas gemacht, als das SAIL_INTO flag entfernt wurde. Da wurde das Flag von SAIL_INTO nach SEA_REGION geändert, das war sicher falsch und überhastet. Der Test war allerdings vorher schon disabled (seit seiner Erschaffung in 9c076ba6 durch Steffen).

Enno

Enno

2016-08-20 15:08

Administrator   ~0006733

Zur Erinnerung an mich, wie Piraterie funktioniert: piracy_cmd macht einen Befehl, und ruft dann damit move_cmd auf, wo die Bewegung ausgeführt wird.

Es gibt da eine Stelle im Code, die sagt:
// TODO this could still result in an illegal movement order (through a wall or whatever)
// which will be prevented by move_cmd below

Das ist mir irgendwie gruselig. Wird dann da ein Fehler ausgegeben, für einen NACH Befehl, den man der Einheit selber nie gegeben hat?

Enno

Enno

2016-08-20 15:13

Administrator   ~0006735

Die tesT_piracycmd* Tests sind schwierig zu verstehen, setup_piracy macht eine Menge Zeug, und benutzt keine regulären Terrains. Das macht die ganze Suche gerade sehr zäh.

Enno

Enno

2016-08-20 17:22

Administrator   ~0006736

Ich glaube der Fehler liegt daran, dass move_cmd in piracy_cmd mit move_on_land=true aufgerufen wird. Wofür diese Option existiert, kann ich nicht nachvollziehen, sie wird auch sonst nirgends benutzt, und hat keine Tests, ich rechne also eher damit, dass das ein totes Feature ist, das hier aus Versehen benutzt wurde.

Die Unit-Tests verstehe ich immer noch nicht, und werde sie eventuell neu schreiben, damit ich noch einen hinzufügen kann.

Enno

Enno

2016-08-20 19:26

Administrator   ~0006737

PR https://github.com/eressea/server/pull/543

Eintrags-Historie

Änderungsdatum Benutzername Feld Änderung
2016-08-19 18:13 Enno Neuer Eintrag
2016-08-19 18:16 Enno Bearbeitung durch => Enno
2016-08-19 18:16 Enno Status neu => zugewiesen
2016-08-19 18:19 Enno Notiz hinzugefügt: 0006729
2016-08-20 11:01 Enno Notiz hinzugefügt: 0006730
2016-08-20 12:23 Enno Notiz hinzugefügt: 0006731
2016-08-20 12:27 Enno Notiz hinzugefügt: 0006732
2016-08-20 15:08 Enno Notiz hinzugefügt: 0006733
2016-08-20 15:13 Enno Notiz hinzugefügt: 0006735
2016-08-20 17:22 Enno Notiz hinzugefügt: 0006736
2016-08-20 17:25 Enno Zielversion => 3.9.1
2016-08-20 19:26 Enno Status zugewiesen => erledigt
2016-08-20 19:26 Enno Lösung offen => erledigt
2016-08-20 19:26 Enno Behoben in Version => 3.9.1
2016-08-20 19:26 Enno Notiz hinzugefügt: 0006737
2017-12-05 19:50 Enno Status erledigt => geschlossen