letsgamedev / Suffragium

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

Visual upgrade + code changes #56

Closed b7g closed 2 years ago

b7g commented 2 years ago

Description

This PR includes a visual and a code makeover.

Visual

splash screen n1

new menu fix1

new info dialog fix2

new section about/contribute/report a bug fix3

new section settings fix4

the new ingame pause menu n6

Changes, changes everywhere

Additionally to the visual changes depicted above following changes were made:

Type of change

Environment

Testing

tested everything at some point, should work

Checklist

RedstoneMedia commented 2 years ago

Also, please capitalize the first character in a sentence and words on buttons (example, "spielen" instead of "Spielen")

Numenter commented 2 years ago

Windows error: data_manager.gd:41 -> error ERR_CANT_CREATE -> "user://savefiles/p/res://games/flappybird/"

Why is game_id a path? "res://games/flappybird"

RedstoneMedia commented 2 years ago

Also _last_played in game_card.gd cannot be of type int, since get_time_last_played can return null, which causes an immediate crash, if the game has not been played yet.

Using a static video instead of animations in Godot is also a problem, on larger monitors and not that easy to edit.

Another thing to note is, that not every game has a high score, so it should not be displayed in the game info.

Joshix-1 commented 2 years ago

0 shouldn't be the default for score in end_game. There should be the option for games without scores. (null as default)

Numenter commented 2 years ago

Windows error: WebM"s stutter on windows and the support for WebM is removed in 4.0 https://github.com/godotengine/godot/issues/55815 Theora is recommended

Numenter commented 2 years ago

Why would you need an image of a black/white square png image. Just use a color rect, if you need the spacing, just use containers (margin) for that.

The black square imange is used in Settings/Boot Splash, which has to be an image. The white square imange is used in the blur shader.

Numenter commented 2 years ago

Not a fan of the cryptic translation codes. image image

T_PAUSED, T_RESUME, T_RESTART, T_BACK_TO_MENU would be a better fit.

PS: here is a PR for this https://github.com/b7g/Suffragium/pull/1

Joshix-1 commented 2 years ago

Yes translation codes should always contain the whole english text, then you know when you have to update all the other langauge variations

b7g commented 2 years ago

Thanks for the lots of comments and info, I appreciate it. Seems like shit hit the fan a bit haha.

I will respond to the comments first and then resolve the issues.

b7g commented 2 years ago

Also, please capitalize the first character in a sentence and words on buttons (example, "spielen" instead of "Spielen")

I think you meant it the other way around in your example?

b7g commented 2 years ago

Windows error: data_manager.gd:41 -> error ERR_CANT_CREATE -> "user://savefiles/p/res://games/flappybird/"

Why is game_id a path? "res://games/flappybird"

shouldn't be, you are right I'm a little confused there I have to say, as I haven't really touched the data_manager at all.

b7g commented 2 years ago

Also _last_played in game_card.gd cannot be of type int, since get_time_last_played can return null, which causes an immediate crash, if the game has not been played yet.

Using a static video instead of animations in Godot is also a problem, on larger monitors and not that easy to edit.

Another thing to note is, that not every game has a high score, so it should not be displayed in the game info.

a. correct b. I don't see how an more complex animation can be easily recreated with code. Also other games use videos too, so there should be at least a way to prevent the issues, if they are present right now. c. just for clarification: just the games that have no score are meant?

b7g commented 2 years ago

0 shouldn't be the default for score in end_game. There should be the option for games without scores. (null as default)

I agree

b7g commented 2 years ago

Windows error: WebM"s stutter on windows and the support for WebM is removed in 4.0 godotengine/godot#55815 Theora is recommended

Will be changed

b7g commented 2 years ago

Not a fan of the cryptic translation codes.

T_PAUSED, T_RESUME, T_RESTART, T_BACK_TO_MENU would be a better fit.

PS: here is a PR for this b7g#1

I agree on this, but had problems with the sizing in the past.

RedstoneMedia commented 2 years ago

The black square imange is used in Settings/Boot Splash, which has to be an image.

I'm pretty sure there is an option to set a custom boot splash screen instead of just an image. If that is not the case just make the image a 1x1 transparent image and make the background black ( this would also only appear very shortly before a splash screen scene is loaded ).

The white square imange is used in the blur shader.

And you can't create a rectangle in a shader ?

RedstoneMedia commented 2 years ago

Also, please capitalize the first character in a sentence and words on buttons (example, "spielen" instead of "Spielen")

I think you meant it the other way around in your example?

Yeah sorry my bad. The other way around is obviously correct.

RedstoneMedia commented 2 years ago

c. just for clarification: just the games that have no score are meant?

Ideally yes. However this would require another manual entry in the game CFGs. That's why hiding the highscore for all games until a non null highscore is returned by end_game, is also a good option.

Numenter commented 2 years ago

The black square imange is used in Settings/Boot Splash, which has to be an image.

I'm pretty sure there is an option to set a custom boot splash screen instead of just an image. If that is not the case just make the image a 1x1 transparent image and make the background black ( this would also only appear very shortly before a splash screen scene is loaded ).

A 1x1 transparent image works fine for the boot splash and it can be used for the blur shader too.

The white square imange is used in the blur shader.

And you can't create a rectangle in a shader ?

Maybe, I'm not a shader guy.

b7g commented 2 years ago

I'm pretty sure there is an option to set a custom boot splash screen instead of just an image. If that is not the case just make the image a 1x1 transparent image and make the background black ( this would also only appear very shortly before a splash screen scene is loaded ).

There is not. Will change to pixel.

And you can't create a rectangle in a shader ?

I just did it like in the example project provided by Godot. Maybe that is possible, but my knowledge with shaders is lacking.

RedstoneMedia commented 2 years ago

Maybe that is possible, but my knowledge with shaders is lacking.

I did it without the image file. Turns out all you have to do is change the TextureRect to a ColorRect. The color of the rect doesn't matter since its overwritten anyway by the shader.

The PR: https://github.com/b7g/Suffragium/pull/3

Numenter commented 2 years ago

Simplified the Tween And Timer Horror. :P https://github.com/b7g/Suffragium/pull/4

ASecondGuy commented 2 years ago

The menu sene hurts to look at. I'll make a PR to fix that mess. Why did you remove bbcode support? That is super easy to implement and gives so many options. That doesn't make any sense. Also. All those fancy buttons are bad practice. You should use one Button node and change the look with a theme. (also in my future PR)

RedstoneMedia commented 2 years ago

Also. All those fancy buttons are bad practice. You should use one Button node and change the look with a theme. (also in my future PR)

Ah do your also working on that ? How far into the process are you with creating a theme for the buttons and stuff ? Because I want to know if it's worth it to continue work on my PR. (I would not fix the other issues you mentioned)

ASecondGuy commented 2 years ago

Also. All those fancy buttons are bad practice. You should use one Button node and change the look with a theme. (also in my future PR)

Ah do your also working on that ? How far into the process are you with creating a theme for the buttons and stuff ? Because I want to know if it's worth it to continue work on my PR. (I would not fix the other issues you mentioned)

I'm just starting. What exactly are you fixing so I can avoid that? And if we both change nodes or resources in the same scene it will probably give merge requests so we should watch out for that. @RedstoneMedia

RedstoneMedia commented 2 years ago

I'm just starting. What exactly are you fixing so I can avoid that? And if we both change nodes or resources in the same scene it will probably give merge requests so we should watch out for that. @RedstoneMedia

I'm creating a theme for the buttons, dorp-downs, check boxes, labels, etc. Because overriding the styles on every single button and label, as it is currently is just painful to look at. So I would probably touch allot of UI nodes, that have a theme override. I won't change the tree structure of the nodes though.

ASecondGuy commented 2 years ago

Ok. Then I can change the Scenes all I want and use your theme once it is merged.

RedstoneMedia commented 2 years ago

Ok. Then I can change the Scenes all I want and use your theme once it is merged.

Yup, that will probably work.

RedstoneMedia commented 2 years ago

Ok, so I was about to change the buttons, but I realized that they are made out of more than one node. The actual button object isn't even really styled. That means that I will need to remove those extra nodes from the tree, to style them.

b7g commented 2 years ago

The menu sene hurts to look at. I'll make a PR to fix that mess. Why did you remove bbcode support? That is super easy to implement and gives so many options. That doesn't make any sense.

Because bbcode in the menu “hurts to look at”. Something should not be done because it’s possible, but because it makes sense. And bbcode in the menu does not make sense, as it gives unnecessary freedoms:

Also. All those fancy buttons are bad practice. You should use one Button node and change the look with a theme. (also in my future PR)

I use Godot to it’s potential with my abilities. And it makes it really hard to properly style things in my opinion. If you are able to do it better, go for it. I would like to see.

ASecondGuy commented 2 years ago

BBcode is the best thing ever You can do a lot of Stuff nice Texteffects to make certain parts stand out. It is not just possible but gives possibilities. Of course you can use it to break readability and make it look inconsistent but you can use any tool wrong. For example you used the Button Node wrong and made the Scenes not readable. You used Themes wrong and that makes accidental inconsistency way more likely. Especially when the look is changed in the future.

When someone introduces inconsistency you help them fix it. You don't remove options in case someone uses them incorrectly. But that last paragraph is just my opinion.

The themes are actually very easy to use once you understand them. But you don't have to do that if you don't want. I'm already working on fixing all that.

b7g commented 2 years ago

When you need to highlight something in the description, it just means you haven’t done your job properly and are bringing it together with highlights in the description on the cost of the user.

ASecondGuy commented 2 years ago

When you need to highlight something in the description, it just means you haven’t done your job properly and are bringing it together with highlights in the description on the cost of the user.

The Description isn't to bandaid over flaws of the game, I agree. If someone uses it like that you should tell them to stop and fix the game. The description is to describe the game (mind bending concept I know). That description can sometimes be improved with the options bbcode offers. I see no downsides to including it. (Again, someone misusing a tool is not a reason to remove the tool)

b7g commented 2 years ago

Please note that I swapped out the images in the description to reflect the latest changes. The changes are rather subtle, therefore the heads-up.

Numenter commented 2 years ago

Bug: Crash on Soriting : Last Played when not all games have been played

Reproduce:

  1. Remove savegames
  2. Sort by last played

ERROR menu.gd:144 Invalid operands 'Nil' and 'Nil' in operator '>'.

Could be the same bug @RedstoneMedia stated before.

Also _last_played in game_card.gd cannot be of type int, since get_time_last_played can return null, which causes an immediate crash, if the game has not been played yet.

b7g commented 2 years ago

should be finally fixed :')

b7g commented 2 years ago

@Joshix-1

I made the formatting of the playtime more precise.

maybe that's precise enough for your liking?

ASecondGuy commented 2 years ago

I finished the scene tree refactor of this. Just need to configure the theme so it looks like it did before.

Joshix-1 commented 2 years ago

Sounds better. Could we maybe show even more precise times on hover? x days y hours z minutes

RedstoneMedia commented 2 years ago

Just need to configure the theme so it looks like it did before.

You mean how it looks now right ? You don't want to change the look back to before my (and b7g's) improvements. Shouldn't you be able to just apply the theme (instead of changing it) ?

A button is going to stay a button, how would a scene tree refactor, change this ?

ASecondGuy commented 2 years ago

Just need to configure the theme so it looks like it did before.

You mean how it looks now right ? You don't want to change the look back to before my (and b7g's) improvements. Shouldn't you be able to just apply the theme (instead of changing it) ?

A button is going to stay a button, how would a scene tree refactor, change this ?

I improved the scene tree structure and removed unnecessary nodes. Because there were still theme overwrites used and the theme didn't have all nodes or they were configured differently it looks like crap. I'm now updating the theme to get the intended look. (I'm not gonna search through old files to find the exact numbers and colors) I'm not much of an artist so someone probably needs to go over the colors again. The buttons stated exactly the same btw.

b7g commented 2 years ago

Sounds better. Could we maybe show even more precise times on hover? x days y hours z minutes

It’s common for playtime to be displayed in hours, not minutes or days

RedstoneMedia commented 2 years ago

@ASecondGuy

I'm not gonna search through old files to find the exact numbers and colors

Okay ? That's a bit suspicious.

Edit: From what I can see, only the top bar looks really off right now. That could be handled with overrides (Or a new custom theme just for the menu bar)

Because there were still theme overwrites used

Also theme overrides are not the devil (If its only used once or twice)

Anyways, your current branch has some obvious issues (I know WIP and stuff but still):

ASecondGuy commented 2 years ago

@RedstoneMedia What is suspicious about that? Most of the visuals were in the theme overwrites before. I reorganized the nodes and therefore most visuals went missing. I don't wanna search for them so I approximated. Btw. the menu bar is a PanelContainer and gets its visuals directly from the main_theme.

And I know overwrites aren't inherently bad. But I found at least a handful of them in V/HBoxContainers where they make limited sense. And I removed many of those Containers.