unknown-horizons / godot-port

Unknown Horizons Godot Engine Port
https://www.unknown-horizons.org
GNU General Public License v2.0
665 stars 83 forks source link

Start working on the New Game screen #35

Closed Jummit closed 5 years ago

Jummit commented 5 years ago

Working on #16.

YeldhamDev commented 5 years ago

Just a tip: Start the pull's title with "WIP:" or "[WIP]" while it's still being worked on, so we can quickly see when it's ready.

Jummit commented 5 years ago

Just a tip: Start the pull's title with "WIP:" or "[WIP]" while it's still being worked on, so we can quickly see when it's ready.

Forgot that.

YeldhamDev commented 5 years ago

You'll also have to squish this and your other pull's commits.

Jummit commented 5 years ago

You'll also have to squish this and your other pull's commits.

How do I do that? Is it this?

YeldhamDev commented 5 years ago

Yep, turn those multiple commits into one.

Jummit commented 5 years ago

Whats the reason to do so?

YeldhamDev commented 5 years ago

A cleaner git history. Besides making much more sense to group together commits that are about a specific change, instead of being scattered around. Looking at the commits, the main change here is turning the main menu buttons into their own scenes, and as far we care about, this is the only commit message we want.

YeldhamDev commented 5 years ago

Apologies for not checking this pull sooner, should've tried to merge this first before upgrading the project to 3.1. Could you rebase this one more time?

Jummit commented 5 years ago

Apologies for not checking this pull sooner, should've tried to merge this first before upgrading the project to 3.1. Could you rebase this one more time?

Done.

YeldhamDev commented 5 years ago

There's still a small conflict in the icon.png.import file.

olivier-grech commented 5 years ago

I have a small suggestion. Currently, the checkbox for the game type (Scenario, Random map, Free play) can be unchecked by clicking on it.

I think it should behave like a set of radio buttons: one of the cases is checked at first (let's say Scenario), and when you click on something that is already checked it does nothing (therefore there is alway one case checked).

Here is an example of how you could do that for the GameTypeButton.gd script:

extends HBoxContainer

export var type = ""
export var checked = false

func _ready():
    $Label.text = type
    if checked:
        get_node("CheckBox").pressed = true

func _on_pressed():
    for type_button in get_parent().get_children():
        if type_button != self:
            type_button.get_node("CheckBox").pressed = false
    if !get_node("CheckBox").pressed:
        get_node("CheckBox").pressed = true
olivier-grech commented 5 years ago

Another small suggestion, I think you should indicate the number of AI players selected, this is not very clear right now. Something like this:

exampe

Though I am not sure if the leftmost option should be 1 or 0.

aaronfranke commented 5 years ago

Not only will you need to rebase, but you will also need to remove some files - I added .DS_Store to the .gitignore file.

LinuxDonald commented 5 years ago

It should be 0 if anyone want to play without ai.

LinuxDonald commented 5 years ago

@Jummit are you still working on this?

Jummit commented 5 years ago

@Jummit are you still working on this?

I sadly can't right now, because I have RSI and don't want it to get worse.

aaronfranke commented 5 years ago

Superseded by https://github.com/unknown-horizons/godot-port/pull/43