Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Seerat/241 country flag selection #247

Merged
merged 6 commits into from
Jul 23, 2018

Conversation

SeeratSek
Copy link
Collaborator

🎟️ Ticket(s): Closes #241


👷 Changes

  • Added drop down select menu for selection of countries
  • Added flag emojis to drop down as well
  • The change is also reflected on the server as Country attribute for the Player so that it gets saved on death of player

🔦 Testing Instructions

Can test by pulling the branch changes and testing the dropdown as well as testing whether dying causes the country to persist.

@coveralls
Copy link

coveralls commented Jul 21, 2018

Coverage Status

Coverage decreased (-0.1%) to 65.927% when pulling a3f1f4c on seerat/241-country-flag-selection into 7dbb6a6 on master.

@SeeratSek SeeratSek force-pushed the seerat/241-country-flag-selection branch from 5c7de7e to 87c2ee2 Compare July 21, 2018 23:20
@SeeratSek SeeratSek added the ready for review This issue or pull request is ready to be reviewed label Jul 21, 2018
"VE": "Venezuela, Bolivarian Republic of",
"VG": "Virgin Islands, British",
"VI": "Virgin Islands, U.S.",
"VN": "Viet Nam",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never knew Vietnam was spelled like this 😩


err := g.Arena.SpawnPlayer(id, spawn.Name)
fmt.Println(spawn.Country)
err := g.Arena.SpawnPlayer(id, spawn.Name, spawn.Country)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this print was for debugging, can you remove it? :)

@@ -38,6 +38,7 @@ type KeysPressed struct {
type Player struct {
Name string `json:"name"`
ID string `json:"id"`
Country string `json:"country"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 👍

Object.keys(countries).forEach((key) => {
listOfCountries.push(<option value={countries[key]} key={key}>{countries[key]} {key.toUpperCase().replace(/./g, char => String.fromCodePoint(char.charCodeAt(0) + 127397))} </option>);
});
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this very much

brian-nguyen
brian-nguyen previously approved these changes Jul 22, 2018
Copy link
Collaborator

@brian-nguyen brian-nguyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really good work. Glad the emojis turned out to save us a lot of time and effort. Just that one comment about the debug print statement, otherwise feel free to merge this whenever you're ready!

Copy link
Collaborator

@brian-nguyen brian-nguyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@SeeratSek SeeratSek merged commit 20b01a6 into master Jul 23, 2018
@brian-nguyen brian-nguyen deleted the seerat/241-country-flag-selection branch July 23, 2018 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review This issue or pull request is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants