Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (ddb906a) 42.97% compared to head (9516a5e) 43.02%.

@@            Coverage Diff             @@
##           master    #2818      +/-   ##
==========================================
+ Coverage   42.97%   43.02%   +0.04%     
==========================================
  Files         252      252              
  Lines       21099    21113      +14     
  Branches     5172     5177       +5     
==========================================
+ Hits         9067     9083      +16     
+ Misses      11221    11093     -128     
- Partials      811      937     +126     

Exploiter and I talked about whether the order of style and state should be reversed. I put state after style for no particular reason. I'm not sure if one parameter order makes more sense than the other.

Rebased on master. No changes made.

Exploiter and I talked about whether the order of style and state should be reversed. I put state after style for no particular reason. I'm not sure if one parameter order makes more sense than the other.

Since it's more likely you want to adjust style than state by default, I'd go with style first.

While I didn't fully realize or verbalize in the past discussion, I think the main issue is, that you currently have to set a style for fullscreen window, even though the fullscreen window won't have a style.
By reversing the order, you could switch to fullscreen without providing a style (if you don't need a context).
The issue might be slightly mitigated by providing dedicated functions to switch to fullscreen mode without recreating the window.

Currently asking myself if we just need another overload (VideoMode, Title, State = default, ContextSettings = default), which has the style set to the default style.

Maybe it goes a bit into off-topic (we can gladly discuss it somewhere else), but did we ever consider a fluent builder API for these lengthy constructors?

Instead of

sf::RenderWindow window(
    sf::VideoMode({800, 600}),
    "SFML graphics with OpenGL",
    sf::Style::Default,
    sf::State::Windowed,
    contextSettings);

we'd have

auto window = sf::RenderWindow::builder()
    .videoMode({800, 600})
    .title("SFML graphics with OpenGL")
    .state(sf::State::Windowed),
    .context(contextSettings)
    .build();
// no .style() needed if it's the default

It may be a bit unidiomatic for SFML, but it would solve the "anything can be default" problem. Would also allow overloads on a per-parameter basis, instead of providing a whole new constructor.

Rebased on master and fixed conflicts

I implemented addition constructor overloads that let users specify a state without specifying a style. While I was looking at the code I found some issues with the docs that needed correcting and added some missing tests so the PR is overall in a better state now.

I'm not sure if we can add extra create() overloads because when I tried that I got compiler errors about hiding base class virtual functions. At least we can add the constructors though.

Periodic rebase on master. No changes made.

Added missing constructor overload to sf::RenderWindow so it matches the constructors that sf::Window and sf::WindowBase have.