radekmie / h3mapgen

An attempt to build a comprehensive map generator for Heroes of Might and Magic III
6 stars 2 forks source link

Changed filling map to behave more naturally, filling is more even. #85

Closed PiotrPytlik closed 6 years ago

radekmie commented 6 years ago

Kod OK, wyniki - średnio?

screenshot from 2018-03-17 14-55-39 screenshot from 2018-03-17 14-56-01

acatai commented 6 years ago

Zgadzam się z oceną. Dla 42 to w ogóle wygląda hmmm... niezachęcajaco ;] 42_mm

acatai commented 6 years ago

Tzn to ma sens - bo closy resemble to co zwraca MDS - ale ogólnie to wszystko mogłoby być podciągnięte dość mocno do środka...

PiotrPytlik commented 6 years ago

Jak widac po sektorach - ten konkretny podzial (z wyniku MDS) nie da sie podciagnac do srodka przeciez, bo byloby nierowno, tzn w lewym gornym rogu jest zone ktory nie moze urosnac, wiec nie bede przeciez pozostalych rozpychal jesli byloby nierowno... :) wlasnie o tym mowilismy wczesniej, bo osobiscie taki podzial sektorow dla mnie wyglada duzo lepiej niż na 100% zapelnienie mapy za kazdym razem. na innych mapach mozna by bylo porownac wyniki. mi sie wydaje ze to jest spoko

PiotrPytlik commented 6 years ago

Przypominam o PR :) Chciałbym zwrócić uwagę tylko na to że jesli kod jest spoko, i wyniki są zgodne z oczekiwaniami (mimo że wejsciowy graf pozostawia wiele do żądania), to merge bylby super, zeby nie trzeba bylo sie mordować w przyszlosci z merge'ami starego kodu :D

radekmie commented 6 years ago

Zgadzam się z oboma problemami, ale problem jest taki, że z tą zmianą generowane mapy są gorsze. Tak, są dokładniejsze/poprawniejsze, ale to tylko jedno. Jestem za mergem na 100%, jeżeli mielibyśmy to za flagą: coś w stylu forceFillMap albo stretchZonesToFill (domyślnie może być wyłączone).

acatai commented 6 years ago

Flaga dobrym pomysłem jest...

PiotrPytlik commented 6 years ago

Wydawalo mi się że już dodawałem flagę dokladnie na to :) Jeśli nie w parametrach wejsciowych, to przynajmniej w kodzie odpalajacym zapelnianie sztuczne.

acatai commented 6 years ago

To zarzuć info gdzie jest albo sam zapamiętaj iz apisz w jakimś tajnym miejscu ;p.

PiotrPytlik commented 6 years ago

Otworzylem zmiany i w pierwszej linii którą ja dodałem jest self::FillMap(forceFill) gdzie forceFill jest parametrem. Otworzylem plik caly i nawet sygnatura metody Generate ma parametr forceFill, ktory jest opisany że udostepnia mozliwosc wymuszenia forceFill. Więc mniej tajnego miejsca nie było na tę informacje :P

radekmie commented 6 years ago

Ja przez flagę myślałem o czymś na poziomie .h3mapgen, ale dobra. Opinie?

EDIT: Konflikty.

radekmie commented 6 years ago

Próbowałem rozwiązać konflikty, ale są dość trudne i to w GridMap. Ponadto - flaga forceFill nie forsfiluje - nie ma różnicy czy jest czy nie (możliwe, że przy rozwiązywaniu konfliktów coś schrzaniłem). Z tego, nie wrzucam. @PiotrPytlik - morzesz to zrobić?

PiotrPytlik commented 6 years ago

Zajme sie tym na dniach, byloby chyba spoko gdybys mogl wrzucic swoje proby rozwiazania konfliktow na jakas galaz poboczna albo cos w tym stylu, jesli jest ich sporo. Jesli sie nie odezwe niedlugo to przypomnij mi prosze w piatek...

On Oct 2, 2018 19:53, "Radosław Miernik" notifications@github.com wrote:

Próbowałem rozwiązać konflikty, ale są dość trudne i to w GridMap. Ponadto - flaga forceFill nie forsfiluje - nie ma różnicy czy jest czy nie (możliwe, że przy rozwiązywaniu konfliktów coś schrzaniłem). Z tego, nie wrzucam. @PiotrPytlik https://github.com/PiotrPytlik - morzesz to zrobić?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/radekmie/h3mapgen/pull/85#issuecomment-426369415, or mute the thread https://github.com/notifications/unsubscribe-auth/AF5-9Ezs0BsQaa-uZurb6PQIf5NlEM7Wks5ug6f8gaJpZM4Suz14 .

radekmie commented 6 years ago

Nie, nie ma ich dużo - 3, wszystkie w GridMap. OK, przypomnę.

radekmie commented 6 years ago

Przypominam: @PiotrPytlik.

PiotrPytlik commented 6 years ago

@radekmie Rzuć okiem prosze na gałąź master-growth. zrobilem merge, usunalem konflikty (w zasadzie to zrobilem all-from-source, bo nie wiem w ogole dlaczego konflikty wystąpiły, te miejsca nie powinne być zmieniane w ogóle...) Sprawdz teraz prosze czy działa, w klasie generate.lua jesli chcesz wylaczyc forceFill, to mozesz w linii 640 mniej wiecej zmienic parametr forceFill, zostawilem TODO żeby było widać gdzie ten parametr wczepić tak jak chciales z parametrow wejsciowych.

Jesli wszystko bedzie okej, to możesz zamiast kombinowac z mergami itp wywołać git checkout master git merge --ff-only master-growth i github powinien ogarnac ze pull request zostal zmergowany i zakonczony, a Ty nie bedziesz musial niczego w kodzie zmieniac żeby zmergowac growth.

radekmie commented 6 years ago

To jest tak: z wyłączonym forceFill działa. Z włączonym mapa jest taka:

screenshot from 2018-10-07 22-22-58

Więc trochę mało uzupełniło. Problem jest jednak taki, że coś jest nie tak z drogami, bo leci segfault, ale tylko z forceFill. Zajmę się tym w najbliższym czasie, ale póki co nie mergeuje. Może jakiś pomysł, jak dopełnić jeszcze trochę?

radekmie commented 6 years ago

Dodałem "ramkę" dla SFP, przez co nie mamy dróg na krawędziach map - jednocześnie pomogło na ten błąd i usunęło niezbyt przyjemne ścieżki.