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

Added Highscore window #32

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

harshit-sharma-gits
Copy link

For issue #21

I've added the floating window for entering highscore, as mentioned in the issue description. Please take a look.

@harshit-sharma-gits
Copy link
Author

I've added the floating window for entering high score, but there seems to be some kind of bug. Whenever the game ends (and the current highscore is greater than the previous one) the window is supposed to pop up, instead the application crashes.

Would you please take a look?

@pulkomandy
Copy link
Member

Can you attach a Debugger debug report? It will help in investigating without having to compile and test the code

@harshit-sharma-gits
Copy link
Author

Here is the debug report

Copy link
Member

@pulkomandy pulkomandy left a comment

Choose a reason for hiding this comment

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

Your debug report will be a lot more useful if you compile the application in debug mode. I think you can use "make DEBUG=1" in the command line to do that. It should help seeing exactly where there is a problem.

HighscoreWindow.cpp Outdated Show resolved Hide resolved
HighscoreWindow.cpp Outdated Show resolved Hide resolved
WindowBoard.h Outdated Show resolved Hide resolved
@harshit-sharma-gits
Copy link
Author

harshit-sharma-gits commented Oct 10, 2022

Your debug report will be a lot more useful if you compile the application in debug mode

Sure, here is the debug report built in Debugging mode

HighscoreWindow.cpp Outdated Show resolved Hide resolved
@scottmc scottmc requested a review from humdingerb November 4, 2022 18:39
Copy link
Member

@humdingerb humdingerb left a comment

Choose a reason for hiding this comment

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

Just commenting on the visuals.

Plus:

  • The app crashes when closing the window.
  • In the main window, it'd be nice if the name of the current high-scoring person were shown as well as the high score.


HighscoreWindow::HighscoreWindow(const char* oldHighscorer, const int32 oldHighscore, const int32 newHighscore)
:
BWindow(BRect(200, 200, 600, 450), B_TRANSLATE("Enter you High Score"), B_TITLED_WINDOW, 0)
Copy link
Member

Choose a reason for hiding this comment

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

You don't actually enter your high score. Maybe something more celebratory as window title:

Suggested change
BWindow(BRect(200, 200, 600, 450), B_TRANSLATE("Enter you High Score"), B_TITLED_WINDOW, 0)
BWindow(BRect(200, 200, 600, 450), B_TRANSLATE("New high-score""), B_TITLED_WINDOW, 0)

Copy link
Member

Choose a reason for hiding this comment

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

The window doesn't need to be resizable or zoomable.

fInputBox->MakeFocus();

BStringView* line1 = new BStringView("line1", B_TRANSLATE("You achieved a new highscore, beating the one"));
BString line2Text = B_TRANSLATE("by %oldHighscorer% by %deltaScore%.");
Copy link
Member

Choose a reason for hiding this comment

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

Needs to take into account if there's no old high score.

line3Text.ReplaceFirst("%newhighscore%", std::to_string(newHighscore).c_str());
BStringView* line2 = new BStringView("line2", line2Text.String());
BStringView* line3 = new BStringView("line3", line3Text.String());

Copy link
Member

Choose a reason for hiding this comment

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

Why not just one BStringView with \n newlines?

It may be nicer to have the new high-score up at the top with the "Congratulations", maybe equally large font size, and everything center aligned:

Congratulations!
New high-score: 23000

You've beaten {UserLoser}'s
old record by 1234 points!

Please enter your name: [_______________]

Copy link
Member

Choose a reason for hiding this comment

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

How do you submit your name?
Maybe with a BButton below it all "Hurray!".

dospuntos added a commit to dospuntos/Haiku2048 that referenced this pull request Oct 10, 2023
dospuntos added a commit to dospuntos/Haiku2048 that referenced this pull request Oct 11, 2023
Top score bar stays put on window resize

Top score bar stays put on window resize (HaikuArchives#34)

Add files from pull request HaikuArchives#32

Update layout on high score window

Send BMessage to save score and name

Designing the highscore window

Redesign High-score window

Handle previous score and username

Handle no previous highscore

Remove unnecessary include

Upload new screenshot

Change high-score message
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants