Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: Getting the ball rolling  (Read 20311 times)

0 Members and 1 Guest are viewing this topic.

SuperV1234

  • Moderator
  • Full Member
  • *****
  • Posts: 190
    • View Profile
Getting the ball rolling
« on: November 19, 2021, 07:15:16 pm »
This is a post from the perspective of a SFML user who has contributed small PRs in the past. To me, it feels like the development of SFML is stagnant. It's 2021, and SFML is still stuck on C++03 for no good reason whatsoever.

I open the roadmap on GitHub and I see a PR for a small reasonable improvement that was created back on the 12th of October 2014, and that is still open and unmerged.

- Why is the development of SFML stagnant?
- Why does it feel like there have been no major improvements for almost a decade?
- Who decides what should be worked on?
- How does someone apply to be part of the team that decides the fate of SFML?

I want to see SFML transition to C++17 and become a modern, useful and popular library.
Yes, I am volunteering to do the work. But I don't want to send a PR that's going to be left untouched for 7 years.

How do we get the ball rolling?

This is my proposal:
1. I'll create a branch for SFML 3.x, which is going to be C++17-only.
2. I'll start making some basic changes (e.g. adding move operations for `sf::Shader` and similar classes), and create PRs towards that branch.
3. Have the SFML team review the changes and get consensus as we go along. Don't aim for perfection. Don't aim to have a perfect plan right from the beginning. Move fast and break things.
4. Get a few major changes done (e.g. the entire codebase uses move semantics), and ask for community guidance and feedback.
5. Rinse and repeat.

Shall we?

SuperV1234

  • Moderator
  • Full Member
  • *****
  • Posts: 190
    • View Profile
Re: Getting the ball rolling
« Reply #1 on: November 19, 2021, 09:44:05 pm »
Here's a proof of concept PR that converts SFML to C++17 and removes some classes made obsolete by the C++ Standard Library: https://github.com/SFML/SFML/pull/1847

Nexus

  • Moderator
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: Getting the ball rolling
« Reply #2 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:
  • Those who want a stable version -- they usually stick to official releases.
  • Those who want the latest features -- they use master.
The problem I see with a separate sfml3 branch, is that it will get next to no exposure, meaning less feedback and less testing. But I'm open to hear other team members' opinions here.

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 :)
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

SuperV1234

  • Moderator
  • Full Member
  • *****
  • Posts: 190
    • View Profile
Re: Getting the ball rolling
« Reply #3 on: November 22, 2021, 04:26:37 pm »
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 that creating a `sfml3` branch is a good starting point. It doesn't mess with `master`, and we don't need a lot of feedback/exposure while we're just starting out. As soon as we have something more significant, we can start asking for feedback and potentially change our approach.

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 :)

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 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.

These are the things I request:
1. Please give me a target branch I can send PRs to, in order to initiate the SFML3 project.
2. Please tell me that my (small) PRs will be reviewed and merged in a timely manner, within reason.

If I can have both (1) and (2), I'll start creating small PRs that can be individually reviewed. The first one will only change CMakeLists to build in C++17 mode. The second one will have some small C++17 usage. And so on.

Looking forward to know how to proceed.
Cheers!

Nexus

  • Moderator
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: Getting the ball rolling
« Reply #4 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 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.
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?

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.
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

eXpl0it3r

  • SFML Team
  • Hero Member
  • *****
  • Posts: 11027
    • View Profile
    • development blog
    • Email
Re: Getting the ball rolling
« Reply #5 on: November 22, 2021, 05:28:38 pm »
It's great to see some more activity here! :)

Due to the failure of me fully finishing the Scancode feature in the past months, I think we're at a point, where the strategy needs to change slightly.

My current plan of attack would be the following:
  • Get the warnings changes merged, as this would cause lots of conflicts against larger changes
  • Create a sort of release branch 2.6.x, for any remaining features/fixes for SFML 2.6 (e.g. Scancodes)
  • Start with step-by-step PRs for C++17 on the master branch
  • Back merge changes on the 2.6.x branch to master (limited set of changes)
  • Back port critical fixes to 2.6.x and consider patch releases after the 2.6.0 release

As originally defined in the Roadmap, I'd like to keep the SFML 3 release relatively slim and just mostly get C++17 changes in and much rather do a bigger API jump for SFML 4. There's some advantages to long living API stability (e.g. see all the SFML learning material that applies for basically all SFML 2 versions), but ultimately trying to bring a whole API evolution next to a C++17 refactoring in one release, will extend the release horizon already to multiple years, but one step at a time. ;D
« Last Edit: November 22, 2021, 05:30:19 pm by eXpl0it3r »
Official FAQ: https://www.sfml-dev.org/faq.php
Official Discord Server: https://discord.gg/nr4X7Fh
——————————————————————
Dev Blog: https://duerrenberger.dev/blog/

SuperV1234

  • Moderator
  • Full Member
  • *****
  • Posts: 190
    • View Profile
Re: Getting the ball rolling
« Reply #6 on: November 23, 2021, 12:46:43 am »
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?

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.

I totally agree with you, and that's why I don't want my big PR to be merged. If someone wants to use it, they can checkout the branch and play around with it.

For merging stuff into SFML, I want to create smaller and easily-reviewable PRs. I already have a plan of how to split the work, I just need a target branch and someone that reviews the stuff.



My current plan of attack would be the following:
  • Get the warnings changes merged, as this would cause lots of conflicts against larger changes
  • Create a sort of release branch 2.6.x, for any remaining features/fixes for SFML 2.6 (e.g. Scancodes)
  • Start with step-by-step PRs for C++17 on the master branch
  • Back merge changes on the 2.6.x branch to master (limited set of changes)
  • Back port critical fixes to 2.6.x and consider patch releases after the 2.6.0 release

The plan sounds good, but if we had a separate `sfml3` branch we could already start sending PRs towards it in parallel, we don't need to wait for scancodes or #1846. Merging either into the `sfml3` would be relatively easy.

As originally defined in the Roadmap, I'd like to keep the SFML 3 release relatively slim and just mostly get C++17 changes in and much rather do a bigger API jump for SFML 4. There's some advantages to long living API stability (e.g. see all the SFML learning material that applies for basically all SFML 2 versions), but ultimately trying to bring a whole API evolution next to a C++17 refactoring in one release, will extend the release horizon already to multiple years, but one step at a time. ;D

I disagree with this point, I think that this is a great opportunity to clean up the API. It's been years and we don't have SFML 3 yet. Are we seriously going to ask people to wait a decade before we can do some API breaks?

If users are not able to make small reasonable changes to their code to keep up with an evolving library, they can always use the older versions. We don't want to end up in a situation where we have to sacrifice the quality and speed of our work. SFML 3 offers an opportunity to improve SFML as a whole, and a new major version does imply API-breaking changes.

eXpl0it3r

  • SFML Team
  • Hero Member
  • *****
  • Posts: 11027
    • View Profile
    • development blog
    • Email
Re: Getting the ball rolling
« Reply #7 on: November 23, 2021, 08:51:38 am »
The plan sounds good, but if we had a separate `sfml3` branch we could already start sending PRs towards it in parallel, we don't need to wait for scancodes or #1846. Merging either into the `sfml3` would be relatively easy.
Given the amount of incoming changes with #1846 I think you're only cause yourself more problem by starting to refactor things in parallel, since you'll end up with a lot of merge conflicts.
Besides #1846 is close to the finish line and we need to do things one step at a time, if everything is done in parallel, you won't find enough resources to stay on top of it.

I disagree with this point, I think that this is a great opportunity to clean up the API. It's been years and we don't have SFML 3 yet. Are we seriously going to ask people to wait a decade before we can do some API breaks?
You misunderstood me. We've been promising a ton of stuff for the past 8 years for SFML 3, I don't want SFML 3 to be delayed by many years, just because we're trying to chase down every last breaking API change, but I'm much more willing to release SFML 3 with the sole focus on C++17 changes and move on to SFML 4 much quicker. Also quite a few open features, can also be implemented without breaking API, so those shouldn't be in focus for SFML 3.0.0 either, but can be part of 3.1 or later.

See the Roadmap: https://en.sfml-dev.org/forums/index.php?topic=24372.0
« Last Edit: December 07, 2021, 03:14:08 pm by eXpl0it3r »
Official FAQ: https://www.sfml-dev.org/faq.php
Official Discord Server: https://discord.gg/nr4X7Fh
——————————————————————
Dev Blog: https://duerrenberger.dev/blog/

eXpl0it3r

  • SFML Team
  • Hero Member
  • *****
  • Posts: 11027
    • View Profile
    • development blog
    • Email
Re: Getting the ball rolling
« Reply #8 on: November 30, 2021, 03:03:24 pm »
I've added a short update to the Roadmap thread.

In short, the master branch can be used as target for SFML 3 changes and thus also C++17 changes! :)
Official FAQ: https://www.sfml-dev.org/faq.php
Official Discord Server: https://discord.gg/nr4X7Fh
——————————————————————
Dev Blog: https://duerrenberger.dev/blog/