Skip to content

Replace the default constructor of sf::Text with one that takes an sf::Font #2194

Description

@therocode

Inspired by a case of a user asking the Discord server for help in why their text was not displaying despite them using setString setStyle setFillColor setCharacterSize. They lacked setFont.

This suggests a breaking change, so I propose this be added to SFML 3. The change is trivial so doesn't take much work, but provides safety and a clearer API for the user.

Proposal

Change the default constructor of sf::Text from:
Text()
Into:
Text(const Font& font, unsigned int charachterSize = 30)

The reasoning is that allowing the user to create an sf::Text with no font is not useful, and opens up for confusion and bugs since rendering an sf::Text with no font will just display nothing. Especially with SFML's history of providing a default font, and also with many beginners not realising that a font needs to be set, this opens up a pitfall.

By removing the ability to even create an sf::Text in this state removes that class of bugs and goes along good API practices of making APIs harder to use without compromising their usefulness.

Drawback

The only obvious rebuttal I see to this are things like:

  • But I want to put uninitialised sf::Text in an std::vector
  • But I create sf::Texts in my engine that are going to reference not-yet-loaded sf::Fonts and I'll use setFont when the time comes
  • ...similar...

These all boil down to "Sometimes it is useful to be able to construct uninitialised values for later use". This is true, and I'd argue the proper modern C++ way of doing this would be through std::optional<sf::Text> or std::unique_ptr<sf::Text> or similar, which to some might seem more verbose but this is a good type of verbose as it:

  • Makes clear which usages of sf::Text might be uninitialised (currently ALL usages of sf::Text could be, which is far from the default case)
  • Uses well tried and tested standard provided methods of creating uninitialised values that communicates intent and provides helper methods for handling it.
  • Makes it hard to accidentally come across uninitalised text objects, in contract to currently when ANY sf::Text opaquely might be not ready for use.

With this in mind, I personally don't see any further drawbacks with this change.

Compile errors

@ChrisThrasher did a quick test to build SFML with the default constructor removed, and there was an error in the joystick code where it seems like it uses an uninitialised instane of sf::Text. This is hopefully trivially fixed by changing it to std::optional<sf::Text>.

Summary

All in all, I think this would be a great change since it adds seatbelts against an issue that might otherwise trip up unexpecting users. It brings the API closer to being safe and modern, utilising the type system for bulletproof eradication of the described bugs. The change is seemingly easy to apply and I don't see any unaddressed drawbacks.

Interested in hearing the thoughts from the regular SFML maintainers in case I've missed something.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions