Skip to content

Log the exception inside flush and fix NullPointerException if LocalStorage is not supported#5838

Merged
MobiDevelop merged 3 commits into
libgdx:masterfrom
SimonIT:gwt-log-flush-exception
Nov 27, 2019
Merged

Log the exception inside flush and fix NullPointerException if LocalStorage is not supported#5838
MobiDevelop merged 3 commits into
libgdx:masterfrom
SimonIT:gwt-log-flush-exception

Conversation

@SimonIT

@SimonIT SimonIT commented Nov 10, 2019

Copy link
Copy Markdown
Member

We experienced some "Couldn't flush preferences" erros. The cause seem to be that getLocalStorageIfSupported can return null if the browser doesn't support it or "block third-party cookies" is enabled. Then in flush, GwtPreferences tries to access to LocalStorage but it's null.
Logging the original exception is helpful to get causes of problems.
Also removed the unnecessary creation of objects from the same type in toObject (the constructors are deprecated since java 9)

…s from the same type in toObject (the constructors are also deprecated since java 9)
@SimonIT SimonIT changed the title Log the exception inside flush Log the exception inside flush and fix NullPointerException if LocalStorage is not supported Nov 10, 2019
@intrigus

intrigus commented Nov 10, 2019

Copy link
Copy Markdown
Contributor

Why should flush not throw an exception when local storage is not supported?

@SimonIT

SimonIT commented Nov 10, 2019

Copy link
Copy Markdown
Member Author

It makes the whole game unplayable if the exception occurs. Maybe an IOException would be better? So the developer know and can handle the case of not saveable?

@MrStahlfelge

Copy link
Copy Markdown
Member

Because of backwards compatibility (in the past, possibility to save was granted) and to keep it conform with the other platforms. There's also nothing to gain here. Handling the exception yourself has no value.

Additionally, it is completely unexpected that the execption is thrown on flush. The moment I would expect it is when the object is loaded with Gdx.app.getPreferences.

@MrStahlfelge MrStahlfelge left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested on Chrome with blocking cookies. Works like a charm. Pulled the fix in my GWT backend fork, thanks.

@SimonIT

SimonIT commented Nov 18, 2019

Copy link
Copy Markdown
Member Author

Thanks for testing and approving

@MobiDevelop

Copy link
Copy Markdown
Member

Looks okay to me.

@MobiDevelop MobiDevelop merged commit c0c1f51 into libgdx:master Nov 27, 2019
@SimonIT SimonIT deleted the gwt-log-flush-exception branch November 27, 2019 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants