fix: avoid writing window size of 0 to the config Re: #4601

[mahela97]

Contains

This PR will fix the Bug mentioned here #4596.

Approach - As in the suggestions in #4601, Now the config file doesn't end up writing zeros to the config.

How to test

Close the game when it is minimized and open the game again. Game window resolution will set to default values.

[DarkWeird]

you can just append commits to your previous PR, or rewrite them with push force

[mahela97]

you can just append commits to your previous PR, or rewrite them with push

Oh, I made separate PR because @keturn told me to work on this on a separate one. Shall I append this on the previous PR or keep this?

[DarkWeird]

you can just append commits to your previous PR, or rewrite them with push

Oh, I made separate PR because @keturn told me to work on this on a separate one. Shall I append this on the previous PR or keep this?

Oh, Ok :)

[skaldarnar]

For reference, #4160 sets the default resolution in the config and also explains where this size is coming from.

I thought there might be another place where we have these constants defined, but it seems that it completely comes from the config - which seems brittle enough. If, for some reason, the config contains (0, 0) as value the fallback solution does not work 😕

Therefore, I do think that preventing this in the rendering config directly is a better way to solve this.

(for the record, the way you retrieve the "default" here is not wrong, it's just that we can find a better solution 🙃 )

[mahela97]

For reference, #4160 sets the default resolution in the config and also explains where this size is coming from.

I thought there might be another place where we have these constants defined, but it seems that it completely comes from the config - which seems brittle enough. If, for some reason, the config contains (0, 0) as value the fallback solution does not work 😕

Therefore, I do think that preventing this in the rendering config directly is a better way to solve this.

(for the record, the way you retrieve the "default" here is not wrong, it's just that we can find a better solution 🙃 )

As I think The bug occurs only if the config contains (0,0). The only way to get (0,0) is if we close the game while it is minimized. (Other than hardcoded the config file).
Also when I am retrieving the default values it retrieves the value which was the resolution of the last game session. (Not from the #4160. So actually they are not the default values. The bug is last game session is closed when it is minimized. So if that happens my PR will ignore the (0,0) and keep the config file unchanged.)
I tried to fix from the rendering config but I felt like this is the efficient way. 🙃🙃
So could you please check this again? 😄 Thank you

[keturn]

Ah, so we only set this on shutdown? Instead of every time the window is moved? Good to know. Only have to worry about it being written one time in that case.

[keturn]

If we have no valid data for this, what if we skip these config.setWindow* calls entirely in that case? Would they keep their old values?

Other suggestion: make the the conditional on zero width or height, not and.

[mahela97]

Yes, that would keep the old values(Default as in #4160). But I suggest keeping it because it gives the user to adjust their screen size and position as they want and keep it so they do not want to re-adjust every time they open the game.

And yes. && should be ||. Thank you. My bad. :)

[keturn]

I'm suggesting we do write the values when we have valid values, but skip it in the 0 case.

[mahela97]

oh ok.... you are suggesting
if (widthBuffer[0]>0 && heightBuffer[0]>0 && xBuffer[0]>0 && yBuffer[0]>0){ config.setWindowWidth(widthBuffer[0]); config.setWindowHeight(heightBuffer[0]); config.setWindowPosX(xBuffer[0]); config.setWindowPosY(yBuffer[0]); }

Yes this is better. I tried it and working fine too. :)
So we don't need default x and y values.

[mahela97]

Ah, so we only set this on shutdown? Instead of every time the window is moved? Good to know. Only have to worry about it being written one time in that case.

Yes, it is just one time when the user closes the game.

[keturn]

replaced by #4619 (because this PR's branch went away)