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

Disallow system updates on closing windows (retry) #2456

Merged
merged 2 commits into from Feb 12, 2021
Merged

Disallow system updates on closing windows (retry) #2456

merged 2 commits into from Feb 12, 2021

Conversation

ghost
Copy link

@ghost ghost commented Feb 9, 2021

This PR is a retry of #2452. I had overreacted and prematurely closed the old PR. After getting a clear mind I decided to try it again. This contains the fix intended by #2451 too.

rueter37 added 2 commits February 10, 2021 09:22
System changes are now applied after the window has been closed.
The system graphic is no longer changed in the load menu before the
fadeout happens.
Copy link
Contributor

@fmatthew5876 fmatthew5876 left a comment

Choose a reason for hiding this comment

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

I think Ghabry proposed solution of removing the update logic entirely and testing it is likely best solution, assuming it works.

Regardless, this is clearly better than the current code and more correct. So this PR gets my approval.

@fmatthew5876 fmatthew5876 added the UX For issues affecting the user experience, such annoyances, counter-intuitive or ugly design label Feb 11, 2021
@Ghabry Ghabry merged commit 8ee5e9e into EasyRPG:master Feb 12, 2021
@Ghabry Ghabry added this to the 0.6.3 milestone Feb 12, 2021
@ghost ghost deleted the window-closing-no-system-update branch February 14, 2021 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UX For issues affecting the user experience, such annoyances, counter-intuitive or ugly design
Development

Successfully merging this pull request may close these issues.

None yet

3 participants