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

Fix getText() function in UIText #3689

Merged
merged 3 commits into from Jun 15, 2019

Conversation

@darshan3
Copy link
Member

commented Jun 14, 2019

Contains

Fixes #3688

How to test

Start from a clean game with no save games. Try reproducing #3688, in both the ways stated over there. The new game should load properly now.

@darshan3

This comment has been minimized.

Copy link
Member Author

commented Jun 14, 2019

The getText() function was incorrectly adding a '\n' character, which leads to parsing error GameProvider.getNextGameName(). Hence, when it incorrectly returns the game name as 'Game 1' even if saved games are present with the same name. This leads to further errors in loading and saving the game in the same directory.

@darshan3

This comment has been minimized.

Copy link
Member Author

commented Jun 14, 2019

I would also request @jellysnake or @eviltak to have a look once since I don't know how multiline text works and haven't tested it out for the changes in this pr.

if (linesOfText.length > 0) {
String arrayText = String.join("\n", linesOfText);
if (text.get().equals("")) {
return text.get() + "\n" + arrayText;

This comment has been minimized.

Copy link
@skaldarnar

skaldarnar Jun 14, 2019

Member

In this case text.get() is the empty string "" - why are we prepending a newline here? Should it rather be:

 if (text.get().equals("")) {
    return arrayText;
} else {
    return text.get() + "\n" + arrayText;
}

This comment has been minimized.

Copy link
@darshan3

darshan3 Jun 14, 2019

Author Member

My mistake, it should be !text.get().equals(""). I will fix it.

This comment has been minimized.

Copy link
@darshan3

darshan3 Jun 14, 2019

Author Member

Should I just return arrayText instead of text.get() + arrayText? There was a discussion in original pr #3678 regarding this but resolved eventually.

@Cervator Cervator merged commit 6ab352f into MovingBlocks:develop Jun 15, 2019

1 check passed

LGTM analysis: Java No new or fixed alerts
Details
Cervator added a commit that referenced this pull request Jun 15, 2019

@Cervator Cervator added this to the v2.3.0 milestone Jun 15, 2019

@Cervator Cervator added the Bug label Jun 15, 2019

@Cervator

This comment has been minimized.

Copy link
Member

commented Jun 15, 2019

Tested out a fair bit - merging! :-)

Since this fixes a blocker I figure merge sooner, ask more questions later if needed. Existing saves may still cause problems even after the fix (since I guess a bad name might persist that way) but in a clean workspace I didn't see any issues colliding with new saves.

@eviltak eviltak referenced this pull request Jul 14, 2019
0 of 1 task complete
@eviltak eviltak referenced this pull request Jul 26, 2019
eviltak added a commit that referenced this pull request Jul 26, 2019
Revert "Merge PR #3689 by @darshan3 - UI text fix"
This reverts commit d2e2b34, reversing
changes made to 67024bd.
@eviltak

This comment has been minimized.

Copy link
Member

commented Jul 26, 2019

Reverted with #3678.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.