1
Show Posts
This section allows you to view all posts made by this member. Note that you can only see posts made in areas you currently have access to.
2
SFML development / Re: Introduce chrono to sf::Time
« on: August 25, 2022, 08:33:18 pm »
In my opinion, the Chrono API is absolutely horrible from a usability point of view. The genericity is rarely needed, because 99% of durations in software fall into human-observable scale. For those, a default implementation would not only suffice, but make life a lot easier.
You also see that every other mainstream language has converged towards one duration type; e.g. Java took several attempts but their latest Duration/Instant pair is quite a nice and intuitive API. Even lower level languages like Rust decided to go with one type. That doesn't mean they're rightTM of course, but it's something to keep in mind.
I don't know what the rationale for genericity is; to me, it falls into classic Boost "what if the user wants quantum scale" kind of over-engineering -- actively making the API worse for the majority of users, in order to satisfy some corner cases. On top of that, SFML in particular operates at a very narrow and specific time scale ranging from milliseconds to seconds, in very rare cases hours or days.
This is why I think a simpler abstraction on top of Chrono has its merits -- this is only my personal opinion however.
More comments on the topic:
You also see that every other mainstream language has converged towards one duration type; e.g. Java took several attempts but their latest Duration/Instant pair is quite a nice and intuitive API. Even lower level languages like Rust decided to go with one type. That doesn't mean they're rightTM of course, but it's something to keep in mind.
I don't know what the rationale for genericity is; to me, it falls into classic Boost "what if the user wants quantum scale" kind of over-engineering -- actively making the API worse for the majority of users, in order to satisfy some corner cases. On top of that, SFML in particular operates at a very narrow and specific time scale ranging from milliseconds to seconds, in very rare cases hours or days.
This is why I think a simpler abstraction on top of Chrono has its merits -- this is only my personal opinion however.
More comments on the topic:
3
C / Re: When will setSmooth be implemented for Fonts?
« on: July 06, 2022, 12:04:57 am »
Mostly, the CSFML binding is lagging behind the C++ SFML project, so some features may not be available yet.
I don't see why Font::setSmooth() should not be possible to implement for CSFML; it's probably more that no one has had the time to do so yet. If this is something you need, you are welcome to contribute with a pull request in the CSFML repository
I don't see why Font::setSmooth() should not be possible to implement for CSFML; it's probably more that no one has had the time to do so yet. If this is something you need, you are welcome to contribute with a pull request in the CSFML repository
4
SFML development / Re: Curious why VertexBuffer implementation for Drawables was reverted?
« on: May 25, 2022, 09:14:53 pm »
See also:
I didn't find any discussion regarding the revert itself (beyond the one eXpl0it3r mentioned).
I didn't find any discussion regarding the revert itself (beyond the one eXpl0it3r mentioned).
5
SFML development / Re: SFML Roadmap
« on: March 31, 2022, 05:36:42 pm »Looks good, but you should definitely plan smaller releases.Generally I agree, during "stable" times this is the way to go and we have neglected this to some extent.
During a huge breaking change however (SFML 3), many small releases directly conflict with Semver, which is also good industry practice. I know that many libraries don't care about this, and it makes everyone's life harder and is one big reason why dependency management is such a mess in C++ (another reason being lack of standard tooling).
If we had released during the SFML 2->3 period, we would have had two options:
- Stay true to Semver and release a major version many times for small changes. This would quickly turn our versioning schema into a meme.
- Don't care about Semver after we've taken it pretty seriously for the entirety of SFML 2. This would not only send a questionable message, but might also ruin the day of many developers going into SFML 3 with the same stability expectations as they had previously.
When game developers prefer outdated releases over Git versions due to policies, that's a reasonable strategy when it comes to minimize the number of code adaptations (at the cost of one bigger change taking more time). But it's ultimately their decision, they are free to use an in-development version to benefit from cutting edge changes (at the risk of bugs and API changes). So everyone can choose a trade-off acceptable to them.
TLDR: it's a matter of communication -- I think the current model makes it relatively clear that the Git version is "in flux" while releases are stable.
6
C / Re: C API Limitations
« on: March 19, 2022, 10:46:53 pm »
When looking at sfWindow and sfRenderWindow, they are defined like this:
This is already the first problem -- we store a concrete sf::Window value, which is not polymorphic. Instead, we should store sf::Window*, which can point to a sf::RenderWindow behind the scenes (more about memory management later).
Once we have that, an upcast could be implemented:
Memory management
Memory management and ownership makes this a bit trickier. Normally, every sfWindow is created using its create and destroyed using its destroy function. Manual, but very straightforward semantics.
Now, it would be possible that a sfWindow actually points to a sfRenderWindow -- so who should destroy it? One could attempt some std::shared_ptr tricks, however that doesn't work well with C -- copying a struct is always a bytewise copy, so ref-counts would need to be managed manually. There is no win with smart pointers here.
We have two options: either we allow polymorphic destruction in sfWindow_destroy or we don't.
If we don't, we should probably have a runtime check like:
(Not sure if this works dependency-wise; since RenderWindow is in Graphics. Of course, there could in the future also be other derived classes. Maybe typeid would be safer, or a bool flag sfWindow::isUpcast).
struct sfWindow
{
sf::Window This;
};
struct sfRenderWindow
{
sf::RenderWindow This;
sfView DefaultView;
sfView CurrentView;
};
{
sf::Window This;
};
struct sfRenderWindow
{
sf::RenderWindow This;
sfView DefaultView;
sfView CurrentView;
};
This is already the first problem -- we store a concrete sf::Window value, which is not polymorphic. Instead, we should store sf::Window*, which can point to a sf::RenderWindow behind the scenes (more about memory management later).
Once we have that, an upcast could be implemented:
sfWindow* sfRenderWindow_upcast(sfRenderWindow* renderWindow)
{
sfWindow window; // assuming C++03 no init-list
window.This = &renderWindow->This; // 'This' has type sf::Window*
return window;
}
{
sfWindow window; // assuming C++03 no init-list
window.This = &renderWindow->This; // 'This' has type sf::Window*
return window;
}
Memory management
Memory management and ownership makes this a bit trickier. Normally, every sfWindow is created using its create and destroyed using its destroy function. Manual, but very straightforward semantics.
Now, it would be possible that a sfWindow actually points to a sfRenderWindow -- so who should destroy it? One could attempt some std::shared_ptr tricks, however that doesn't work well with C -- copying a struct is always a bytewise copy, so ref-counts would need to be managed manually. There is no win with smart pointers here.
We have two options: either we allow polymorphic destruction in sfWindow_destroy or we don't.
If we don't, we should probably have a runtime check like:
extern "C" void sfWindow_destroy(sf::Window *window) {
assert(dynamic_cast<sf::RenderWindow*>(window.This) == nullptr);
delete window.This;
delete window;
}
assert(dynamic_cast<sf::RenderWindow*>(window.This) == nullptr);
delete window.This;
delete window;
}
(Not sure if this works dependency-wise; since RenderWindow is in Graphics. Of course, there could in the future also be other derived classes. Maybe typeid would be safer, or a bool flag sfWindow::isUpcast).
7
General / Re: SFML adding a menu bar to my project
« on: January 16, 2022, 11:52:10 am »From my understanding, I need an extra panel on top of my preexisting window, wherein I will add the buttons under a certain layout. So maybe extend the height of my window, and draw my chess board with a vertical offset downwards?Yes, that's one option. Another would be to keep drawing the board from Y=0 downwards, and set up a sf::View which begins at negative Y.
But then, in my chess game I use theIn general, you should differentiate between "viewport coordinates" (like mouse) and "world coordinates" (your chess board). Always translate from one to the other via mapPixelToCoords(), don't just static_cast. Then, it's also very easy to add custom logic in just one place instead of dozens.event.mouseButton.ycoordinates all the time so will I have to subtract the height offset? That seems like an incorrect approach.
8
General discussions / Re: about inheriting sf::Drawable and sf::Sprite as property.
« on: January 13, 2022, 11:19:34 am »
The entire source code of the book is also available here:
https://github.com/SFML/SFML-Game-Development-Book
It helps understand the context. The code is separated by chapters, so you're not overwhelmed with content the book hasn't taught yet.
https://github.com/SFML/SFML-Game-Development-Book
It helps understand the context. The code is separated by chapters, so you're not overwhelmed with content the book hasn't taught yet.
9
SFML projects / Re: Nero Game Engine
« on: December 18, 2021, 09:05:20 pm »
I haven't followed Nero up for a while, but there's some really nice progress!
Are you still using ImGUI for the UI? Are you happy with it, and the concept of immediate-mode GUIs in general?
Are you still using ImGUI for the UI? Are you happy with it, and the concept of immediate-mode GUIs in general?
10
SFML development / Sensible use of certain C++17 features
« on: December 08, 2021, 11:06:29 am »
While doing the C++17 migration, it's probably good if we settle down on some conventions regarding the use of newer C++ features. A lot of features are intended for a specific purpose and highly useful within that scope, but not intended as a broadband catch-all measure to be thoughtlessly applied to all code.
I'd suggest to be pragmatic during the migration, which means: we avoid unnecessary changes to existing code where benefits are mostly theoretical, and we focus on C++17 features which directly benefit the user (the currently ongoing PRs are a good example of this! )
A non-exhaustive list + my opinions:
[[nodiscard]]
From this comment:
[[nodiscard]] is primarily useful to prevent mistakes in non-obvious cases where a return value could be accidentally ignored. For example:
Same for methods which could be interpreted in a different way due to English language ambiguity:
In other words: "should the caller use the return value" is a necessary, but not sufficient criterion for [[nodiscard]].
95% of all functions which have a return value intend that to be used. We don't want that attribute on functions like Sprite::getPosition() or Sound::isRelativeToListener(), even if they'd technically qualify. Why not?
constexpr
See also this comment.
Similarly, we could apply constexpr to a ton of APIs. But it comes again at a cost:
noexcept
Primarily useful on move constructors, swap, destructors.
Benefit outside such special methods is questionable in SFML.
explicit
Useful for constructors that can act as conversion constructors (callable with 1 argument) or conversion operators.
Uniform initialization
SFML currently uses (a) for implicit conversions or single-value initializations and (b) for constructor calls:
I think we should stay with this convention, this also reduces amount of code to be changed for cosmetic reasons.
What we can do however is:
Type inference (auto + decltype)
We should use auto where it helps increase readability. Iterator declarations are such an example, also repetition of type names are potential candidates:
I would not use it where the type information helps the understanding of the code, or in function signatures or return types. It's also possible to introduce bugs by propagating a wrong type through multiple type-inferred functions.
decltype is typically used in generic code when it's hard to deduce the type of an expression. I don't think SFML has many places where it's needed.
Emplacement
Good if heavy copies can be avoided (moving SFML resource classes into a container). Otherwise, similar thoughts as in type inference section apply: it can make some code harder to understand, while not making a notable difference (due to optimization of temporary objects, or simply small enough sizes). Depends a bit on the context, good variable names can also mitigate that.
We should particularly avoid emplace_back(), when push_back() does the job (that will just cause an extra copy constructor call).
Trailing return types
In my opinion only useful when the return type depends on the parameter types. As such, we should limit their use to generic code which truly needs them.
I'd suggest to be pragmatic during the migration, which means: we avoid unnecessary changes to existing code where benefits are mostly theoretical, and we focus on C++17 features which directly benefit the user (the currently ongoing PRs are a good example of this!
)A non-exhaustive list + my opinions:
[[nodiscard]]
From this comment:
[[nodiscard]] is primarily useful to prevent mistakes in non-obvious cases where a return value could be accidentally ignored. For example:
sf::Image image;
image.loadFromFile(...);
// WARN: didn't check 'bool' return type!
image.loadFromFile(...);
// WARN: didn't check 'bool' return type!
Same for methods which could be interpreted in a different way due to English language ambiguity:
std::vector<T> container = ...;
container.empty(); // empty the container
// WARN: this is an adjective, not a verb!
container.empty(); // empty the container
// WARN: this is an adjective, not a verb!
In other words: "should the caller use the return value" is a necessary, but not sufficient criterion for [[nodiscard]].
95% of all functions which have a return value intend that to be used. We don't want that attribute on functions like Sprite::getPosition() or Sound::isRelativeToListener(), even if they'd technically qualify. Why not?
- It makes APIs less readable, making function signatures more complex without adding meaningful information
- The cases where where [[nodiscard]] is justifiably used are no longer recognized as such, thus the attribute loses its intended meaning.
- We should prevent likely errors while not sacrificing ergonomics for unlikely ones.
constexpr
See also this comment.
Similarly, we could apply constexpr to a ton of APIs. But it comes again at a cost:
- more complex API signatures
- the necessity to move implementation to the header (which can in turn require exposing dependencies in headers)
- restricted freedom in implementation (the function can no longer just compute values at runtime, cache them, or similar) -- removing constexpr becomes a breaking change
noexcept
Primarily useful on move constructors, swap, destructors.
Benefit outside such special methods is questionable in SFML.
explicit
Useful for constructors that can act as conversion constructors (callable with 1 argument) or conversion operators.
Uniform initialization
int value = 3; // (a)
int value(3); // (b)
int value{3}; // (c)
int value = {3}; // (d)
int value(3); // (b)
int value{3}; // (c)
int value = {3}; // (d)
SFML currently uses (a) for implicit conversions or single-value initializations and (b) for constructor calls:
int value = 3;
sf::Color a = sf::Color::Red;
sf::Color b(255, 0, 0);
sf::Color a = sf::Color::Red;
sf::Color b(255, 0, 0);
I think we should stay with this convention, this also reduces amount of code to be changed for cosmetic reasons.
What we can do however is:
MyCStruct aggregate = {};
std::vector<int> v = {1, 2, 3}; // std::initializer_list, not uniform initialization
std::vector<int> v = {1, 2, 3}; // std::initializer_list, not uniform initialization
Type inference (auto + decltype)
We should use auto where it helps increase readability. Iterator declarations are such an example, also repetition of type names are potential candidates:
auto red = sf::Color::Red;
// instead of
sf::Color red = sf::Color::Red;
// instead of
sf::Color red = sf::Color::Red;
I would not use it where the type information helps the understanding of the code, or in function signatures or return types. It's also possible to introduce bugs by propagating a wrong type through multiple type-inferred functions.
decltype is typically used in generic code when it's hard to deduce the type of an expression. I don't think SFML has many places where it's needed.
Emplacement
Good if heavy copies can be avoided (moving SFML resource classes into a container). Otherwise, similar thoughts as in type inference section apply: it can make some code harder to understand, while not making a notable difference (due to optimization of temporary objects, or simply small enough sizes). Depends a bit on the context, good variable names can also mitigate that.
We should particularly avoid emplace_back(), when push_back() does the job (that will just cause an extra copy constructor call).
Trailing return types
In my opinion only useful when the return type depends on the parameter types. As such, we should limit their use to generic code which truly needs them.
11
General discussions / Re: New SFML Team Member: SuperV1234
« on: November 30, 2021, 09:34:54 pm »
Welcome to the team, and have a great start!
12
SFML development / Re: Getting the ball rolling
« on: November 22, 2021, 04:52:54 pm »The large PR is, as previously mentioned, a "proof of concept". More of a statement than anything, saying: "Hey, I did this in two days and it works on a real game. Let's get to work!".I see, but since you opened a PR in the SFML repo, the idea is probably also that other users can benefit from the changes and at some point it becomes part of the "official" version, no?
I don't want that PR to be merged, I want to use it as a testbed and maybe maintain my own fork of SFML as I don't care about stability and want to experiment with stuff.
It's totally fine to have things messy as a PoC, but I'd still say we should clean a bit up before merging to the main repo (on any branch). And by that, I definitely don't mean the code needs to be perfect, but things like passing CI, readable commit history, and an easy way to comprehend changes (this is where commit messages help, I don't insist on changelog for now). Not for me personally, but to give other users the option to comprehend the changes and to contribute themselves with feedback, comments, improvements.
It's part of continuous integration: make sure things are more or less "green" right away -- otherwise we accumulate tech debt that we have to sort out later, in a giant PR when merging to master. In my opinion, if we take SFML 3 serious, we should make the branch usable from the start, and motivate users to try things out.
This is also what we would communicate by staying on master. It doesn't mean there won't be many changes, or even breaking ones, but that it's in an internally consistent state and ready-to-use, and it makes us accountable
Would be great to hear other team members' input here.
13
SFML development / Re: Getting the ball rolling
« on: November 22, 2021, 03:44:03 pm »
Thanks a lot for taking the initiative!
Your proposal makes sense; I wonder though if we should not use the master branch directly for SFML 3, and maintain a separate sfml2 branch for a while.
I think there are mainly two categories of SFML users:
Regarding sf::Angle in particular, it may seem like a small reasonable improvement, but it's 1. a breaking change, so cannot be merged to SFML 2 even if everyone agreed, and 2. not uncontroversial -- many gamedev libraries and game engines stick to floats and a fixed convention (radian/degree). But generally, I get your point with the lack of progress.
What I would appreciate regarding contributions, is that pull requests are reasonably sized. We can make an exception for the initial one, but if possible let's not add even more to this one. The risk of big PRs is that it becomes a daunting task to review, and people are scared off if they have to invest multiple hours just to go through the code -- which is exactly what we don't want
Also, would it be possible to merge dozens of commits to one, or a few, with meaningful commit messages? That would also help understanding a lot
Your proposal makes sense; I wonder though if we should not use the master branch directly for SFML 3, and maintain a separate sfml2 branch for a while.
I think there are mainly two categories of SFML users:
- Those who want a stable version -- they usually stick to official releases.
- Those who want the latest features -- they use master.
Regarding sf::Angle in particular, it may seem like a small reasonable improvement, but it's 1. a breaking change, so cannot be merged to SFML 2 even if everyone agreed, and 2. not uncontroversial -- many gamedev libraries and game engines stick to floats and a fixed convention (radian/degree). But generally, I get your point with the lack of progress.
What I would appreciate regarding contributions, is that pull requests are reasonably sized. We can make an exception for the initial one, but if possible let's not add even more to this one. The risk of big PRs is that it becomes a daunting task to review, and people are scared off if they have to invest multiple hours just to go through the code -- which is exactly what we don't want
Also, would it be possible to merge dozens of commits to one, or a few, with meaningful commit messages? That would also help understanding a lot
14
SFML development / Re: FreeType "All users should update immediately."
« on: September 02, 2021, 11:06:18 am »
Hey, thanks for the heads up!
According to the release page:
We don't use that option in SFML. Explanation can be found in extlibs/headers/freetype2/config/ftoption.h:
That being said, this is definitely not the only vulnerability that was fixed in our C dependencies, and it would probably make sense to update all of them. What do others think?
According to the release page:
Quote
I. IMPORTANT BUG FIXES
- A heap buffer overflow has been found in the handling of embedded
PNG bitmaps, introduced in FreeType version 2.6.
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-15999
If you use option FT_CONFIG_OPTION_USE_PNG you should upgrade
immediately.
We don't use that option in SFML. Explanation can be found in extlibs/headers/freetype2/config/ftoption.h:
/*************************************************************************/
/* */
/* PNG bitmap support. */
/* */
/* FreeType now handles loading color bitmap glyphs in the PNG format. */
/* This requires help from the external libpng library. Uncompressed */
/* color bitmaps do not need any external libraries and will be */
/* supported regardless of this configuration. */
/* */
/* Define this macro if you want to enable this `feature'. */
/* */
/* #define FT_CONFIG_OPTION_USE_PNG */
/* */
/* PNG bitmap support. */
/* */
/* FreeType now handles loading color bitmap glyphs in the PNG format. */
/* This requires help from the external libpng library. Uncompressed */
/* color bitmaps do not need any external libraries and will be */
/* supported regardless of this configuration. */
/* */
/* Define this macro if you want to enable this `feature'. */
/* */
/* #define FT_CONFIG_OPTION_USE_PNG */
That being said, this is definitely not the only vulnerability that was fixed in our C dependencies, and it would probably make sense to update all of them. What do others think?
15
Graphics / Re: State machine and sf::Drawable
« on: August 07, 2021, 12:13:31 pm »Should I make my State class inherit from sf::Drawable? That enables me to do:The question you should ask yourself: what do you want to achieve with the sf::Drawable? Ideally the answer is not syntax on its own, but rather some of the abstraction it provides. Do you use the sf::RenderStates parameter, for example?_window->draw(myAmazingState.top());
// rather than myAmazingState.top().Draw(_window);
If there's no benefit right now, I would tend to keep things simple. It's not uncommon that user classes have a custom Update() and Draw() function, for game logic and rendering. The advantage of a custom function is that you can pass extra parameters that don't fit the sf::Drawable interface, for example:
void GameScene::Draw(bool showGui, /* other options */)
If it should really prove to be a problem, it's quite an easy and quick change.