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

Event::operator bool #2968

Merged
merged 1 commit into from
May 3, 2024
Merged

Conversation

trustytrojan
Copy link
Contributor

Description

This PR aims to simplify the usage of Event when polling events from a RenderWindow. Instead of having to initialize an Event on the stack and pass it by reference to pollEvent/waitEvent, this PR makes the methods return an Event, which can be converted to a bool based on whether its subtype is Empty.

There was discussion about having pollEvent and waitEvent return std::optional<Event>, but considering Event already uses subtyping to determine if it contains event data, letting Event be convertible to bool (based on whether the inner data was of type Event::Empty) sounded like the better option. Plus, we can avoid the clunky -> syntax used for accessing members of an std::optional's inner type.

Tasks

  • Tested on Linux
  • Tested on Windows
  • Tested on macOS
  • Tested on iOS
  • Tested on Android

How to test this PR?

Use an sf::RenderWindow while processing events.

#include <SFML/Graphics.hpp>

int main()
{
    sf::RenderWindow window(sf::VideoMode({1280, 720}), "Minimal, complete and verifiable example");
    window.setFramerateLimit(60);

    while (window.isOpen())
    {
        while (const auto event = window.pollEvent())
        {
            if (event.is<sf::Event::Closed>())
                window.close();
        }

        window.clear();
        window.display();
    }
}

@ChrisThrasher ChrisThrasher added this to the 3.0 milestone May 2, 2024
Copy link
Member

@ChrisThrasher ChrisThrasher left a comment

Choose a reason for hiding this comment

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

I like this! It's great to remove output parameters without adding any extra ergonomic issues.

include/SFML/Window/Event.hpp Outdated Show resolved Hide resolved
@ChrisThrasher ChrisThrasher changed the title Event::operator bool: take 2 Event::operator bool May 2, 2024
@coveralls
Copy link
Collaborator

coveralls commented May 2, 2024

Pull Request Test Coverage Report for Build 8934117997

Details

  • 12 of 14 (85.71%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 51.897%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/SFML/Window/WindowImpl.cpp 0 2 0.0%
Totals Coverage Status
Change from base Build 8932890594: 0.01%
Covered Lines: 10474
Relevant Lines: 19002

💛 - Coveralls

@ChrisThrasher
Copy link
Member

Cleaned up the commit history to prepare for merge. No changes made.

@ChrisThrasher ChrisThrasher force-pushed the event-operator-bool branch 2 times, most recently from 7cb7f96 to 00d69ac Compare May 3, 2024 04:06
@trustytrojan
Copy link
Contributor Author

trustytrojan commented May 3, 2024

Looks like a macos build failed because it took too long to clone? Any way to re-trigger it?

@eXpl0it3r
Copy link
Member

Nothing you can do, I've rerun the failed job.

@ChrisThrasher ChrisThrasher merged commit ca0a231 into SFML:master May 3, 2024
113 checks passed
@trustytrojan trustytrojan deleted the event-operator-bool branch May 4, 2024 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants