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

Windowedfullscreen fix #5243

Merged

Conversation

Khaled-Dridi
Copy link
Contributor

this PR fixes #5228 when windowed full screen is pressed it will still show the task bar i fix that buy making it double click the windowed full screen so it will change its state twice (i used for loop for that)

How to test
go to setting go to video change to windowed then change to windowed full screen
because previously when you press windowed then press windowed fullscreen it will keep the task bar

Outstanding before merging
If anything. You can use neat checkboxes! Feel free to delete if not needed

[no ] Need to consider use case x
[no ] Still have to adjust the wiki doc
[no ] Will need translation work

Copy link
Contributor

@BenjaminAmos BenjaminAmos left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this! The changes here are mostly fine but I cannot accept supressing checkstyle warnings for no good reason. Please try and fix those first.

For future reference, you do not need to open a new pull request every time you make changes. You can just push new commits to this pull request's branch and they will been shown here.

@Khaled-Dridi
Copy link
Contributor Author

Thanks for fixing this! The changes here are mostly fine but I cannot accept supressing checkstyle warnings for no good reason. Please try and fix those first.

For future reference, you do not need to open a new pull request every time you make changes. You can just push new commits to this pull request's branch and they will been shown here.

can you rereview it I did what you told me to do and ty

@BenjaminAmos
Copy link
Contributor

I was all ready to approve this earlier but after testing it again it does not appear to work anymore? I was looking through the changes and it is puzzling. Did you test this version before submitting it?

The only difference I can see from your last pull request is that the following line is not run twice anymore. Could that be related?

GLFW.glfwSetWindowAttrib(window, GLFW.GLFW_DECORATED, GLFW.GLFW_FALSE);

@Khaled-Dridi
Copy link
Contributor Author

I was all ready to approve this earlier but after testing it again it does not appear to work anymore? I was looking through the changes and it is puzzling. Did you test this version before submitting it?

The only difference I can see from your last pull request is that the following line is not run twice anymore. Could that be related?

GLFW.glfwSetWindowAttrib(window, GLFW.GLFW_DECORATED, GLFW.GLFW_FALSE);

yes i did test it and it worked for me i will check it again when i am next to my laptop

@Khaled-Dridi
Copy link
Contributor Author

@BenjaminAmos hi there i found the problem and i posted a new commit if you can check it that would be great

@BenjaminAmos BenjaminAmos merged commit 81b53f4 into MovingBlocks:develop May 7, 2024
10 checks passed
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.

Bug on trying to change display settings to windowed fullscreen
2 participants