letsgamedev / Suffragium

A game developed in a democratic cycle.
GNU Affero General Public License v3.0
51 stars 21 forks source link

Improved pause menu #82

Closed ASecondGuy closed 2 years ago

ASecondGuy commented 2 years ago

Description

In #65 people talked about standardizing help windows. The discussion continued on discord and also mentioned just custom buttons & other pause menu extensions. I went ahead and implemented them.

Related to # (issue)

Type of change

Testing

I opened and closed different games many times to look for issues with loading and deleting the help pages.

Checklist

RedstoneMedia commented 2 years ago

Prinzipiell nicht schlecht gelöst in der game.cfg als optionale Szene und mit custom Buttons und Pages. Ich sehe jedoch ein paar Probleme (hauptsächlich mit SortIt):

Der Help Button im player selector Menü macht zwar so wie gewohnt den help screen auf, jedoch macht er auch das pause Menü auf, wenn man also wieder zum Game möchte, muss man zweimal Escape drücken. Dazu ist noch das Problem, das es kein popup_hide Signal mehr gibt, und deswegen _on_help_popup_hide nicht mehr in SortIt aufgerufen wird (Es gibt noch nicht mal ein Signal, was man manuell connecten kann). Das Problem könnte man lösen, wenn man nur an dieser Stelle einfach das Help Menü ohne die custom Pages aufmacht, sondern einfach wie vorher ein Popup verwendet.

Zudem kann man nicht mehr einfach außerhalb des help Menüs klicken, um es zu schießen.

Du hast außerdem die Styles von dem SortIt help Menü mit dem main theme überschrieben (Divider und Schriftart sehen jetzt anders aus). Die richtigen Styles kommen da eigentlich aus dem Theme im player selector. Eventuell musst du dafür noch einen Margin container um den Button drum packen (Weil der SortIt Button nicht so viel Margin out of the box, hat)

Die Location des close Buttons im SortIt pause Menü ist suboptimal (Ich bin mir aber auch selber nicht sicher, wo der am besten hinpasst, was der Grund war, wieso das original keinen hatte)

Das die pause Menü custom Page dauerhaft im Hintergrund geladen ist (eben nur hidden) ist auch nicht optimal (Wenn z. B. scripts in der help Szene eingebettet sind, die dann während des Spiels Ressourcen verbrauchen) Ich würde da entweder set_process, set_process_internal, set_process_input usw. auf false setzten, wenn es nicht geladen ist oder einfach die custom Page nur dann als Child adden, wenn der help Button geklickt wird (und am Ende wieder entfernen).

Das ist jetzt erstmal alles, was mir dazu einfällt.

(Sorry it's just way to much right now to translate this to english)

ASecondGuy commented 2 years ago

@RedstoneMedia Das 2 mal Esc drücken ist doof da überleg ich mir was.

Das Signal wird gar nicht mehr gebraucht weil das spiel pausiert ist und deswegen muss die Input erkennung nicht mehr abgeschaltet werden. Ich hab aber vergessen die anderen obsoleten Funktionen weg zu machen. Das hol ich gleich nach. Wenn man das signal noch für was anderes bräuchte könnte man aber auch noch visibility_changed benutzen.

Das Theme hab ich übersehen das fix ich auch gleich. Der Close Button kann ich wieder weg machen wenn du meinst aber ich halte ihn für nötig. Das schließen wenn man raus klickt sollte sich auch schnell einbauen lassen.

Dass die pages im Hintergrund geladen sind ist nicht wirklich ein Problem. In dem help script ist nur die eine funktion die auf den button reagiert. Performance technisch ist das gleich wie gar kein script. set_process und co würden hier nichts bringen. Falls jemand ein _process in seine page einbauen will ist das natürlich nicht unbedingt sinnvoll aber möglich.

Das wärs dann fürs erste von mir.

RedstoneMedia commented 2 years ago

Der Close Button kann ich wieder weg machen wenn du meinst

Kannst du erstmal lassen

ASecondGuy commented 2 years ago

@RedstoneMedia I fixed it. As far as I can tell everything still works as intended and I removed all the obsolete code.

RedstoneMedia commented 2 years ago

You forgot to remove _help_popup_just_closed and _selectors_get_input_state_copy in the player selector.

You should also probably save the theme under assets/style/sort_it_theme.tres or something and then load the theme from that location in help.tscn and player_selector.tscn. That way the same theme code is not in two separate files and updates in both, if changes were to be made in the future.

Apart from that this looks pretty good now.

ASecondGuy commented 2 years ago

Yes, I guess that makes sense.