Remove 'Empty' state from 'Event', rely on 'optional'

[vittorioromeo]

Proof of concept right now as I don't have access to a compiler, but I think this is a superior design compared to #2991.

Removing the empty state from event results in a much more type-safe API. Events cannot be default-constructed anymore, and APIs that poll events now return an optional event, making it clear that an empty state can be present without polluting usages of events that cannot be in such an empty state.

@ChrisThrasher: feel free to continue working on this branch to adjust the usages of event in tests and examples.

[ChrisThrasher]

I considered this in the past and decided I didn’t like it because it requires excessive operator-> usage. I find the current operator bool API more ergonomic even if it’s less ideologically pure.

[ChrisThrasher]

master...optional_events

Here's my branch from back in November. I looked at this and didn't think it was worth doing. To avoid lots of operator-> usage I had to create a new intermediate variable that unwrapped the optional.

[coveralls]

Pull Request Test Coverage Report for Build 9066610392

Details

• 3 of 44 (6.82%) changed or added relevant lines in 4 files are covered.
• 2 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-0.09%) to 51.811%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/SFML/Window/DRM/WindowImplDRM.cpp	0	2	0.0%
src/SFML/Window/WindowImpl.cpp	0	4	0.0%
src/SFML/Window/WindowBase.cpp	3	11	27.27%
src/SFML/Window/DRM/InputImpl.cpp	0	27	0.0%

Files with Coverage Reduction	New Missed Lines	%
src/SFML/Window/WindowBase.cpp	1	70.29%
src/SFML/Window/DRM/InputImpl.cpp	1	0.0%

Totals
Change from base Build 9057848497:	-0.09%
Covered Lines:	10464
Relevant Lines:	18991

---

💛 - Coveralls

[L0laapk3]

What are the advantages of this over std::monostate?

[vittorioromeo]

What are the advantages of this over std::monostate?

Monostate is just another symbol for ab empty state, which is a state a valid event should never be in.

As @ChrisThrasher mentioned in his PR, event shouldn't even have an empty state and most importantly such a state should not be public and accessible to users.

The reason why we had an empty state to begin with is because some APIa such as pollEvent need to return immediately and tell the user that no event was polled.

In C++17 we have an idiomatic way of expressing the possible absence of a result: optional. We are using that throughout the library already and using it for events solves the "public empty state but not really" issue and, even better, makes event a type that's always guaranteed to be valid and non empty.

[L0laapk3]

... since users already should not be using sf::Event::Empty.

@ChrisThrasher Can you explain the rationale behind this? What is wrong with sf::Event::Empty? Is it just about having to implement similar safeties to std::monostate?

[vittorioromeo]

... since users already should not be using sf::Event::Empty.

@ChrisThrasher Can you explain the rationale behind this? What is wrong with sf::Event::Empty? Is it just about having to implement similar safeties to std::monostate?

It's a meaningless implementation detail. Operating systems or windowing systems do not produce empty events. We simply needed a sentinel to signal "hey, you tried to poll events but nothing was available" in user loops.