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.
Inspired by a case of a user asking the Discord server for help in why their text was not displaying despite them using
setStringsetStylesetFillColorsetCharacterSize. They lackedsetFont.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::Textfrom:Text()Into:
Text(const Font& font, unsigned int charachterSize = 30)The reasoning is that allowing the user to create an
sf::Textwith no font is not useful, and opens up for confusion and bugs since rendering ansf::Textwith 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::Textin 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:
sf::Textin anstd::vectorsf::Texts in my engine that are going to reference not-yet-loadedsf::Fonts and I'll usesetFontwhen the time comesThese 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>orstd::unique_ptr<sf::Text>or similar, which to some might seem more verbose but this is a good type of verbose as it:sf::Textmight be uninitialised (currently ALL usages ofsf::Textcould be, which is far from the default case)sf::Textopaquely 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 tostd::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.