Eintragsdetails ansehen

IDProjektKategorieSichtbarkeitZuletzt aktualisiert
0002337EresseaFeaturewunschöffentlich2017-07-17 17:23
ReporterCTDBearbeitung durchEnno 
PrioritätnormalAuswirkungkleinerer FehlerReproduzierbarimmer
Status erledigtLösungerledigt 
Produktversion3.12.1 
Zielversion3.13Behoben in Version3.13 
Zusammenfassung0002337: Regionen um Leuchttürme werden nicht mehr angezeigt
Beschreibung

Meine Einheit Ruhmsucher (xuhw) steht als 2te Einheit in einem Leuchtturm einer anderen Partei. Diese andere Einheit hat 12 Personen.
Obwohl der Leuchturm normal arbeitet (Besitzer hat einen Report der umliegenden Regionen bekommen) war das bei mir mit dieser Auswertung nicht mehr so.
Vermutlich hat hier die E2 Limitierung auf 2 Personen zugeschalgen, die es in E3 jedoch nie gab. Vermutlich war das unabsichtlich, da der Check beim Wahrnehmungstalent gemacht wurde, das es aber für E3 nicht gibt. Bitte die Größenlimitierung (am besten incl. E2) komplett aufheben, zumindest was den Report an geht. Einer Einheiten in den Leuchtturm Stellen zu müssen damit eine 2te Partei auch etwas sieht ist erscheint mir in keiner Weise irgendwie sinnvoll.

Parteict0d
SpielE3
Report411

Eintrags-Beziehungen

verwandt mit 0002221 erledigtEnno Größenbegrenztes Gebäude (Hafen) größer als Maximalgröße 

Notizen / Dateien

Enno

Enno

2017-06-17 22:14

Administrator   ~0007256

Auch das klingt mir sehr nach einem Featurewunsch. Den Regeln entspricht es auf jeden Fall (was ein unangekündigter bzw. ungewollter Bugfix sein könnte): https://wiki.eressea.de/index.php/Andere_Geb%C3%A4ude#Leuchtturm sagt Kapazität: 4 Personen.

Enno

Enno

2017-06-18 12:43

Administrator   ~0007259

Es gibt offenbar keine Partei ct0d in E3. Kann die Sache daher gerade nicht nachvollziehen.

Enno

Enno

2017-06-18 12:44

Administrator   ~0007260

Aha! Ruhmsucher (xuhw) gehört zur Partei 6t0d. Tippfelher.

Enno

Enno

2017-06-18 12:52

Administrator   ~0007261

In reports.c:1444 steht:

                if (u->building && u->building->type == bt_lighthouse && inside_building(u)) {

Der Ausdruck inside_building(u) ist false, weil die Kapazität 4 ist, und eine 12-Personen Einheit den ganzen Platz beansprucht, deshalb bekommen beide Einheiten den Effekt des Leuchtturmes nicht. Soweit ist das alles im Rahmen der Regeln. Ergo: Featurewunsch.

Enno

Enno

2017-06-18 12:56

Administrator   ~0007262

Generell finde ich es nicht verkehrt, wenn die ersten vier EINHEITEN statt der ersten vier PERSONEN den Effekt des Leuchtturmes bekommen würden, aber da ich selber keine Leuchttürme besitze, in denen mehr als eine Person steht, würde ich da erst die Spieler befragen wollen. Selbst dann fehlt mir die Zeit für Features, die nicht jemand anderes implementiert, da ich mich demnächst primär um Bugs kümmern möchte. Davon haben wir gerade wieder genug.

CTD

CTD

2017-06-24 13:16

Entwickler   ~0007296

Verstehe ich alles, aber 2 Anmerkungen habe ich da doch noch:

  1. Die 12er Einheit sieht die Regionen. Wäre auch echt bescheiden wenn man den Unterhalt bezahlt und dann nix sieht weil die Eineheiten aus zu vielen Personen besteht. Das ganze ist auch unlogisch.
  2. Es ging die letzen 7 Jahre. Die Anleitung als Master heranzuziehen halte ich nicht für den besten Weg. Da steht genug falsches drin. Daher sehe ich das durchaus als Bug.

Und was spricht gegen die Änderung das Größenlimit einfach ganz zu entfernen? Das ist in 5 min gemacht, und ich sehe da kein unfairen Vorteil oder Nachteil im Spiel. Man kann halt Einheiten mit anderen Aufgeben auch mit in den Leuchtturm stellen.

Enno

Enno

2017-06-24 13:43

Administrator   ~0007299

Du unterschätzt glaube ich, wie kompliziert die Erstellung des Reports (und Liste der anzuzeigenden Regionen) ist, und die Anzahl Tests, die davon betroffen sind. Aber ich glaube auch, das Limit hier nicht einzubeziehen ist die beste Lösung, und kann mir das als Feature für das nächste Release vorstellen. Priorität haben aber trotzdem die Untoten, mit denen ich immer noch nicht fertig bin.

Enno

Enno

2017-07-09 17:16

Administrator   ~0007336

Heute habe ich mir das mal wieder ansehen wollen: Der Code im develop Branch hat sich seit meiner ersten Untersuchung stark geändert, die Zeile reports.c:1444 sieht jetzt komplett anders aus. Aber das Verhalten ist noch immer das selbe. Ich bleibe dran.

Enno

Enno

2017-07-15 12:48

Administrator   ~0007341

Starting a new feature branch: feature/2337-lighthouse-capacity

Enno

Enno

2017-07-15 13:29

Administrator   ~0007342

Aha. Der neue Code ruft nicht inside_building auf, weil das O(n^2) war. Der zählt mit, während er durch die Einheiten scannt:

                if (u->building && b != u->building) {
                    b = u->building;
                    c = buildingcapacity(b);
                    br = 0;
                }
                c -= u->number;
                if (u->faction == f && c >= 0) {

Das muss dann alles anders, wenn es die Kapazität ignorieren soll.

Enno

Enno

2017-07-15 13:37

Administrator   ~0007343

Wenn ich den Code ändere, gibt es einen Test, der das anmeckert, den sollte ich also wohl als erstes Mal anpassen:
1) test_prepare_lighthouse_capacity: C:\Users\Enno\Documents\Eressea\git\src\reports.test.c:496: expected <1> but was <2>

Enno

Enno

2017-07-15 13:45

Administrator   ~0007344

Die korrekte Vorgehensweise ist hier bestimmt, buildingcapacity() für einen Leuchtturm irgendwie unendlich groß zu machen (INT_MAX sollte reichen). Das kann man aber, soweit ich es sehe, derzeit (noch) nicht über die XML config machen.

Enno

Enno

2017-07-15 13:59

Administrator   ~0007345

buildingcapacity hat keine Tests, also habe ich die erst einmal nachgeliefert.

Enno

Enno

2017-07-15 14:27

Administrator   ~0007346

Blöderweise haben wir sowohl eine maxsize als auch eine maxcapacity für Gebäude. Wenn das Gebäude eine size >= maxsize hat, gilt maxcapacity sonst size * capacity. Oft (aber wohl nicht immer) ist maxsize == maxcapacity, aber für evtl. existierende Ausnahmen ist der Code extra verkompliziert. Wenn ich daran aber bastele, um diese Änderung zu vereinfachen, dann mache ich evtl. ein anderes Gebäude kaputt. Das wäre natürlich Mist, deshalb muss ich hier mit extra Vorsicht vorgehen, und das nicht im Halbschlaf machen.

Enno

Enno

2017-07-15 18:44

Administrator   ~0007347

Kann man die Sache reduzieren auf 1. Gebäude, die eine Kapazität haben, die von ihrere Größe abhängt (size * capacity), und solcher, die bei Vollendung eine Kapazität haben (if (size>=maxsize) maxcapacity)? Es lohnt sich bestimmt, die existierenden Gebäude erst einmal in solchen Gruppen zu klassifizieren, und dann Tests für jede Sorte zu schreiben.

Enno

Enno

2017-07-17 17:13

Administrator   ~0007348

Ich habe das jetzt glaube ich entwirrt, und so gelöst, dass die Kapazität von Leuchttürmen sich auf die Anzahl der Einheiten, nicht Personen, bezieht. Als Resultat erscheint in 411-6t0d.nr:

Capygilbos (14,-3) (vom Turm erblickt), Hochland, 134/40 Bäume, 609 treue Bauern, 33 Pferde.

So war das wohl gewünscht.

Enno

Enno

2017-07-17 17:23

Administrator   ~0007349

Änderung ist in PR https://github.com/eressea/server/pull/710

Eintrags-Historie

Änderungsdatum Benutzername Feld Änderung
2017-06-17 20:53 CTD Neuer Eintrag
2017-06-17 20:53 CTD Status neu => zugewiesen
2017-06-17 20:53 CTD Bearbeitung durch => Enno
2017-06-17 22:14 Enno Notiz hinzugefügt: 0007256
2017-06-18 12:43 Enno Notiz hinzugefügt: 0007259
2017-06-18 12:44 Enno Notiz hinzugefügt: 0007260
2017-06-18 12:52 Enno Notiz hinzugefügt: 0007261
2017-06-18 12:52 Enno Bearbeitung durch Enno =>
2017-06-18 12:52 Enno Kategorie General => Featurewunsch
2017-06-18 12:52 Enno Produktversion 3.13 => 3.12.1
2017-06-18 12:56 Enno Bearbeitung durch => Enno
2017-06-18 12:56 Enno Status zugewiesen => Rückmeldung
2017-06-18 12:56 Enno Notiz hinzugefügt: 0007262
2017-06-24 13:16 CTD Notiz hinzugefügt: 0007296
2017-06-24 13:16 CTD Status Rückmeldung => zugewiesen
2017-06-24 13:43 Enno Notiz hinzugefügt: 0007299
2017-06-24 13:43 Enno Zielversion => 3.13
2017-07-09 17:16 Enno Notiz hinzugefügt: 0007336
2017-07-15 12:48 Enno Notiz hinzugefügt: 0007341
2017-07-15 13:29 Enno Notiz hinzugefügt: 0007342
2017-07-15 13:37 Enno Notiz hinzugefügt: 0007343
2017-07-15 13:45 Enno Notiz hinzugefügt: 0007344
2017-07-15 13:59 Enno Notiz hinzugefügt: 0007345
2017-07-15 14:27 Enno Notiz hinzugefügt: 0007346
2017-07-15 18:44 Enno Notiz hinzugefügt: 0007347
2017-07-17 13:43 Enno Beziehung hinzugefügt verwandt mit 0002221
2017-07-17 17:13 Enno Notiz hinzugefügt: 0007348
2017-07-17 17:23 Enno Status zugewiesen => erledigt
2017-07-17 17:23 Enno Lösung offen => erledigt
2017-07-17 17:23 Enno Behoben in Version => 3.13
2017-07-17 17:23 Enno Notiz hinzugefügt: 0007349